From: "J . Bruce Fields" <bfields@fieldses.org>
To: Trond Myklebust <trondmy@gmail.com>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [PATCH 01/15] SUNRPC: Remove the server 'authtab_lock' and just use RCU
Date: Tue, 2 Oct 2018 15:39:08 -0400 [thread overview]
Message-ID: <20181002193908.GB9073@fieldses.org> (raw)
In-Reply-To: <20181001144157.3515-2-trond.myklebust@hammerspace.com>
On Mon, Oct 01, 2018 at 10:41:43AM -0400, Trond Myklebust wrote:
> Module removal is RCU safe by design, so we really have no need to
> lock the 'authtab[]' array.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> net/sunrpc/svcauth.c | 52 +++++++++++++++++++++++++++++---------------
> 1 file changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/net/sunrpc/svcauth.c b/net/sunrpc/svcauth.c
> index bb8db3cb8032..f83443856cd1 100644
> --- a/net/sunrpc/svcauth.c
> +++ b/net/sunrpc/svcauth.c
> @@ -27,12 +27,32 @@
> extern struct auth_ops svcauth_null;
> extern struct auth_ops svcauth_unix;
>
> -static DEFINE_SPINLOCK(authtab_lock);
> -static struct auth_ops *authtab[RPC_AUTH_MAXFLAVOR] = {
> - [0] = &svcauth_null,
> - [1] = &svcauth_unix,
> +static struct auth_ops __rcu *authtab[RPC_AUTH_MAXFLAVOR] = {
> + [RPC_AUTH_NULL] = (struct auth_ops __force __rcu *)&svcauth_null,
> + [RPC_AUTH_UNIX] = (struct auth_ops __force __rcu *)&svcauth_unix,
I didn't know about "__rcu" and "__force __rcu". Grepping turns up
relevant documentation for RCU_INIT_POINTER in include/linux/rcupdate.h,
OK, I think I get it.--b.
> };
>
> +static struct auth_ops *
> +svc_get_auth_ops(rpc_authflavor_t flavor)
> +{
> + struct auth_ops *aops;
> +
> + if (flavor >= RPC_AUTH_MAXFLAVOR)
> + return NULL;
> + rcu_read_lock();
> + aops = rcu_dereference(authtab[flavor]);
> + if (aops != NULL && !try_module_get(aops->owner))
> + aops = NULL;
> + rcu_read_unlock();
> + return aops;
> +}
> +
> +static void
> +svc_put_auth_ops(struct auth_ops *aops)
> +{
> + module_put(aops->owner);
> +}
> +
> int
> svc_authenticate(struct svc_rqst *rqstp, __be32 *authp)
> {
> @@ -45,14 +65,11 @@ svc_authenticate(struct svc_rqst *rqstp, __be32 *authp)
>
> dprintk("svc: svc_authenticate (%d)\n", flavor);
>
> - spin_lock(&authtab_lock);
> - if (flavor >= RPC_AUTH_MAXFLAVOR || !(aops = authtab[flavor]) ||
> - !try_module_get(aops->owner)) {
> - spin_unlock(&authtab_lock);
> + aops = svc_get_auth_ops(flavor);
> + if (aops == NULL) {
> *authp = rpc_autherr_badcred;
> return SVC_DENIED;
> }
> - spin_unlock(&authtab_lock);
>
> rqstp->rq_auth_slack = 0;
> init_svc_cred(&rqstp->rq_cred);
> @@ -82,7 +99,7 @@ int svc_authorise(struct svc_rqst *rqstp)
>
> if (aops) {
> rv = aops->release(rqstp);
> - module_put(aops->owner);
> + svc_put_auth_ops(aops);
> }
> return rv;
> }
> @@ -90,13 +107,14 @@ int svc_authorise(struct svc_rqst *rqstp)
> int
> svc_auth_register(rpc_authflavor_t flavor, struct auth_ops *aops)
> {
> + struct auth_ops *old;
> int rv = -EINVAL;
> - spin_lock(&authtab_lock);
> - if (flavor < RPC_AUTH_MAXFLAVOR && authtab[flavor] == NULL) {
> - authtab[flavor] = aops;
> - rv = 0;
> +
> + if (flavor < RPC_AUTH_MAXFLAVOR) {
> + old = cmpxchg((struct auth_ops ** __force)&authtab[flavor], NULL, aops);
> + if (old == NULL || old == aops)
> + rv = 0;
> }
> - spin_unlock(&authtab_lock);
> return rv;
> }
> EXPORT_SYMBOL_GPL(svc_auth_register);
> @@ -104,10 +122,8 @@ EXPORT_SYMBOL_GPL(svc_auth_register);
> void
> svc_auth_unregister(rpc_authflavor_t flavor)
> {
> - spin_lock(&authtab_lock);
> if (flavor < RPC_AUTH_MAXFLAVOR)
> - authtab[flavor] = NULL;
> - spin_unlock(&authtab_lock);
> + rcu_assign_pointer(authtab[flavor], NULL);
> }
> EXPORT_SYMBOL_GPL(svc_auth_unregister);
>
> --
> 2.17.1
next prev parent reply other threads:[~2018-10-03 2:24 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-01 14:41 [PATCH 00/15] Performance improvements for knfsd Trond Myklebust
2018-10-01 14:41 ` [PATCH 01/15] SUNRPC: Remove the server 'authtab_lock' and just use RCU Trond Myklebust
2018-10-01 14:41 ` [PATCH 02/15] SUNRPC: Add lockless lookup of the server's auth domain Trond Myklebust
2018-10-01 14:41 ` [PATCH 03/15] SUNRPC: Allow cache lookups to use RCU protection rather than the r/w spinlock Trond Myklebust
2018-10-01 14:41 ` [PATCH 04/15] SUNRPC: Make server side AUTH_UNIX use lockless lookups Trond Myklebust
2018-10-01 14:41 ` [PATCH 05/15] knfsd: Allow lockless lookups of the exports Trond Myklebust
2018-10-01 14:41 ` [PATCH 06/15] SUNRPC: Lockless server RPCSEC_GSS context lookup Trond Myklebust
2018-10-01 14:41 ` [PATCH 07/15] knfsd: Lockless lookup of NFSv4 identities Trond Myklebust
2018-10-01 14:41 ` [PATCH 08/15] NFS: Lockless DNS lookups Trond Myklebust
2018-10-01 14:41 ` [PATCH 09/15] SUNRPC: Remove non-RCU protected lookup Trond Myklebust
2018-10-01 14:41 ` [PATCH 10/15] SUNRPC: Replace the cache_detail->hash_lock with a regular spinlock Trond Myklebust
2018-10-01 14:41 ` [PATCH 11/15] SUNRPC: Simplify TCP receive code Trond Myklebust
2018-10-01 14:41 ` [PATCH 12/15] knfsd: Remove dead code from nfsd_cache_lookup Trond Myklebust
2018-10-01 14:41 ` [PATCH 13/15] knfsd: Simplify NFS duplicate replay cache Trond Myklebust
2018-10-01 14:41 ` [PATCH 14/15] knfsd: Further simplify the cache lookup Trond Myklebust
2018-10-01 14:41 ` [PATCH 15/15] knfsd: Improve lookup performance in the duplicate reply cache using an rbtree Trond Myklebust
2018-10-04 0:44 ` J . Bruce Fields
2018-10-03 17:14 ` [PATCH 13/15] knfsd: Simplify NFS duplicate replay cache J . Bruce Fields
2018-10-03 18:01 ` Trond Myklebust
2018-10-03 18:11 ` bfields
2018-10-03 23:51 ` bfields
2018-10-03 16:10 ` [PATCH 08/15] NFS: Lockless DNS lookups J . Bruce Fields
2018-10-03 17:55 ` Trond Myklebust
2018-10-03 16:08 ` [PATCH 03/15] SUNRPC: Allow cache lookups to use RCU protection rather than the r/w spinlock J . Bruce Fields
2018-10-02 19:39 ` J . Bruce Fields [this message]
2018-10-01 15:29 ` [PATCH 00/15] Performance improvements for knfsd Trond Myklebust
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=20181002193908.GB9073@fieldses.org \
--to=bfields@fieldses.org \
--cc=linux-nfs@vger.kernel.org \
--cc=trondmy@gmail.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;
as well as URLs for NNTP newsgroup(s).