From: cel@kernel.org
To: Neil Brown <neilb@suse.de>, Jeff Layton <jlayton@kernel.org>,
Olga Kornievskaia <okorniev@redhat.com>,
Dai Ngo <dai.ngo@oracle.com>, Tom Talpey <tom@talpey.com>
Cc: <linux-nfs@vger.kernel.org>, Dave Chinner <david@fromorbit.com>
Subject: [PATCH v2 5/7] nfsd: filecache: introduce NFSD_FILE_RECENT
Date: Tue, 18 Feb 2025 10:39:35 -0500 [thread overview]
Message-ID: <20250218153937.6125-6-cel@kernel.org> (raw)
In-Reply-To: <20250218153937.6125-1-cel@kernel.org>
From: NeilBrown <neilb@suse.de>
The filecache lru is walked in 2 circumstances for 2 different reasons.
1/ When called from the shrinker we want to discard the first few
entries on the list, ignoring any with NFSD_FILE_REFERENCED set
because they should really be at the end of the LRU as they have been
referenced recently. So those ones are ROTATED.
2/ When called from the nfsd_file_gc() timer function we want to discard
anything that hasn't been used since before the previous call, and
mark everything else as unused at this point in time.
Using the same flag for both of these can result in some unexpected
outcomes. If the shrinker callback clears NFSD_FILE_REFERENCED then
nfsd_file_gc() will think the file hasn't been used in a while, while
really it has.
I think it is easier to reason about the behaviour if we instead have
two flags.
NFSD_FILE_REFERENCED means "this should be at the end of the LRU, please
put it there when convenient"
NFSD_FILE_RECENT means "this has been used recently - since the last
run of nfsd_file_gc()
When either caller finds an NFSD_FILE_REFERENCED entry, that entry
should be moved to the end of the LRU and the flag cleared. This can
safely happen at any time. The actual order on the lru might not be
strictly least-recently-used, but that is normal for linux lrus.
The shrinker callback can ignore the "recent" flag. If it ends up
freeing something that is "recent" that simply means that memory
pressure is sufficient to limit the acceptable cache age to less than
the nfsd_file_gc frequency.
The gc callback should primarily focus on NFSD_FILE_RECENT. It should
free everything that doesn't have this flag set, and should clear the
flag on everything else. When it clears the flag it is convenient to
clear the "REFERENCED" flag and move to the end of the LRU too.
With this, calls from the shrinker do not prematurely age files. It
will focus only on freeing those that are least recently used.
Signed-off-by: NeilBrown <neilb@suse.de>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
fs/nfsd/filecache.c | 22 ++++++++++++++++++++--
fs/nfsd/filecache.h | 1 +
fs/nfsd/trace.h | 3 +++
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
index 747929c8c0d5..0d621833a9f2 100644
--- a/fs/nfsd/filecache.c
+++ b/fs/nfsd/filecache.c
@@ -319,10 +319,10 @@ nfsd_file_check_writeback(struct nfsd_file *nf)
mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK);
}
-
static bool nfsd_file_lru_add(struct nfsd_file *nf)
{
set_bit(NFSD_FILE_REFERENCED, &nf->nf_flags);
+ set_bit(NFSD_FILE_RECENT, &nf->nf_flags);
if (list_lru_add_obj(&nfsd_file_lru, &nf->nf_lru)) {
trace_nfsd_file_lru_add(nf);
return true;
@@ -534,6 +534,24 @@ nfsd_file_lru_cb(struct list_head *item, struct list_lru_one *lru,
return LRU_REMOVED;
}
+static enum lru_status
+nfsd_file_gc_cb(struct list_head *item, struct list_lru_one *lru,
+ void *arg)
+{
+ struct nfsd_file *nf = list_entry(item, struct nfsd_file, nf_lru);
+
+ if (test_and_clear_bit(NFSD_FILE_RECENT, &nf->nf_flags)) {
+ /*
+ * "REFERENCED" really means "should be at the end of the
+ * LRU. As we are putting it there we can clear the flag.
+ */
+ clear_bit(NFSD_FILE_REFERENCED, &nf->nf_flags);
+ trace_nfsd_file_gc_aged(nf);
+ return LRU_ROTATE;
+ }
+ return nfsd_file_lru_cb(item, lru, arg);
+}
+
static void
nfsd_file_gc(void)
{
@@ -544,7 +562,7 @@ nfsd_file_gc(void)
for_each_node_state(nid, N_NORMAL_MEMORY) {
unsigned long nr = list_lru_count_node(&nfsd_file_lru, nid);
- ret += list_lru_walk_node(&nfsd_file_lru, nid, nfsd_file_lru_cb,
+ ret += list_lru_walk_node(&nfsd_file_lru, nid, nfsd_file_gc_cb,
&dispose, &nr);
}
trace_nfsd_file_gc_removed(ret, list_lru_count(&nfsd_file_lru));
diff --git a/fs/nfsd/filecache.h b/fs/nfsd/filecache.h
index d5db6b34ba30..de5b8aa7fcb0 100644
--- a/fs/nfsd/filecache.h
+++ b/fs/nfsd/filecache.h
@@ -38,6 +38,7 @@ struct nfsd_file {
#define NFSD_FILE_PENDING (1)
#define NFSD_FILE_REFERENCED (2)
#define NFSD_FILE_GC (3)
+#define NFSD_FILE_RECENT (4)
unsigned long nf_flags;
refcount_t nf_ref;
unsigned char nf_may;
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index 49bbd26ffcdb..d93573504fa4 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -1050,6 +1050,7 @@ DEFINE_CLID_EVENT(confirmed_r);
{ 1 << NFSD_FILE_HASHED, "HASHED" }, \
{ 1 << NFSD_FILE_PENDING, "PENDING" }, \
{ 1 << NFSD_FILE_REFERENCED, "REFERENCED" }, \
+ { 1 << NFSD_FILE_RECENT, "RECENT" }, \
{ 1 << NFSD_FILE_GC, "GC" })
DECLARE_EVENT_CLASS(nfsd_file_class,
@@ -1328,6 +1329,7 @@ DEFINE_NFSD_FILE_GC_EVENT(nfsd_file_lru_del_disposed);
DEFINE_NFSD_FILE_GC_EVENT(nfsd_file_gc_in_use);
DEFINE_NFSD_FILE_GC_EVENT(nfsd_file_gc_writeback);
DEFINE_NFSD_FILE_GC_EVENT(nfsd_file_gc_referenced);
+DEFINE_NFSD_FILE_GC_EVENT(nfsd_file_gc_aged);
DEFINE_NFSD_FILE_GC_EVENT(nfsd_file_gc_disposed);
DECLARE_EVENT_CLASS(nfsd_file_lruwalk_class,
@@ -1357,6 +1359,7 @@ DEFINE_EVENT(nfsd_file_lruwalk_class, name, \
TP_ARGS(removed, remaining))
DEFINE_NFSD_FILE_LRUWALK_EVENT(nfsd_file_gc_removed);
+DEFINE_NFSD_FILE_LRUWALK_EVENT(nfsd_file_gc_recent);
DEFINE_NFSD_FILE_LRUWALK_EVENT(nfsd_file_shrinker_removed);
TRACE_EVENT(nfsd_file_close,
--
2.47.0
next prev parent reply other threads:[~2025-02-18 15:39 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-18 15:39 [PATCH v2 0/7] nfsd: filecache: various fixes cel
2025-02-18 15:39 ` [PATCH v2 1/7] nfsd: filecache: remove race handling cel
2025-02-18 15:39 ` [PATCH v2 2/7] NFSD: Re-organize nfsd_file_gc_worker() cel
2025-02-18 19:59 ` Jeff Layton
2025-02-19 0:33 ` Dave Chinner
2025-02-19 1:20 ` NeilBrown
2025-02-19 14:01 ` Chuck Lever
2025-02-18 15:39 ` [PATCH v2 3/7] nfsd: filecache: use nfsd_file_dispose_list() in nfsd_file_close_inode_sync() cel
2025-02-18 15:39 ` [PATCH v2 4/7] nfsd: filecache: use list_lru_walk_node() in nfsd_file_gc() cel
2025-02-18 15:39 ` cel [this message]
2025-02-18 15:39 ` [PATCH v2 6/7] nfsd: filecache: don't repeatedly add/remove files on the lru list cel
2025-02-18 20:27 ` Jeff Layton
2025-02-18 15:39 ` [PATCH v2 7/7] nfsd: filecache: drop the list_lru lock during lock gc scans cel
2025-02-18 20:51 ` Jeff Layton
2025-02-20 18:22 ` [PATCH v2 0/7] nfsd: filecache: various fixes Chuck Lever
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250218153937.6125-6-cel@kernel.org \
--to=cel@kernel.org \
--cc=dai.ngo@oracle.com \
--cc=david@fromorbit.com \
--cc=jlayton@kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=neilb@suse.de \
--cc=okorniev@redhat.com \
--cc=tom@talpey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox