| Message ID | 162429001766.2770648.1072619730387446884.stgit@warthog.procyon.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | netfs, afs: Fix write_begin/end | expand |
On Mon, Jun 21, 2021 at 04:40:17PM +0100, David Howells wrote: > Fix afs_write_end() to correctly handle a short copy into the intended > write region of the page. Two things are necessary: > > (1) If the page is not up to date, then we should just return 0 > (ie. indicating a zero-length copy). The loop in > generic_perform_write() will go around again, possibly breaking up the > iterator into discrete chunks[1]. > > This is analogous to commit b9de313cf05fe08fa59efaf19756ec5283af672a > for ceph. > > (2) The page should not have been set uptodate if it wasn't completely set > up by netfs_write_begin() (this will be fixed in the next patch), so > we need to set uptodate here in such a case. > > Also remove the assertion that was checking that the page was set uptodate > since it's now set uptodate if it wasn't already a few lines above. The > assertion was from when uptodate was set elsewhere. > > Changes: > v3: Remove the handling of len exceeding the end of the page. > > Fixes: 3003bbd0697b ("afs: Use the netfs_write_begin() helper") > Reported-by: Jeff Layton <jlayton@kernel.org> > Signed-off-by: David Howells <dhowells@redhat.com> > Acked-by: Jeff Layton <jlayton@kernel.org> > cc: Al Viro <viro@zeniv.linux.org.uk> > cc: Matthew Wilcox <willy@infradead.org> > cc: linux-afs@lists.infradead.org > Link: https://lore.kernel.org/r/YMwVp268KTzTf8cN@zeniv-ca.linux.org.uk/ [1] > Link: https://lore.kernel.org/r/162367682522.460125.5652091227576721609.stgit@warthog.procyon.org.uk/ # v1 > Link: https://lore.kernel.org/r/162391825688.1173366.3437507255136307904.stgit@warthog.procyon.org.uk/ # v2 Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
diff --git a/fs/afs/write.c b/fs/afs/write.c index a523bb86915d..641c54679399 100644 --- a/fs/afs/write.c +++ b/fs/afs/write.c @@ -118,6 +118,15 @@ int afs_write_end(struct file *file, struct address_space *mapping, _enter("{%llx:%llu},{%lx}", vnode->fid.vid, vnode->fid.vnode, page->index); + if (!PageUptodate(page)) { + if (copied < len) { + copied = 0; + goto out; + } + + SetPageUptodate(page); + } + if (copied == 0) goto out; @@ -132,8 +141,6 @@ int afs_write_end(struct file *file, struct address_space *mapping, write_sequnlock(&vnode->cb_lock); } - ASSERT(PageUptodate(page)); - if (PagePrivate(page)) { priv = page_private(page); f = afs_page_dirty_from(page, priv);
Fix afs_write_end() to correctly handle a short copy into the intended write region of the page. Two things are necessary: (1) If the page is not up to date, then we should just return 0 (ie. indicating a zero-length copy). The loop in generic_perform_write() will go around again, possibly breaking up the iterator into discrete chunks[1]. This is analogous to commit b9de313cf05fe08fa59efaf19756ec5283af672a for ceph. (2) The page should not have been set uptodate if it wasn't completely set up by netfs_write_begin() (this will be fixed in the next patch), so we need to set uptodate here in such a case. Also remove the assertion that was checking that the page was set uptodate since it's now set uptodate if it wasn't already a few lines above. The assertion was from when uptodate was set elsewhere. Changes: v3: Remove the handling of len exceeding the end of the page. Fixes: 3003bbd0697b ("afs: Use the netfs_write_begin() helper") Reported-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: David Howells <dhowells@redhat.com> Acked-by: Jeff Layton <jlayton@kernel.org> cc: Al Viro <viro@zeniv.linux.org.uk> cc: Matthew Wilcox <willy@infradead.org> cc: linux-afs@lists.infradead.org Link: https://lore.kernel.org/r/YMwVp268KTzTf8cN@zeniv-ca.linux.org.uk/ [1] Link: https://lore.kernel.org/r/162367682522.460125.5652091227576721609.stgit@warthog.procyon.org.uk/ # v1 Link: https://lore.kernel.org/r/162391825688.1173366.3437507255136307904.stgit@warthog.procyon.org.uk/ # v2 --- fs/afs/write.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)