From owner-svn-src-all@freebsd.org Wed Jul 17 16:02:10 2019 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id F35C9AFE14; Wed, 17 Jul 2019 16:02:10 +0000 (UTC) (envelope-from asomers@gmail.com) Received: from mail-lj1-f196.google.com (mail-lj1-f196.google.com [209.85.208.196]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 03F3487E41; Wed, 17 Jul 2019 16:02:09 +0000 (UTC) (envelope-from asomers@gmail.com) Received: by mail-lj1-f196.google.com with SMTP id z28so24226652ljn.4; Wed, 17 Jul 2019 09:02:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=qWSoCM83uz2K0MeYTftuNQazKMdqtB89zbdt8psr4lA=; b=V9awrTs/3YsvsbWJ8+RM4IJQZgjsxDEW/EQ2ypXicOce1OvmztkIhyKB9upW8st1tr Bw+5rBg1zEfDP4P1uaE1Rw0oEi8MJt7hNWT8mCgkBMR8SuimraZH5+g+58LMNtmcNxHp 0jZj8Ah/idiDmOhq5owcIlJG36XA9OwMPUDThbO3nvEwfK8N1S7XRNas5ZngzbPon5OA 8vOU9d8uIoO0B7ZYIJs4rRzp3yDIyRKU97mgRaK4lGRTuxahq/y/fTpoyvid+Y7fWSPk 8UsN3rB+u4k2AecCxYXGiDFLJNLPUI7fn0i80XGchx77h9qU5gxVm5cwhpNbPxHQsHeT DJnA== X-Gm-Message-State: APjAAAXmL6Hd+fiLclsY+CHGATtHfN8MeuhvGasbSoHcdr5Lb7XoeFvR JK7bBZ4LZQ1TWr/u5t+bZW6VWyTtf4OXz6TfRkCLWA== X-Google-Smtp-Source: APXvYqybVfYOLsGePOcMiSuq/oZsVsEp3vb+OF95n2xwmO7JTqY3NAHMSY687EbRe6BUN8VujYXyqbqXilzcFfcGaVE= X-Received: by 2002:a2e:9cd1:: with SMTP id g17mr21864167ljj.234.1563379321957; Wed, 17 Jul 2019 09:02:01 -0700 (PDT) MIME-Version: 1.0 References: <201906251944.x5PJiNaA093352@repo.freebsd.org> <3b2ade4e-d19d-31e4-7ee2-c0085a00e53e@FreeBSD.org> <20190716180618.C962@besplex.bde.org> <20190717235525.L1726@besplex.bde.org> In-Reply-To: <20190717235525.L1726@besplex.bde.org> From: Alan Somers Date: Wed, 17 Jul 2019 10:01:50 -0600 Message-ID: Subject: Re: svn commit: r349391 - head/sys/kern To: Bruce Evans Cc: John Baldwin , "Conrad E. Meyer" , src-committers , svn-src-all , svn-src-head Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 03F3487E41 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; spf=pass (mx1.freebsd.org: domain of asomers@gmail.com designates 209.85.208.196 as permitted sender) smtp.mailfrom=asomers@gmail.com X-Spamd-Result: default: False [-2.90 / 15.00]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-0.94)[-0.937,0]; FROM_HAS_DN(0.00)[]; R_SPF_ALLOW(-0.20)[+ip4:209.85.128.0/17]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; MIME_GOOD(-0.10)[text/plain]; MIME_TRACE(0.00)[0:+]; DMARC_NA(0.00)[freebsd.org]; RCPT_COUNT_FIVE(0.00)[6]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TO_DN_ALL(0.00)[]; MX_GOOD(-0.01)[cached: alt3.gmail-smtp-in.l.google.com]; RCVD_IN_DNSWL_NONE(0.00)[196.208.85.209.list.dnswl.org : 127.0.5.0]; NEURAL_SPAM_SHORT(0.24)[0.239,0]; RCVD_TLS_LAST(0.00)[]; FORGED_SENDER(0.30)[asomers@freebsd.org,asomers@gmail.com]; FREEMAIL_TO(0.00)[optusnet.com.au]; RWL_MAILSPIKE_POSSIBLE(0.00)[196.208.85.209.rep.mailspike.net : 127.0.0.17]; R_DKIM_NA(0.00)[]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US]; FROM_NEQ_ENVFROM(0.00)[asomers@freebsd.org,asomers@gmail.com]; IP_SCORE(-1.19)[ipnet: 209.85.128.0/17(-3.45), asn: 15169(-2.44), country: US(-0.05)]; RCVD_COUNT_TWO(0.00)[2] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Jul 2019 16:02:11 -0000 On Wed, Jul 17, 2019 at 8:07 AM Bruce Evans wrote: > > On Tue, 16 Jul 2019, Alan Somers wrote: > > > On Tue, Jul 16, 2019 at 3:48 AM Bruce Evans wrote: > >> > >> On Mon, 15 Jul 2019, John Baldwin wrote: > >>> ... > >>> I'm not sure which variants are most readable: > >> ... > >>> C) > >>> if (fp->f_seqcount + howmany(resid, 16384) < fp->f_seqcount) > >>> fp->f_seqcount = IO_SEQMAX; > >>> else > >>> fp->f_seqcount = lmin(IO_SEQMAX, > >>> fp->f_seqcount + howmany(resid, 16384)); > >>> > >>> I'm probably not a fan of C). > >> > >> On supported arches, it recovers from overflow in howmany(), but only if > >> the compiler doesn't optimize away the recovery. > >> > >> The first part can be done better: > >> > >> if (uio->uio_resid >= IO_SEQMAX * 16384) > >> fp->f_seqcount = IO_SEQMAX; > >> > >> Then for the second part, any version works since all values are small and > >> non-negative, but I prefer to restore the the version that I rewrote 15-20 > >> years ago and committed 11 years ago (r175106): > >> > >> fp->f_seqcount += howmany(uio->uio_resid, 16384); > >> if (fp->f_seqcount > IO_SEQMAX) > >> fp->f_seqcount = IO_SEQMAX; > >> ... > > Bruce, > > Is this how you want it? > > Index: sys/kern/vfs_vnops.c > > =================================================================== > > --- sys/kern/vfs_vnops.c (revision 349977) > > +++ sys/kern/vfs_vnops.c (working copy) > > @@ -499,8 +499,13 @@ > > * closely related to the best I/O size for real disks than > > * to any block size used by software. > > */ > > - fp->f_seqcount += lmin(IO_SEQMAX, > > - howmany(uio->uio_resid, 16384)); > > + if (uio->uio_resid >= IO_SEQMAX * 16384) > > + fp->f_seqcount = IO_SEQMAX; > > + else { > > + fp->f_seqcount += howmany(uio->uio_resid, 16384); > > + if (fp->f_seqcount > IO_SEQMAX) > > + fp->f_seqcount = IO_SEQMAX; > > + } > > return (fp->f_seqcount << IO_SEQSHIFT); > > } > > That looks OK, except it is misformatted with tabs corrupted to other than > 8 spaces. The screwed up whitespace is just an artifact of gmail. > > I checked that uio_resid is checked by some callers to be >= 0, so we > don't have to check that here. (Callers between userland and here > start with size_t nbytes or an array of sizes for readv(), and have > to check that nbytes or the sum of the sizes fits in ssize_t, else > overflow would alread have occurred on assigment to uio_resid. Callers > that construct a uio that is not so directly controlled by userland > presumably don't construct ones with unusable sizes.) > > Bruce Ok, I'll commit it then.