On Thu, Jun 17, 2021 at 09:24:16AM +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.
>
> 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.
Thanks for adding that explanation.
> +++ b/fs/afs/write.c
> @@ -119,6 +119,16 @@ int afs_write_end(struct file *file, struct address_space *mapping,
> _enter("{%llx:%llu},{%lx}",
> vnode->fid.vid, vnode->fid.vnode, page->index);
>
> + len = min_t(size_t, len, thp_size(page) - from);
This line isn't necessary yet, right?
> + if (!PageUptodate(page)) {
> + if (copied < len) {
> + copied = 0;
> + goto out;
> + }
> +
> + SetPageUptodate(page);
> + }
> +
> if (copied == 0)
> goto out;
>
> @@ -133,8 +143,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);
The rest of this looks good.
