public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: Chuck Lever <chuck.lever@oracle.com>, Jeff Layton <jlayton@kernel.org>
Cc: linux-nfs@vger.kernel.org,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>
Subject: [PATCH intro] nfsd: add scheduling point in nfsd_file_gc()
Date: Mon,  6 Jan 2025 10:11:58 +1100	[thread overview]
Message-ID: <20250105234022.2361576-1-neilb@suse.de> (raw)

This patch is intended to fix the problem:
    However, I've seen the laundrette running for multiple milliseconds
    on some workloads, delaying other work. 

reported by Chuck in 
   [PATCH v1 2/2] NFSD: Change the filecache laundrette workqueue again

I believe this problem is mostly likely caused by a lack of scheduling
points in nfsd_file_gc() so this patches adds them as needed.

On reflecting, I think that the approach here is wrong but that would
need a bigger fix.

We generally expect a given file to be used repeatedly for a while, and
then for accesses to stop when the client closes the file.  The
nfsd_file_gc() call happens every two seconds with the aim of discarding
all the files that haven't been used since two seconds ago.

To do this, it scans all the files currently on the list - many of which
are likely to have been used recently.  This seems like a waste of
effort.

I think it would be better to have two lists. A and B.
When refcount reaches zero for a GC file, it is added to A.
When the refcount is incremented we removed from wherever it is.
Every 2 seconds we free everything on B and then splice A across to B.

This completely avoids walking through all the still-active files,
moving them to the end of the LRU.

However this would make the shrinker a little more complex as we
wouldn't be able to use list_lru.  So I'm not proposing that immediately
but would like to know what others think first.

Thanks,
NeilBrown




 [PATCH] nfsd: add scheduling point in nfsd_file_gc()

             reply	other threads:[~2025-01-05 23:40 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-05 23:11 NeilBrown [this message]
2025-01-05 23:11 ` [PATCH] nfsd: add scheduling point in nfsd_file_gc() NeilBrown
2025-01-06  1:55   ` Chuck Lever
2025-01-06  3:02     ` NeilBrown
2025-01-06 13:57       ` Chuck Lever
2025-01-07 23:01         ` NeilBrown
2025-01-08 13:39           ` Chuck Lever
2025-01-08 20:41             ` NeilBrown
2025-01-08 22:27               ` 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=20250105234022.2361576-1-neilb@suse.de \
    --to=neilb@suse.de \
    --cc=Dai.Ngo@oracle.com \
    --cc=chuck.lever@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --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