From: Jeff Layton <jlayton@kernel.org>
To: Matthew Wilcox <willy@infradead.org>
Cc: ceph-devel@vger.kernel.org, linux-cachefs@redhat.com,
pfmeec@rit.edu, dhowells@redhat.com, idryomov@gmail.com,
stable@vger.kernel.org, Andrew W Elble <aweits@rit.edu>
Subject: Re: [PATCH v4] ceph: fix write_begin optimization when write is beyond EOF
Date: Sun, 13 Jun 2021 11:25:48 -0400 [thread overview]
Message-ID: <04174e6beaff47fb4dc32ff6cc32ae8667008edd.camel@kernel.org> (raw)
In-Reply-To: <YMYg+dYOhSVGg58R@casper.infradead.org>
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@kernel.org>
prev parent reply other threads:[~2021-06-13 15:25 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-11 19:59 [PATCH] ceph: fix write_begin optimization when write is beyond EOF Jeff Layton
2021-06-11 20:48 ` Matthew Wilcox
2021-06-11 22:20 ` Jeff Layton
2021-06-12 0:11 ` [PATCH v2] " Jeff Layton
2021-06-12 13:36 ` Matthew Wilcox
2021-06-12 18:35 ` [PATCH v3] " Jeff Layton
2021-06-13 11:04 ` Matthew Wilcox
2021-06-13 11:36 ` [PATCH v4] " Jeff Layton
2021-06-13 12:02 ` Jeff Layton
2021-06-13 15:15 ` Matthew Wilcox
2021-06-13 15:25 ` Jeff Layton [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=04174e6beaff47fb4dc32ff6cc32ae8667008edd.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=aweits@rit.edu \
--cc=ceph-devel@vger.kernel.org \
--cc=dhowells@redhat.com \
--cc=idryomov@gmail.com \
--cc=linux-cachefs@redhat.com \
--cc=pfmeec@rit.edu \
--cc=stable@vger.kernel.org \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).