On Wed, Jul 17, 2019 at 8:07 AM Bruce Evans <b...@optusnet.com.au> wrote:
>
> 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.
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.
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
