Commit 0201ebf2 authored by David Howells's avatar David Howells Committed by Andrew Morton

mm: merge folio_has_private()/filemap_release_folio() call pairs

Patch series "mm, netfs, fscache: Stop read optimisation when folio
removed from pagecache", v7.

This fixes an optimisation in fscache whereby we don't read from the cache
for a particular file until we know that there's data there that we don't
have in the pagecache.  The problem is that I'm no longer using PG_fscache
(aka PG_private_2) to indicate that the page is cached and so I don't get
a notification when a cached page is dropped from the pagecache.

The first patch merges some folio_has_private() and
filemap_release_folio() pairs and introduces a helper,
folio_needs_release(), to indicate if a release is required.

The second patch is the actual fix.  Following Willy's suggestions[1], it
adds an AS_RELEASE_ALWAYS flag to an address_space that will make
filemap_release_folio() always call ->release_folio(), even if
PG_private/PG_private_2 aren't set.  folio_needs_release() is altered to
add a check for this.


This patch (of 2):

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.

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.

After this, the only remaining caller of folio_has_private() outside of
mm/ is a check in fuse.

Link: https://lkml.kernel.org/r/20230628104852.3391651-1-dhowells@redhat.com
Link: https://lkml.kernel.org/r/20230628104852.3391651-2-dhowells@redhat.comReported-by: default avatarRohith Surabattula <rohiths.msft@gmail.com>
Suggested-by: default avatarMatthew Wilcox <willy@infradead.org>
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Steve French <sfrench@samba.org>
Cc: Shyam Prasad N <nspmangalore@gmail.com>
Cc: Rohith Surabattula <rohiths.msft@gmail.com>
Cc: Dave Wysochanski <dwysocha@redhat.com>
Cc: Dominique Martinet <asmadeus@codewreck.org>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
Cc: Xiubo Li <xiubli@redhat.com>
Cc: Jingbo Xu <jefflexu@linux.alibaba.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
parent dba438bd
...@@ -340,10 +340,8 @@ move_extent_per_page(struct file *o_filp, struct inode *donor_inode, ...@@ -340,10 +340,8 @@ move_extent_per_page(struct file *o_filp, struct inode *donor_inode,
ext4_double_up_write_data_sem(orig_inode, donor_inode); ext4_double_up_write_data_sem(orig_inode, donor_inode);
goto data_copy; goto data_copy;
} }
if ((folio_has_private(folio[0]) && if (!filemap_release_folio(folio[0], 0) ||
!filemap_release_folio(folio[0], 0)) || !filemap_release_folio(folio[1], 0)) {
(folio_has_private(folio[1]) &&
!filemap_release_folio(folio[1], 0))) {
*err = -EBUSY; *err = -EBUSY;
goto drop_data_sem; goto drop_data_sem;
} }
...@@ -362,10 +360,8 @@ move_extent_per_page(struct file *o_filp, struct inode *donor_inode, ...@@ -362,10 +360,8 @@ move_extent_per_page(struct file *o_filp, struct inode *donor_inode,
/* At this point all buffers in range are uptodate, old mapping layout /* At this point all buffers in range are uptodate, old mapping layout
* is no longer required, try to drop it now. */ * is no longer required, try to drop it now. */
if ((folio_has_private(folio[0]) && if (!filemap_release_folio(folio[0], 0) ||
!filemap_release_folio(folio[0], 0)) || !filemap_release_folio(folio[1], 0)) {
(folio_has_private(folio[1]) &&
!filemap_release_folio(folio[1], 0))) {
*err = -EBUSY; *err = -EBUSY;
goto unlock_folios; goto unlock_folios;
} }
......
...@@ -83,8 +83,7 @@ static bool page_cache_pipe_buf_try_steal(struct pipe_inode_info *pipe, ...@@ -83,8 +83,7 @@ static bool page_cache_pipe_buf_try_steal(struct pipe_inode_info *pipe,
*/ */
folio_wait_writeback(folio); folio_wait_writeback(folio);
if (folio_has_private(folio) && if (!filemap_release_folio(folio, GFP_KERNEL))
!filemap_release_folio(folio, GFP_KERNEL))
goto out_unlock; goto out_unlock;
/* /*
......
...@@ -4073,6 +4073,8 @@ bool filemap_release_folio(struct folio *folio, gfp_t gfp) ...@@ -4073,6 +4073,8 @@ bool filemap_release_folio(struct folio *folio, gfp_t gfp)
struct address_space * const mapping = folio->mapping; struct address_space * const mapping = folio->mapping;
BUG_ON(!folio_test_locked(folio)); BUG_ON(!folio_test_locked(folio));
if (!folio_needs_release(folio))
return true;
if (folio_test_writeback(folio)) if (folio_test_writeback(folio))
return false; return false;
......
...@@ -2697,8 +2697,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list) ...@@ -2697,8 +2697,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
gfp = current_gfp_context(mapping_gfp_mask(mapping) & gfp = current_gfp_context(mapping_gfp_mask(mapping) &
GFP_RECLAIM_MASK); GFP_RECLAIM_MASK);
if (folio_test_private(folio) && if (!filemap_release_folio(folio, gfp)) {
!filemap_release_folio(folio, gfp)) {
ret = -EBUSY; ret = -EBUSY;
goto out; goto out;
} }
......
...@@ -176,6 +176,14 @@ static inline void set_page_refcounted(struct page *page) ...@@ -176,6 +176,14 @@ static inline void set_page_refcounted(struct page *page)
set_page_count(page, 1); 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; extern unsigned long highest_memmap_pfn;
/* /*
......
...@@ -2078,8 +2078,7 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr, ...@@ -2078,8 +2078,7 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
goto out_unlock; goto out_unlock;
} }
if (folio_has_private(folio) && if (!filemap_release_folio(folio, GFP_KERNEL)) {
!filemap_release_folio(folio, GFP_KERNEL)) {
result = SCAN_PAGE_HAS_PRIVATE; result = SCAN_PAGE_HAS_PRIVATE;
folio_putback_lru(folio); folio_putback_lru(folio);
goto out_unlock; goto out_unlock;
......
...@@ -936,14 +936,12 @@ static int truncate_error_page(struct page *p, unsigned long pfn, ...@@ -936,14 +936,12 @@ static int truncate_error_page(struct page *p, unsigned long pfn,
struct folio *folio = page_folio(p); struct folio *folio = page_folio(p);
int err = mapping->a_ops->error_remove_page(mapping, p); int err = mapping->a_ops->error_remove_page(mapping, p);
if (err != 0) { if (err != 0)
pr_info("%#lx: Failed to punch page: %d\n", pfn, err); pr_info("%#lx: Failed to punch page: %d\n", pfn, err);
} else if (folio_has_private(folio) && else if (!filemap_release_folio(folio, GFP_NOIO))
!filemap_release_folio(folio, GFP_NOIO)) {
pr_info("%#lx: failed to release buffers\n", pfn); pr_info("%#lx: failed to release buffers\n", pfn);
} else { else
ret = MF_RECOVERED; ret = MF_RECOVERED;
}
} else { } else {
/* /*
* If the file system doesn't support it just invalidate * If the file system doesn't support it just invalidate
......
...@@ -922,8 +922,7 @@ static int fallback_migrate_folio(struct address_space *mapping, ...@@ -922,8 +922,7 @@ static int fallback_migrate_folio(struct address_space *mapping,
* Buffers may be managed in a filesystem specific way. * Buffers may be managed in a filesystem specific way.
* We must have no buffers or drop them. * We must have no buffers or drop them.
*/ */
if (folio_test_private(src) && if (!filemap_release_folio(src, GFP_KERNEL))
!filemap_release_folio(src, GFP_KERNEL))
return mode == MIGRATE_SYNC ? -EAGAIN : -EBUSY; return mode == MIGRATE_SYNC ? -EAGAIN : -EBUSY;
return migrate_folio(mapping, dst, src, mode); return migrate_folio(mapping, dst, src, mode);
......
...@@ -19,7 +19,6 @@ ...@@ -19,7 +19,6 @@
#include <linux/highmem.h> #include <linux/highmem.h>
#include <linux/pagevec.h> #include <linux/pagevec.h>
#include <linux/task_io_accounting_ops.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/shmem_fs.h>
#include <linux/rmap.h> #include <linux/rmap.h>
#include "internal.h" #include "internal.h"
...@@ -276,7 +275,7 @@ static long mapping_evict_folio(struct address_space *mapping, ...@@ -276,7 +275,7 @@ static long mapping_evict_folio(struct address_space *mapping,
if (folio_ref_count(folio) > if (folio_ref_count(folio) >
folio_nr_pages(folio) + folio_has_private(folio) + 1) folio_nr_pages(folio) + folio_has_private(folio) + 1)
return 0; return 0;
if (folio_has_private(folio) && !filemap_release_folio(folio, 0)) if (!filemap_release_folio(folio, 0))
return 0; return 0;
return remove_mapping(mapping, folio); return remove_mapping(mapping, folio);
...@@ -573,8 +572,7 @@ static int invalidate_complete_folio2(struct address_space *mapping, ...@@ -573,8 +572,7 @@ static int invalidate_complete_folio2(struct address_space *mapping,
if (folio->mapping != mapping) if (folio->mapping != mapping)
return 0; return 0;
if (folio_has_private(folio) && if (!filemap_release_folio(folio, GFP_KERNEL))
!filemap_release_folio(folio, GFP_KERNEL))
return 0; return 0;
spin_lock(&mapping->host->i_lock); spin_lock(&mapping->host->i_lock);
......
...@@ -2064,7 +2064,7 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, ...@@ -2064,7 +2064,7 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
* (refcount == 1) it can be freed. Otherwise, leave * (refcount == 1) it can be freed. Otherwise, leave
* the folio on the LRU so it is swappable. * 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)) if (!filemap_release_folio(folio, sc->gfp_mask))
goto activate_locked; goto activate_locked;
if (!mapping && folio_ref_count(folio) == 1) { if (!mapping && folio_ref_count(folio) == 1) {
...@@ -2729,9 +2729,9 @@ static void shrink_active_list(unsigned long nr_to_scan, ...@@ -2729,9 +2729,9 @@ static void shrink_active_list(unsigned long nr_to_scan,
} }
if (unlikely(buffer_heads_over_limit)) { if (unlikely(buffer_heads_over_limit)) {
if (folio_test_private(folio) && folio_trylock(folio)) { if (folio_needs_release(folio) &&
if (folio_test_private(folio)) folio_trylock(folio)) {
filemap_release_folio(folio, 0); filemap_release_folio(folio, 0);
folio_unlock(folio); folio_unlock(folio);
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment