mm, netfs, fscache: Stop read optimisation when folio removed from pagecache
Fscache has an optimisation by which reads from the cache are skipped until
we know that (a) there's data there to be read and (b) that data isn't
entirely covered by pages resident in the netfs pagecache. This is done
with two flags manipulated by fscache_note_page_release():
if (...
test_bit(FSCACHE_COOKIE_HAVE_DATA, &cookie->flags) &&
test_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags))
clear_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags);
where the NO_DATA_TO_READ flag causes cachefiles_prepare_read() to indicate
that netfslib should download from the server or clear the page instead.
The fscache_note_page_release() function is intended to be called from
->releasepage() - but that only gets called if PG_private or PG_private_2
is set - and currently the former is at the discretion of the netfs and the
latter is only set whilst a page is being written to the cache, so
sometimes we miss clearing the optimisation.
Fix this by adding an extra address_space operation, ->removing folio(),
and flag, AS_NOTIFY_REMOVING_FOLIO. The operation is called if the flag is
set when a folio is removed from the pagecache. The flag should be set if
a non-NULL cookie is obtained from fscache and cleared in ->evict_inode()
before truncate_inode_pages_final() is called.
This can be tested by reading some data and then dropping caches. The
number of times the new op is called is counted in /proc/fs/fscache/stats:
RdHelp : DR=0 RA=4100 RP=0 WB=0 WBZ=0 rm=131072 <----
Reported-by: Rohith Surabattula <rohiths.msft@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Matthew Wilcox <willy@infradead.org>
cc: Steve French <sfrench@samba.org>
cc: Shyam Prasad N <nspmangalore@gmail.com>
cc: Rohith Surabattula <rohiths.msft@gmail.com>
cc: linux-cifs@vger.kernel.org
diff --git a/fs/afs/file.c b/fs/afs/file.c
index 5cd3e2f..b5c4ee5 100644
--- a/fs/afs/file.c
+++ b/fs/afs/file.c
@@ -54,6 +54,7 @@
.launder_folio = afs_launder_folio,
.releasepage = netfs_releasepage,
.invalidate_folio = netfs_invalidate_folio,
+ .removing_folio = netfs_removing_folio,
.write_begin = afs_write_begin,
.write_end = afs_write_end,
.writepage = afs_writepage,
diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index a2b75bd..a3c7b9f 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -789,6 +789,7 @@
ASSERTCMP(inode->i_ino, ==, vnode->fid.vnode);
+ clear_bit(AS_NOTIFY_REMOVING_FOLIO, &inode->i_mapping->flags);
truncate_inode_pages_final(&inode->i_data);
afs_set_cache_aux(vnode, &aux);
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 7b7ef94..7756217 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -686,6 +686,8 @@
{
#ifdef CONFIG_AFS_FSCACHE
vnode->netfs_ctx.cache = cookie;
+ if (cookie)
+ set_bit(AS_NOTIFY_REMOVING_FOLIO, &vnode->vfs_inode.i_mapping->flags);
#endif
}
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 773be62..3bdab70 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -406,6 +406,7 @@
static void
cifs_evict_inode(struct inode *inode)
{
+ clear_bit(AS_NOTIFY_REMOVING_FOLIO, &inode->i_mapping->flags);
truncate_inode_pages_final(&inode->i_data);
if (inode->i_state & I_PINNING_FSCACHE_WB)
cifs_fscache_unuse_inode_cookie(inode, true);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index b675260..5f9a973 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -3925,6 +3925,7 @@
.direct_IO = cifs_direct_io,
.invalidate_folio = netfs_invalidate_folio,
.launder_folio = cifs_launder_folio,
+ .removing_folio = netfs_removing_folio,
/*
* TODO: investigate and if useful we could add an cifs_migratePage
* helper (under an CONFIG_MIGRATION) in the future, and also
diff --git a/fs/cifs/fscache.c b/fs/cifs/fscache.c
index bb1c3a3..5d8464b 100644
--- a/fs/cifs/fscache.c
+++ b/fs/cifs/fscache.c
@@ -108,6 +108,9 @@
&cifsi->uniqueid, sizeof(cifsi->uniqueid),
&cd, sizeof(cd),
i_size_read(&cifsi->vfs_inode));
+ if (cifsi->netfs_ctx.cache)
+ set_bit(AS_NOTIFY_REMOVING_FOLIO, inode->i_mapping->flags);
+
}
void cifs_fscache_unuse_inode_cookie(struct inode *inode, bool update)
diff --git a/fs/netfs/internal.h b/fs/netfs/internal.h
index c38b787..f89e0d7 100644
--- a/fs/netfs/internal.h
+++ b/fs/netfs/internal.h
@@ -115,6 +115,7 @@
extern atomic_t netfs_n_rh_write_done;
extern atomic_t netfs_n_rh_write_failed;
extern atomic_t netfs_n_rh_write_zskip;
+extern atomic_t netfs_n_rh_remove_folio;
static inline void netfs_stat(atomic_t *stat)
diff --git a/fs/netfs/misc.c b/fs/netfs/misc.c
index e046fce..f42094d 100644
--- a/fs/netfs/misc.c
+++ b/fs/netfs/misc.c
@@ -179,7 +179,18 @@
folio_wait_fscache(folio);
}
- fscache_note_page_release(netfs_i_cookie(folio_inode(folio)));
return true;
}
EXPORT_SYMBOL(netfs_releasepage);
+
+/**
+ * netfs_removing_folio - Notification of a folio about to be removed
+ * @mapping: The pagecache about to be altered
+ * @folio: The folio about to be removed
+ */
+void netfs_removing_folio(struct address_space *mapping, struct folio *folio)
+{
+ netfs_stat(&netfs_n_rh_remove_folio);
+ fscache_note_page_release(netfs_i_cookie(mapping->host));
+}
+EXPORT_SYMBOL(netfs_removing_folio);
diff --git a/fs/netfs/stats.c b/fs/netfs/stats.c
index 85aef51..9a0c495 100644
--- a/fs/netfs/stats.c
+++ b/fs/netfs/stats.c
@@ -28,17 +28,20 @@
atomic_t netfs_n_rh_write_done;
atomic_t netfs_n_rh_write_failed;
atomic_t netfs_n_rh_write_zskip;
+atomic_t netfs_n_rh_remove_folio;
void netfs_stats_show(struct seq_file *m)
{
- seq_printf(m, "RdHelp : DR=%u RA=%u RP=%u WB=%u WBZ=%u rr=%u sr=%u\n",
+ seq_printf(m, "RdHelp : rr=%u sr=%u\n",
+ atomic_read(&netfs_n_rh_rreq),
+ atomic_read(&netfs_n_rh_sreq));
+ seq_printf(m, "RdHelp : DR=%u RA=%u RP=%u WB=%u WBZ=%u rm=%u\n",
atomic_read(&netfs_n_rh_dio_read),
atomic_read(&netfs_n_rh_readahead),
atomic_read(&netfs_n_rh_readpage),
atomic_read(&netfs_n_rh_write_begin),
atomic_read(&netfs_n_rh_write_zskip),
- atomic_read(&netfs_n_rh_rreq),
- atomic_read(&netfs_n_rh_sreq));
+ atomic_read(&netfs_n_rh_remove_folio));
seq_printf(m, "RdHelp : ZR=%u sh=%u sk=%u\n",
atomic_read(&netfs_n_rh_zero),
atomic_read(&netfs_n_rh_short_read),
diff --git a/include/linux/fs.h b/include/linux/fs.h
index bbde953..269327d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -360,6 +360,8 @@
sector_t (*bmap)(struct address_space *, sector_t);
void (*invalidate_folio) (struct folio *, size_t offset, size_t len);
int (*releasepage) (struct page *, gfp_t);
+ void (*removing_folio)(struct address_space *mapping,
+ struct folio *folio);
void (*freepage)(struct page *);
ssize_t (*direct_IO)(struct kiocb *, struct iov_iter *iter);
/*
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index b225ee0..d97d6ab 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -316,6 +316,7 @@
void **);
extern void netfs_invalidate_folio(struct folio *folio, size_t offset, size_t length);
extern int netfs_releasepage(struct page *page, gfp_t gfp_flags);
+extern void netfs_removing_folio(struct address_space *mapping, struct folio *folio);
extern void netfs_subreq_terminated(struct netfs_io_subrequest *, ssize_t, bool);
extern void netfs_get_subrequest(struct netfs_io_subrequest *subreq,
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 993994c..976b561 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -199,6 +199,7 @@
/* writeback related tags are not used */
AS_NO_WRITEBACK_TAGS = 5,
AS_LARGE_FOLIO_SUPPORT = 6,
+ AS_NOTIFY_REMOVING_FOLIO, /* Call aops->removing_folio() */
};
/**
diff --git a/mm/filemap.c b/mm/filemap.c
index 9a1eef6..d836f34 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -210,6 +210,20 @@
}
/*
+ * Note that a page is about to be removed from the pagecache. If a page that
+ * is about to be removed had been copied to the cache, then in future fscache
+ * won't be able to skip checking in the cache. We do, however, need to do the
+ * notification before the removal lest we race with the page being brought
+ * back again.
+ */
+static void fscache_notify_removing_page(struct address_space *mapping,
+ struct folio *folio)
+{
+ if (unlikely(test_bit(AS_NOTIFY_REMOVING_FOLIO, &mapping->flags)))
+ mapping->a_ops->removing_folio(mapping, folio);
+}
+
+/*
* Delete a page from the page cache and free it. Caller has to make
* sure the page is locked and that nobody else uses it - or that usage
* is safe. The caller must hold the i_pages lock.
@@ -219,6 +233,7 @@
struct address_space *mapping = folio->mapping;
trace_mm_filemap_delete_from_page_cache(folio);
+ fscache_notify_removing_page(mapping, folio);
filemap_unaccount_folio(mapping, folio);
page_cache_delete(mapping, folio, shadow);
}
