Date: Thu, 18 Jul 2019 00:07:00 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Alan Somers <asomers@freebsd.org> Cc: Bruce Evans <brde@optusnet.com.au>, John Baldwin <jhb@freebsd.org>, "Conrad E. Meyer" <cem@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org> Subject: Re: svn commit: r349391 - head/sys/kern Message-ID: <20190717235525.L1726@besplex.bde.org> In-Reply-To: <CAOtMX2gVQs7j%2BNnszvSeaEA5MTDLKS7%2BRz1W-xaq_DR=1VHgXg@mail.gmail.com> References: <201906251944.x5PJiNaA093352@repo.freebsd.org> <CAG6CVpUscojbGvA0=BmZPhSR%2B5Yt5ah8SQKrBaouUNDpu9tw9A@mail.gmail.com> <3b2ade4e-d19d-31e4-7ee2-c0085a00e53e@FreeBSD.org> <20190716180618.C962@besplex.bde.org> <CAOtMX2gVQs7j%2BNnszvSeaEA5MTDLKS7%2BRz1W-xaq_DR=1VHgXg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 16 Jul 2019, Alan Somers wrote: > On Tue, Jul 16, 2019 at 3:48 AM Bruce Evans <brde@optusnet.com.au> 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. 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190717235525.L1726>