Re: [PATCH 1/3] SUNRPC: Reduce contention in svc_xprt_enqueue()

From: Bruce Fields
Date: Tue Jul 29 2014 - 15:31:42 EST


All three patches look good to me, thanks.

>From private email, this:

On Thu, Jul 24, 2014 at 11:59:31PM -0400, Trond Myklebust wrote:
> @@ -222,11 +223,12 @@ static void svc_xprt_received(struct svc_xprt *xprt)
> if (!test_bit(XPT_BUSY, &xprt->xpt_flags))
> return;
> /* As soon as we clear busy, the xprt could be closed and
> - * 'put', so we need a reference to call svc_xprt_enqueue with:
> + * 'put', so we need a reference to call svc_xprt_do_enqueue with:
> */
> svc_xprt_get(xprt);
> + smp_mb__before_clear_bit();

triggered a warning about smp_mb__before_clear_bit noticed by the kbuild
robot. Looks like that was due to
febdbfe8a91ce0d11939d4940b592eb0dba8d663 "arch: Prepare for
smp_mb__{before,after}_atomic()".

You questioned whether deprecating smp_mb__{before,after}_clear_bit was
an unnecessary burden on people maintaining stable kernels or doing
backports more generally. Cc'ing some addresses from that commit.

Whatever--I'll probably just replace do the clear_bit->before_atomic
replacement and apply unless there's some objection.

--b.

> clear_bit(XPT_BUSY, &xprt->xpt_flags);
> - svc_xprt_enqueue(xprt);
> + svc_xprt_do_enqueue(xprt);
> svc_xprt_put(xprt);
> }
>
> @@ -335,12 +337,7 @@ static bool svc_xprt_has_something_to_do(struct svc_xprt *xprt)
> return false;
> }
>
> -/*
> - * Queue up a transport with data pending. If there are idle nfsd
> - * processes, wake 'em up.
> - *
> - */
> -void svc_xprt_enqueue(struct svc_xprt *xprt)
> +static void svc_xprt_do_enqueue(struct svc_xprt *xprt)
> {
> struct svc_pool *pool;
> struct svc_rqst *rqstp;
> @@ -398,6 +395,18 @@ void svc_xprt_enqueue(struct svc_xprt *xprt)
> out_unlock:
> spin_unlock_bh(&pool->sp_lock);
> }
> +
> +/*
> + * Queue up a transport with data pending. If there are idle nfsd
> + * processes, wake 'em up.
> + *
> + */
> +void svc_xprt_enqueue(struct svc_xprt *xprt)
> +{
> + if (test_bit(XPT_BUSY, &xprt->xpt_flags))
> + return;
> + svc_xprt_do_enqueue(xprt);
> +}
> EXPORT_SYMBOL_GPL(svc_xprt_enqueue);
>
> /*
> --
> 1.9.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/