From: Jeff Layton <jlayton@kernel.org>
To: Christoph Hellwig <hch@lst.de>, Trond Myklebust <trondmy@kernel.org>
Cc: Anna Schumaker <anna@kernel.org>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 3/4] NFS: track active delegations per-server
Date: Mon, 14 Jul 2025 09:07:30 -0400 [thread overview]
Message-ID: <6db09cc485ae37a4c8c89b4a9a2bcf0dd1302733.camel@kernel.org> (raw)
In-Reply-To: <20250714111651.1565055-4-hch@lst.de>
On Mon, 2025-07-14 at 13:16 +0200, Christoph Hellwig wrote:
> The active delegation watermark was added to avoid overloading servers.
> Track the active delegation per-server instead of globally so that clients
> talking to multiple servers aren't limited by the global limit.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/nfs/client.c | 1 +
> fs/nfs/delegation.c | 35 +++++++++++++++++++----------------
> include/linux/nfs_fs_sb.h | 1 +
> 3 files changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 1a55debab6e5..f55188928f67 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -1017,6 +1017,7 @@ struct nfs_server *nfs_alloc_server(void)
> INIT_LIST_HEAD(&server->ss_src_copies);
>
> atomic_set(&server->active, 0);
> + atomic_long_set(&server->nr_active_delegations, 0);
>
> server->io_stats = nfs_alloc_iostats();
> if (!server->io_stats) {
> diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
> index d036796dbe69..621b635d1c56 100644
> --- a/fs/nfs/delegation.c
> +++ b/fs/nfs/delegation.c
> @@ -27,7 +27,6 @@
>
> #define NFS_DEFAULT_DELEGATION_WATERMARK (15000U)
>
> -static atomic_long_t nfs_active_delegations;
> static unsigned nfs_delegation_watermark = NFS_DEFAULT_DELEGATION_WATERMARK;
> module_param_named(delegation_watermark, nfs_delegation_watermark, uint, 0644);
>
> @@ -38,11 +37,12 @@ static void __nfs_free_delegation(struct nfs_delegation *delegation)
> kfree_rcu(delegation, rcu);
> }
>
> -static void nfs_mark_delegation_revoked(struct nfs_delegation *delegation)
> +static void nfs_mark_delegation_revoked(struct nfs_server *server,
> + struct nfs_delegation *delegation)
> {
> if (!test_and_set_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) {
> delegation->stateid.type = NFS4_INVALID_STATEID_TYPE;
> - atomic_long_dec(&nfs_active_delegations);
> + atomic_long_dec(&server->nr_active_delegations);
> if (!test_bit(NFS_DELEGATION_RETURNING, &delegation->flags))
> nfs_clear_verifier_delegated(delegation->inode);
> }
> @@ -60,9 +60,10 @@ static void nfs_put_delegation(struct nfs_delegation *delegation)
> __nfs_free_delegation(delegation);
> }
>
> -static void nfs_free_delegation(struct nfs_delegation *delegation)
> +static void nfs_free_delegation(struct nfs_server *server,
> + struct nfs_delegation *delegation)
> {
> - nfs_mark_delegation_revoked(delegation);
> + nfs_mark_delegation_revoked(server, delegation);
> nfs_put_delegation(delegation);
> }
>
> @@ -261,7 +262,7 @@ void nfs_inode_reclaim_delegation(struct inode *inode, const struct cred *cred,
> }
> clear_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags);
> if (test_and_clear_bit(NFS_DELEGATION_REVOKED, &delegation->flags))
> - atomic_long_inc(&nfs_active_delegations);
> + atomic_long_inc(&NFS_SERVER(inode)->nr_active_delegations);
> spin_unlock(&delegation->lock);
> rcu_read_unlock();
> put_cred(oldcred);
> @@ -413,7 +414,8 @@ nfs_update_delegation_cred(struct nfs_delegation *delegation,
> }
>
> static void
> -nfs_update_inplace_delegation(struct nfs_delegation *delegation,
> +nfs_update_inplace_delegation(struct nfs_server *server,
> + struct nfs_delegation *delegation,
> const struct nfs_delegation *update)
> {
> if (nfs4_stateid_is_newer(&update->stateid, &delegation->stateid)) {
> @@ -426,7 +428,7 @@ nfs_update_inplace_delegation(struct nfs_delegation *delegation,
> nfs_update_delegation_cred(delegation, update->cred);
> /* smp_mb__before_atomic() is implicit due to xchg() */
> clear_bit(NFS_DELEGATION_REVOKED, &delegation->flags);
> - atomic_long_inc(&nfs_active_delegations);
> + atomic_long_inc(&server->nr_active_delegations);
> }
> }
> }
> @@ -481,7 +483,7 @@ int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred,
> if (nfs4_stateid_match_other(&old_delegation->stateid,
> &delegation->stateid)) {
> spin_lock(&old_delegation->lock);
> - nfs_update_inplace_delegation(old_delegation,
> + nfs_update_inplace_delegation(server, old_delegation,
> delegation);
> spin_unlock(&old_delegation->lock);
> goto out;
> @@ -530,7 +532,7 @@ int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred,
> rcu_assign_pointer(nfsi->delegation, delegation);
> delegation = NULL;
>
> - atomic_long_inc(&nfs_active_delegations);
> + atomic_long_inc(&server->nr_active_delegations);
>
> trace_nfs4_set_delegation(inode, type);
>
> @@ -544,7 +546,7 @@ int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred,
> __nfs_free_delegation(delegation);
> if (freeme != NULL) {
> nfs_do_return_delegation(inode, freeme, 0);
> - nfs_free_delegation(freeme);
> + nfs_free_delegation(server, freeme);
> }
> return status;
> }
> @@ -756,7 +758,7 @@ void nfs_inode_evict_delegation(struct inode *inode)
> set_bit(NFS_DELEGATION_RETURNING, &delegation->flags);
> set_bit(NFS_DELEGATION_INODE_FREEING, &delegation->flags);
> nfs_do_return_delegation(inode, delegation, 1);
> - nfs_free_delegation(delegation);
> + nfs_free_delegation(NFS_SERVER(inode), delegation);
> }
> }
>
> @@ -842,7 +844,8 @@ void nfs4_inode_return_delegation_on_close(struct inode *inode)
> if (!delegation)
> goto out;
> if (test_bit(NFS_DELEGATION_RETURN_IF_CLOSED, &delegation->flags) ||
> - atomic_long_read(&nfs_active_delegations) >= nfs_delegation_watermark) {
> + atomic_long_read(&NFS_SERVER(inode)->nr_active_delegations) >=
> + nfs_delegation_watermark) {
> spin_lock(&delegation->lock);
> if (delegation->inode &&
> list_empty(&NFS_I(inode)->open_files) &&
> @@ -1018,7 +1021,7 @@ static void nfs_revoke_delegation(struct inode *inode,
> }
> spin_unlock(&delegation->lock);
> }
> - nfs_mark_delegation_revoked(delegation);
> + nfs_mark_delegation_revoked(NFS_SERVER(inode), delegation);
> ret = true;
> out:
> rcu_read_unlock();
> @@ -1050,7 +1053,7 @@ void nfs_delegation_mark_returned(struct inode *inode,
> delegation->stateid.seqid = stateid->seqid;
> }
>
> - nfs_mark_delegation_revoked(delegation);
> + nfs_mark_delegation_revoked(NFS_SERVER(inode), delegation);
> clear_bit(NFS_DELEGATION_RETURNING, &delegation->flags);
> spin_unlock(&delegation->lock);
> if (nfs_detach_delegation(NFS_I(inode), delegation, NFS_SERVER(inode)))
> @@ -1272,7 +1275,7 @@ static int nfs_server_reap_unclaimed_delegations(struct nfs_server *server,
> if (delegation != NULL) {
> if (nfs_detach_delegation(NFS_I(inode), delegation,
> server) != NULL)
> - nfs_free_delegation(delegation);
> + nfs_free_delegation(server, delegation);
> /* Match nfs_start_delegation_return_locked */
> nfs_put_delegation(delegation);
> }
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 73bed04529a7..fe930d685780 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -255,6 +255,7 @@ struct nfs_server {
> struct list_head state_owners_lru;
> struct list_head layouts;
> struct list_head delegations;
> + atomic_long_t nr_active_delegations;
> struct list_head ss_copies;
> struct list_head ss_src_copies;
>
Good idea. I like this.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
next prev parent reply other threads:[~2025-07-14 13:07 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-14 11:16 use a hash for looking up delegation Christoph Hellwig
2025-07-14 11:16 ` [PATCH 1/4] NFS: cleanup nfs_inode_reclaim_delegation Christoph Hellwig
2025-07-14 13:06 ` Jeff Layton
2025-07-14 11:16 ` [PATCH 2/4] NFS: move the delegation_watermark module parameter Christoph Hellwig
2025-07-14 13:06 ` Jeff Layton
2025-07-15 8:03 ` Christoph Hellwig
2025-07-15 11:15 ` Jeff Layton
2025-07-14 11:16 ` [PATCH 3/4] NFS: track active delegations per-server Christoph Hellwig
2025-07-14 13:07 ` Jeff Layton [this message]
2025-07-14 11:16 ` [PATCH 4/4] NFS: use a hash table for delegation lookup Christoph Hellwig
2025-07-14 13:14 ` Jeff Layton
2025-07-14 13:18 ` Christoph Hellwig
2025-07-15 8:58 ` Christoph Hellwig
2025-07-15 11:19 ` Jeff Layton
-- strict thread matches above, loose matches on Subject: below --
2025-07-16 13:26 use a hash for looking up delegation v2 Christoph Hellwig
2025-07-16 13:26 ` [PATCH 3/4] NFS: track active delegations per-server Christoph Hellwig
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=6db09cc485ae37a4c8c89b4a9a2bcf0dd1302733.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=anna@kernel.org \
--cc=hch@lst.de \
--cc=linux-nfs@vger.kernel.org \
--cc=trondmy@kernel.org \
/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