On 7/14/19 12:08 PM, Conrad Meyer wrote:
> Hi Alan,
>
> This change restores the possible overflow beyond IO_SEQMAX that the
> removed conditional prevented.
>
> On Tue, Jun 25, 2019 at 12:44 PM Alan Somers <asom...@freebsd.org> wrote:
>>
>> Author: asomers
>> Date: Tue Jun 25 19:44:22 2019
>> New Revision: 349391
>> URL: https://svnweb.freebsd.org/changeset/base/349391
>>
>> --- head/sys/kern/vfs_vnops.c Tue Jun 25 19:36:01 2019 (r349390)
>> +++ head/sys/kern/vfs_vnops.c Tue Jun 25 19:44:22 2019 (r349391)
>> @@ -499,10 +499,8 @@ sequential_heuristic(struct uio *uio, struct file *fp)
>> * closely related to the best I/O size for real disks than
>> * to any block size used by software.
>> */
>> - fp->f_seqcount += MIN(IO_SEQMAX,
>> + fp->f_seqcount += lmin(IO_SEQMAX,
>> howmany(uio->uio_resid, 16384));
>> - if (fp->f_seqcount > IO_SEQMAX)
>> - fp->f_seqcount = IO_SEQMAX;
>> return (fp->f_seqcount << IO_SEQSHIFT);
>> }
>
> Perhaps instead this should be:
>
> fp->f_seqcount = lmin(IO_SEQMAX,
> fp->f_seqcount + howmany(...));
While I agree with your assessment of the removed conditional, I think the
suggestion above can have a funky overflow where f_seqcount + howmany(...) ends
up less than IO_SEQMAX and this expression would then be used instead of
IO_SEQMAX?
The first lmin seems designed to minimize what we add in the hope that since the
existing value is bounded to '0...IO_SEQMAX' the result of the += can only be in
the range '0...2*IO_SEQMAX'.
I'm not sure which variants are most readable:
A)
fp->f_seqcount += lmin(IO_SEQMAX,
howmany(resid, 16384));
if (fp->f_seqcount > IO_SEQMAX)
fp->f_seqcount = IO_SEQMAX;
B)
fp->f_seqcount += lmin(IO_SEQMAX,
howmany(resid, 16384));
fp->f_seqcount = lmin(fp->f_seqcount, IO_SEQMAX);
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).
--
John Baldwin
_______________________________________________
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"
