Linux NFS development
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Josef Bacik <josef@toxicpanda.com>, Chuck Lever <chuck.lever@oracle.com>
Cc: linux-nfs@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH v2 10/13] nfsd: move th_cnt into nfsd_net
Date: Fri, 26 Jan 2024 08:01:41 -0500	[thread overview]
Message-ID: <889ecfaa124883cd99e40d457562af45b5e97e7d.camel@kernel.org> (raw)
In-Reply-To: <20240125215618.GB1602047@perftesting>

On Thu, 2024-01-25 at 16:56 -0500, Josef Bacik wrote:
> On Thu, Jan 25, 2024 at 04:01:27PM -0500, Chuck Lever wrote:
> > On Thu, Jan 25, 2024 at 02:53:20PM -0500, Josef Bacik wrote:
> > > This is the last global stat, move it into nfsd_net and adjust all the
> > > users to use that variant instead of the global one.
> > 
> > Hm. I thought nfsd threads were a global resource -- they service
> > all network namespaces. So, shouldn't the same thread count be
> > surfaced to all containers? Won't they all see all of the nfsd
> > processes?
> > 
> 

Each container is going to start /proc/fs/nfsd/threads number of threads
regardless. I hadn't actually grokked that they just get tossed onto the
pile of threads that service requests.

Is is possible for one container to start a small number of threads but
have its client load be such that it spills over and ends up stealing
threads from other containers?

> I don't think we want the network namespaces seeing how many threads exist in
> the entire system right?
> 
> Additionally it appears that we can have multiple threads per network namespace,
> so it's not like this will just show 1 for each individual nn, it'll show
> however many threads have been configured for that nfsd in that network
> namespace.
> 
> I'm good either way, but it makes sense to me to only surface the network
> namespace related thread count.  I could probably have a global counter and only
> surface the global counter if net == &init_net.  Let me know what you prefer.
> Thanks,
> 

The old stat meant "number of threads that are currently busy". The new
one will mean "number of threads that are busy servicing requests in
this namespace". The latter seems more useful on a per-ns basis.

In fact, it might be interesting to throw out some kind of warning when
the th_cnt exceeds the number of threads that you've allocated in the
container. That might be an indicator that you didn't spin up enough and
are poaching from other containers.

Or... maybe we should be queueing the RPC when that happens so that you
can't use more than you've allocated for the container?
-- 
Jeff Layton <jlayton@kernel.org>

  reply	other threads:[~2024-01-26 13:01 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-25 19:53 [PATCH v2 00/13] Make nfs and nfsd stats visible in network ns Josef Bacik
2024-01-25 19:53 ` [PATCH v2 01/13] sunrpc: don't change ->sv_stats if it doesn't exist Josef Bacik
2024-01-25 19:53 ` [PATCH v2 02/13] nfs: stop setting ->pg_stats for unused stats Josef Bacik
2024-01-25 19:53 ` [PATCH v2 03/13] sunrpc: pass in the sv_stats struct through svc_create* Josef Bacik
2024-01-25 20:56   ` Chuck Lever
2024-01-25 21:56     ` Josef Bacik
2024-01-25 19:53 ` [PATCH v2 04/13] sunrpc: remove ->pg_stats from svc_program Josef Bacik
2024-01-25 19:53 ` [PATCH v2 05/13] sunrpc: add a struct rpc_stats arg to rpc_create_args Josef Bacik
2024-01-25 20:53   ` Chuck Lever
2024-01-25 21:54     ` Josef Bacik
2024-01-25 22:30       ` Jeff Layton
2024-01-26 13:49       ` Chuck Lever III
2024-01-25 19:53 ` [PATCH v2 06/13] sunrpc: use the struct net as the svc proc private Josef Bacik
2024-01-25 19:53 ` [PATCH v2 07/13] nfsd: rename NFSD_NET_* to NFSD_STATS_* Josef Bacik
2024-01-25 19:53 ` [PATCH v2 08/13] nfsd: expose /proc/net/sunrpc/nfsd in net namespaces Josef Bacik
2024-01-25 19:53 ` [PATCH v2 09/13] nfsd: make all of the nfsd stats per-network namespace Josef Bacik
2024-01-25 19:53 ` [PATCH v2 10/13] nfsd: move th_cnt into nfsd_net Josef Bacik
2024-01-25 21:01   ` Chuck Lever
2024-01-25 21:56     ` Josef Bacik
2024-01-26 13:01       ` Jeff Layton [this message]
2024-01-26 13:48         ` Chuck Lever III
2024-01-26 14:08           ` Jeff Layton
2024-01-26 14:27             ` Chuck Lever III
2024-01-26 15:03               ` Jeff Layton
2024-01-26 15:16                 ` Chuck Lever III
2024-01-26 15:35                   ` Jeff Layton
2024-01-25 19:53 ` [PATCH v2 11/13] nfsd: make svc_stat per-network namespace instead of global Josef Bacik
2024-01-25 19:53 ` [PATCH v2 12/13] nfs: expose /proc/net/sunrpc/nfs in net namespaces Josef Bacik
2024-01-25 19:53 ` [PATCH v2 13/13] nfs: make the rpc_stat per net namespace Josef Bacik
2024-01-26 13:12 ` [PATCH v2 00/13] Make nfs and nfsd stats visible in network ns Jeff Layton

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=889ecfaa124883cd99e40d457562af45b5e97e7d.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=josef@toxicpanda.com \
    --cc=kernel-team@fb.com \
    --cc=linux-nfs@vger.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