afs: Stop implementing ->writepage()

We're trying to get rid of the ->writepage() hook[1].  Stop afs from using
it by unlocking the page and calling afs_writepages_region() rather than
folio_write_one().

A flag is passed to afs_writepages_region() to indicate that it should only
write a single region so that we don't flush the entire file in
->write_begin(), but do add other dirty data to the region being written to
try and reduce the number of RPC ops.

This requires ->migrate_folio() to be implemented, so point that at
filemap_migrate_folio() for files and also for symlinks and directories.

This can be tested by turning on the afs_folio_dirty tracepoint and then
doing something like:

   xfs_io -c "w 2223 7000" -c "w 15000 22222" -c "w 23 7" /afs/my/test/foo

and then looking in the trace to see if the write at position 15000 gets
stored before page 0 gets dirtied for the write at position 23.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Christoph Hellwig <hch@lst.de>
cc: Matthew Wilcox <willy@infradead.org>
cc: linux-afs@lists.infradead.org
Link: https://lore.kernel.org/r/20221113162902.883850-1-hch@lst.de/
Link: https://lore.kernel.org/r/166876785552.222254.4403222906022558715.stgit@warthog.procyon.org.uk/

diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index 104df29..b7c1f8c 100644 (file)
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -77,6 +77,7 @@ const struct address_space_operations afs_dir_aops = {
        .dirty_folio    = afs_dir_dirty_folio,
        .release_folio  = afs_dir_release_folio,
        .invalidate_folio = afs_dir_invalidate_folio,
+       .migrate_folio  = filemap_migrate_folio,
 };
 
 const struct dentry_operations afs_fs_dentry_operations = {

diff --git a/fs/afs/file.c b/fs/afs/file.c
index 2eeab57..68d6d5d 100644 (file)
--- a/fs/afs/file.c
+++ b/fs/afs/file.c
@@ -58,14 +58,15 @@ const struct address_space_operations afs_file_aops = {
        .invalidate_folio = afs_invalidate_folio,
        .write_begin    = afs_write_begin,
        .write_end      = afs_write_end,
-       .writepage      = afs_writepage,
        .writepages     = afs_writepages,
+       .migrate_folio  = filemap_migrate_folio,
 };
 
 const struct address_space_operations afs_symlink_aops = {
        .read_folio     = afs_symlink_read_folio,
        .release_folio  = afs_release_folio,
        .invalidate_folio = afs_invalidate_folio,
+       .migrate_folio  = filemap_migrate_folio,
 };
 
 static const struct vm_operations_struct afs_vm_ops = {

diff --git a/fs/afs/write.c b/fs/afs/write.c
index 08fd456..19df10d 100644 (file)
--- a/fs/afs/write.c
+++ b/fs/afs/write.c
@@ -14,6 +14,11 @@
 #include <linux/netfs.h>
 #include "internal.h"
 
+static int afs_writepages_region(struct address_space *mapping,
+                                struct writeback_control *wbc,
+                                loff_t start, loff_t end, loff_t *_next,
+                                bool max_one_loop);
+
 static void afs_write_to_cache(struct afs_vnode *vnode, loff_t start, size_t len,
                               loff_t i_size, bool caching);
 
@@ -39,6 +44,25 @@ static void afs_folio_start_fscache(bool caching, struct folio *folio)
 #endif
 
 /*
+ * Flush out a conflicting write.  This may extend the write to the surrounding
+ * pages if also dirty and contiguous to the conflicting region..
+ */
+static int afs_flush_conflicting_write(struct address_space *mapping,
+                                      struct folio *folio)
+{
+       struct writeback_control wbc = {
+               .sync_mode      = WB_SYNC_ALL,
+               .nr_to_write    = LONG_MAX,
+               .range_start    = folio_pos(folio),
+               .range_end      = LLONG_MAX,
+       };
+       loff_t next;
+
+       return afs_writepages_region(mapping, &wbc, folio_pos(folio), LLONG_MAX,
+                                    &next, true);
+}
+
+/*
  * prepare to perform part of a write to a page
  */
 int afs_write_begin(struct file *file, struct address_space *mapping,
@@ -80,7 +104,8 @@ try_again:
 
                if (folio_test_writeback(folio)) {
                        trace_afs_folio_dirty(vnode, tracepoint_string("alrdy"), folio);
-                       goto flush_conflicting_write;
+                       folio_unlock(folio);
+                       goto wait_for_writeback;
                }
                /* If the file is being filled locally, allow inter-write
                 * spaces to be merged into writes.  If it's not, only write
@@ -99,8 +124,15 @@ try_again:
         * flush the page out.
         */
 flush_conflicting_write:
-       _debug("flush conflict");
-       ret = folio_write_one(folio);
+       trace_afs_folio_dirty(vnode, tracepoint_string("confl"), folio);
+       folio_unlock(folio);
+
+       ret = afs_flush_conflicting_write(mapping, folio);
+       if (ret < 0)
+               goto error;
+
+wait_for_writeback:
+       ret = folio_wait_writeback_killable(folio);
        if (ret < 0)
                goto error;
 
@@ -664,39 +696,12 @@ static ssize_t afs_write_back_from_locked_folio(struct address_space *mapping,
 }
 
 /*
- * write a page back to the server
- * - the caller locked the page for us
- */
-int afs_writepage(struct page *subpage, struct writeback_control *wbc)
-{
-       struct folio *folio = page_folio(subpage);
-       ssize_t ret;
-       loff_t start;
-
-       _enter("{%lx},", folio_index(folio));
-
-#ifdef CONFIG_AFS_FSCACHE
-       folio_wait_fscache(folio);
-#endif
-
-       start = folio_index(folio) * PAGE_SIZE;
-       ret = afs_write_back_from_locked_folio(folio_mapping(folio), wbc,
-                                              folio, start, LLONG_MAX - start);
-       if (ret < 0) {
-               _leave(" = %zd", ret);
-               return ret;
-       }
-
-       _leave(" = 0");
-       return 0;
-}
-
-/*
  * write a region of pages back to the server
  */
 static int afs_writepages_region(struct address_space *mapping,
                                 struct writeback_control *wbc,
-                                loff_t start, loff_t end, loff_t *_next)
+                                loff_t start, loff_t end, loff_t *_next,
+                                bool max_one_loop)
 {
        struct folio *folio;
        struct page *head_page;
@@ -775,6 +780,9 @@ static int afs_writepages_region(struct address_space *mapping,
 
                start += ret;
 
+               if (max_one_loop)
+                       break;
+
                cond_resched();
        } while (wbc->nr_to_write > 0);
 
@@ -806,24 +814,27 @@ int afs_writepages(struct address_space *mapping,
 
        if (wbc->range_cyclic) {
                start = mapping->writeback_index * PAGE_SIZE;
-               ret = afs_writepages_region(mapping, wbc, start, LLONG_MAX, &next);
+               ret = afs_writepages_region(mapping, wbc, start, LLONG_MAX,
+                                           &next, false);
                if (ret == 0) {
                        mapping->writeback_index = next / PAGE_SIZE;
                        if (start > 0 && wbc->nr_to_write > 0) {
                                ret = afs_writepages_region(mapping, wbc, 0,
-                                                           start, &next);
+                                                           start, &next, false);
                                if (ret == 0)
                                        mapping->writeback_index =
                                                next / PAGE_SIZE;
                        }
                }
        } else if (wbc->range_start == 0 && wbc->range_end == LLONG_MAX) {
-               ret = afs_writepages_region(mapping, wbc, 0, LLONG_MAX, &next);
+               ret = afs_writepages_region(mapping, wbc, 0, LLONG_MAX,
+                                           &next, false);
                if (wbc->nr_to_write > 0 && ret == 0)
                        mapping->writeback_index = next / PAGE_SIZE;
        } else {
                ret = afs_writepages_region(mapping, wbc,
-                                           wbc->range_start, wbc->range_end, &next);
+                                           wbc->range_start, wbc->range_end,
+                                           &next, false);
        }
 
        up_read(&vnode->validate_lock);