Re: svc_xprt_put is no longer BH-safe

["J. Bruce Fields" <bfields@fieldses.org>]

On Sat, Oct 29, 2016 at 12:21:03PM -0400, Chuck Lever wrote:
> Hi Bruce-

Sorry for the slow response!

...
> In commit 39a9beab5acb83176e8b9a4f0778749a09341f1f ('rpc: share one xps between
> all backchannels') you added:
> 
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index f5572e3..4f01f63 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -136,6 +136,8 @@ static void svc_xprt_free(struct kref *kref)
>         /* See comment on corresponding get in xs_setup_bc_tcp(): */
>         if (xprt->xpt_bc_xprt)
>                 xprt_put(xprt->xpt_bc_xprt);
> +       if (xprt->xpt_bc_xps)
> +               xprt_switch_put(xprt->xpt_bc_xps);
>         xprt->xpt_ops->xpo_free(xprt);
>         module_put(owner);
>  }
> 
> svc_xprt_free() is invoked by svc_xprt_put(). svc_xprt_put() is called
> from svc_rdma in soft IRQ context (eg. svc_rdma_wc_receive).

Is that necessary?  I wonder why the svcrdma code seems to be taking so
many of its own references on svc_xprts.

> However, xprt_switch_put() takes a spin lock (xps_lock) which is locked
> everywhere without disabling BHs.
> 
> It looks to me like 39a9beab5acb makes svc_xprt_put() no longer BH-safe?
> Not sure if svc_xprt_put() was intended to be BH-safe beforehand.
> 
> Maybe xprt_switch_put() could be invoked in ->xpo_free, but that seems
> like a temporary solution.

Since xpo_free is also called from svc_xprt_put that doesn't sound like
it would change anything.  Or do we not trunk over RDMA for some reason?

--b.

> 
> 
> --
> Chuck Lever
> 
>