Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "Chuck Lever III" <chuck.lever@oracle.com>
Cc:     "Bruce Fields" <bfields@fieldses.org>,
        "Linux NFS Mailing List" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 05/19] SUNRPC: use sv_lock to protect updates to sv_nrthreads.
In-reply-to: <3877F926-545B-41AF-8C0C-80582E83F1FE@oracle.com>
References: <163763078330.7284.10141477742275086758.stgit@noble.brown>,
 <163763097544.7284.15751243743215653521.stgit@noble.brown>,
 <3877F926-545B-41AF-8C0C-80582E83F1FE@oracle.com>
Date:   Mon, 29 Nov 2021 11:11:17 +1100
Message-id: <163814467728.26075.9969415181039584323@noble.neil.brown.name>
Precedence: bulk

On Wed, 24 Nov 2021, Chuck Lever III wrote:
> > @@ -639,7 +639,10 @@ svc_prepare_thread(struct svc_serv *serv, struct svc=
_pool *pool, int node)
> > 		return ERR_PTR(-ENOMEM);
> >=20
> > 	svc_get(serv);
> > -	serv->sv_nrthreads++;
> > +	spin_lock_bh(&serv->sv_lock);
> > +	serv->sv_nrthreads +=3D 1;
> > +	spin_unlock_bh(&serv->sv_lock);
>=20
> atomic_t would be somewhat lighter weight. Can it be used here
> instead?
>=20

We could....  but sv_nrthreads is read-mostly.  There are 11 places
where we would need to call "atomic_read()", and just two where we
benefit from the simplicity of atomic_inc/dec.

And even if I did achieve dynamic threads count management, we would not
be changing sv_nrthreads often enough that any performance difference
would be noticeable.
So I'd rather stick with using the spinlock and keeping the read-side
simple.

Thanks,
NeilBrown