Make filemap_release_folio() check folio_has_private(). Then, in most cases, where a call to folio_has_private() is immediately followed by a call to filemap_release_folio(), we can get rid of the test in the pair. The same is done to page_has_private()/try_to_release_page() call pairs. There are a couple of sites in mm/vscan.c that this can't so easily be done. In shrink_folio_list(), there are actually three cases (something different is done for incompletely invalidated buffers), but filemap_release_folio() elides two of them. In shrink_active_list(), we don't have have the folio lock yet, so the check allows us to avoid locking the page unnecessarily. A wrapper function to check if a folio needs release is provided for those places that still need to do it in the mm/ directory. This will acquire additional parts to the condition in a future patch. Changes: ======== ver #4) - Split from fscache fix. - Moved folio_needs_release() to mm/internal.h and removed open-coded version from filemap_release_folio(). ver #3) - Fixed mapping_clear_release_always() to use clear_bit() not set_bit(). - Moved a '&&' to the correct line. ver #2) - Rewrote entirely according to Willy's suggestion[1]. Reported-by: Rohith Surabattula <rohiths.msft@xxxxxxxxx> Suggested-by: Matthew Wilcox <willy@xxxxxxxxxxxxx> Signed-off-by: David Howells <dhowells@xxxxxxxxxx> cc: Matthew Wilcox <willy@xxxxxxxxxxxxx> cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> cc: Steve French <sfrench@xxxxxxxxx> cc: Shyam Prasad N <nspmangalore@xxxxxxxxx> cc: Rohith Surabattula <rohiths.msft@xxxxxxxxx> cc: Dave Wysochanski <dwysocha@xxxxxxxxxx> cc: Dominique Martinet <asmadeus@xxxxxxxxxxxxx> cc: Ilya Dryomov <idryomov@xxxxxxxxx> cc: linux-cachefs@xxxxxxxxxx cc: linux-cifs@xxxxxxxxxxxxxxx cc: linux-afs@xxxxxxxxxxxxxxxxxxx cc: v9fs-developer@xxxxxxxxxxxxxxxxxxxxx cc: ceph-devel@xxxxxxxxxxxxxxx cc: linux-nfs@xxxxxxxxxxxxxxx cc: linux-fsdevel@xxxxxxxxxxxxxxx cc: linux-mm@xxxxxxxxx Link: https://lore.kernel.org/r/Yk9V/03wgdYi65Lb@xxxxxxxxxxxxxxxxxxxx/ [1] Link: https://lore.kernel.org/r/164928630577.457102.8519251179327601178.stgit@xxxxxxxxxxxxxxxxxxxxxx/ # v1 Link: https://lore.kernel.org/r/166844174069.1124521.10890506360974169994.stgit@xxxxxxxxxxxxxxxxxxxxxx/ # v2 Link: https://lore.kernel.org/r/166869495238.3720468.4878151409085146764.stgit@xxxxxxxxxxxxxxxxxxxxxx/ # v3 Link: https://lore.kernel.org/r/1459152.1669208550@xxxxxxxxxxxxxxxxxxxxxx/ # v3 also --- fs/splice.c | 3 +-- mm/filemap.c | 2 ++ mm/huge_memory.c | 3 +-- mm/internal.h | 8 ++++++++ mm/khugepaged.c | 3 +-- mm/memory-failure.c | 3 +-- mm/migrate.c | 3 +-- mm/truncate.c | 6 ++---- mm/vmscan.c | 7 +++---- 9 files changed, 20 insertions(+), 18 deletions(-) diff --git a/fs/splice.c b/fs/splice.c index 0878b852b355..563105304ccc 100644 --- a/fs/splice.c +++ b/fs/splice.c @@ -65,8 +65,7 @@ static bool page_cache_pipe_buf_try_steal(struct pipe_inode_info *pipe, */ folio_wait_writeback(folio); - if (folio_has_private(folio) && - !filemap_release_folio(folio, GFP_KERNEL)) + if (!filemap_release_folio(folio, GFP_KERNEL)) goto out_unlock; /* diff --git a/mm/filemap.c b/mm/filemap.c index 08341616ae7a..93757247cd11 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -3941,6 +3941,8 @@ bool filemap_release_folio(struct folio *folio, gfp_t gfp) struct address_space * const mapping = folio->mapping; BUG_ON(!folio_test_locked(folio)); + if (!folio_needs_release(folio)) + return true; if (folio_test_writeback(folio)) return false; diff --git a/mm/huge_memory.c b/mm/huge_memory.c index 811d19b5c4f6..308d36aa3197 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -2683,8 +2683,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list) gfp = current_gfp_context(mapping_gfp_mask(mapping) & GFP_RECLAIM_MASK); - if (folio_test_private(folio) && - !filemap_release_folio(folio, gfp)) { + if (!filemap_release_folio(folio, gfp)) { ret = -EBUSY; goto out; } diff --git a/mm/internal.h b/mm/internal.h index 6b7ef495b56d..1fefb5181ab7 100644 --- a/mm/internal.h +++ b/mm/internal.h @@ -163,6 +163,14 @@ static inline void set_page_refcounted(struct page *page) set_page_count(page, 1); } +/* + * Return true if a folio needs ->release_folio() calling upon it. + */ +static inline bool folio_needs_release(struct folio *folio) +{ + return folio_has_private(folio); +} + extern unsigned long highest_memmap_pfn; /* diff --git a/mm/khugepaged.c b/mm/khugepaged.c index 4734315f7940..7e9e0e3e678e 100644 --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -1883,8 +1883,7 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr, goto out_unlock; } - if (page_has_private(page) && - !try_to_release_page(page, GFP_KERNEL)) { + if (!try_to_release_page(page, GFP_KERNEL)) { result = SCAN_PAGE_HAS_PRIVATE; putback_lru_page(page); goto out_unlock; diff --git a/mm/memory-failure.c b/mm/memory-failure.c index bead6bccc7f2..82673fc01eed 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -831,8 +831,7 @@ static int truncate_error_page(struct page *p, unsigned long pfn, if (err != 0) { pr_info("%#lx: Failed to punch page: %d\n", pfn, err); - } else if (page_has_private(p) && - !try_to_release_page(p, GFP_NOIO)) { + } else if (!try_to_release_page(p, GFP_NOIO)) { pr_info("%#lx: failed to release buffers\n", pfn); } else { ret = MF_RECOVERED; diff --git a/mm/migrate.c b/mm/migrate.c index dff333593a8a..d721ef340943 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -905,8 +905,7 @@ static int fallback_migrate_folio(struct address_space *mapping, * Buffers may be managed in a filesystem specific way. * We must have no buffers or drop them. */ - if (folio_test_private(src) && - !filemap_release_folio(src, GFP_KERNEL)) + if (!filemap_release_folio(src, GFP_KERNEL)) return mode == MIGRATE_SYNC ? -EAGAIN : -EBUSY; return migrate_folio(mapping, dst, src, mode); diff --git a/mm/truncate.c b/mm/truncate.c index c0be77e5c008..0d4dd233f518 100644 --- a/mm/truncate.c +++ b/mm/truncate.c @@ -19,7 +19,6 @@ #include <linux/highmem.h> #include <linux/pagevec.h> #include <linux/task_io_accounting_ops.h> -#include <linux/buffer_head.h> /* grr. try_to_release_page */ #include <linux/shmem_fs.h> #include <linux/rmap.h> #include "internal.h" @@ -276,7 +275,7 @@ static long mapping_evict_folio(struct address_space *mapping, if (folio_ref_count(folio) > folio_nr_pages(folio) + folio_has_private(folio) + 1) return 0; - if (folio_has_private(folio) && !filemap_release_folio(folio, 0)) + if (!filemap_release_folio(folio, 0)) return 0; return remove_mapping(mapping, folio); @@ -581,8 +580,7 @@ static int invalidate_complete_folio2(struct address_space *mapping, if (folio->mapping != mapping) return 0; - if (folio_has_private(folio) && - !filemap_release_folio(folio, GFP_KERNEL)) + if (!filemap_release_folio(folio, GFP_KERNEL)) return 0; spin_lock(&mapping->host->i_lock); diff --git a/mm/vmscan.c b/mm/vmscan.c index 04d8b88e5216..b9316f447238 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1978,7 +1978,7 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, * (refcount == 1) it can be freed. Otherwise, leave * the folio on the LRU so it is swappable. */ - if (folio_has_private(folio)) { + if (folio_needs_release(folio)) { if (!filemap_release_folio(folio, sc->gfp_mask)) goto activate_locked; if (!mapping && folio_ref_count(folio) == 1) { @@ -2592,9 +2592,8 @@ static void shrink_active_list(unsigned long nr_to_scan, } if (unlikely(buffer_heads_over_limit)) { - if (folio_test_private(folio) && folio_trylock(folio)) { - if (folio_test_private(folio)) - filemap_release_folio(folio, 0); + if (folio_needs_release(folio) && folio_trylock(folio)) { + filemap_release_folio(folio, 0); folio_unlock(folio); } }
