From a9eb558a5bea66cc43950632f5fffec6b5795233 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 18 Nov 2022 07:57:27 +0000 Subject: [PATCH] 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 cc: Marc Dionne cc: Christoph Hellwig cc: Matthew Wilcox cc: linux-afs@lists.infradead.org Link: https://lore.kernel.org/r/20221113162902.883850-1-hch@lst.de/ [1] Link: https://lore.kernel.org/r/166876785552.222254.4403222906022558715.stgit@warthog.procyon.org.uk/ # v1 --- fs/afs/dir.c | 1 + fs/afs/file.c | 3 ++- fs/afs/write.c | 83 +++++++++++++++++++++++++++++++++------------------------- 3 files changed, 50 insertions(+), 37 deletions(-) diff --git a/fs/afs/dir.c b/fs/afs/dir.c index 104df29..b7c1f8c 100644 --- 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 --- 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 --- a/fs/afs/write.c +++ b/fs/afs/write.c @@ -14,6 +14,11 @@ #include #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); -- 2.7.4