On Tue, 16 Jul 2019, Alan Somers wrote:
On Tue, Jul 16, 2019 at 3:48 AM Bruce Evans <b...@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
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
