On Sun, 2021-06-13 at 16:15 +0100, Matthew Wilcox wrote:
> On Sun, Jun 13, 2021 at 08:02:12AM -0400, Jeff Layton wrote:
> > > + /* clamp length to end of the current page */
> > > + if (len > PAGE_SIZE)
> > > + len = PAGE_SIZE - offset;
> >
> > Actually, I think this should be:
> >
> > len = min(len, PAGE_SIZE - offset);
> >
> > Otherwise, len could still go beyond the end of the page.
>
> I don't understand why you want to clamp length instead of just coping
> with len being > PAGE_SIZE.
>
> > > +
> > > + /* full page write */
> > > + if (offset == 0 && len == PAGE_SIZE)
> > > + goto zero_out;
>
> That becomes >=.
>
> > > + /* zero-length file */
> > > + if (i_size == 0)
> > > + goto zero_out;
> > > +
> > > + /* position beyond last page in the file */
> > > + if (index > ((i_size - 1) / PAGE_SIZE))
> > > + goto zero_out;
> > > +
> > > + /* write that covers the the page from start to EOF or beyond it */
> > > + if (offset == 0 && (pos + len) >= i_size)
> > > + goto zero_out;
>
> That doesn't need any change.
>
> > > + return false;
> > > +zero_out:
> > > + zero_user_segments(page, 0, offset, offset + len, PAGE_SIZE);
>
> That also doesn't need any change.
>
Won't it though? offset+len will could be beyond the end of the page at
that point. Hmm I guess zero_user_segments does this:
if (start2 >= end2)
start2 = end2 = 0;
...so that makes the second segment copy a no-op.
Ok, fair enough -- I'll get rid of the clamping and just allow len to be
longer than PAGE_SIZE in the checks.
Thanks,
--
Jeff Layton <jlayton@xxxxxxxxxx>
