Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\))
Subject: Re: [PATCH 3/3] SUNRPC: Allow dynamic allocation of back channel
 slots
From:   Chuck Lever <chuck.lever@oracle.com>
In-Reply-To: <20190304001928.4397-3-trond.myklebust@hammerspace.com>
Date:   Mon, 4 Mar 2019 10:26:20 -0500
Cc:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <D12A8479-F573-4B7C-84D0-EC0FE0F87520@oracle.com>
References: <20190304001928.4397-1-trond.myklebust@hammerspace.com>
 <20190304001928.4397-2-trond.myklebust@hammerspace.com>
 <20190304001928.4397-3-trond.myklebust@hammerspace.com>
To:     Trond Myklebust <trondmy@gmail.com>
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

Hi Trond-


> On Mar 3, 2019, at 7:19 PM, Trond Myklebust <trondmy@gmail.com> wrote:
>=20
> Now that the reads happen in a process context rather than a softirq,
> it is safe to allocate back channel slots using a reclaiming
> allocation.

Is this a required change for transports, or simply an optimization?
Wondering if a similar change is needed for RPC-over-RDMA.


> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> net/sunrpc/backchannel_rqst.c | 41 +++++++++++++++++++++--------------
> 1 file changed, 25 insertions(+), 16 deletions(-)
>=20
> diff --git a/net/sunrpc/backchannel_rqst.c =
b/net/sunrpc/backchannel_rqst.c
> index b9313c15ee3a..c47d82622fd1 100644
> --- a/net/sunrpc/backchannel_rqst.c
> +++ b/net/sunrpc/backchannel_rqst.c
> @@ -235,7 +235,8 @@ void xprt_destroy_bc(struct rpc_xprt *xprt, =
unsigned int max_reqs)
> 		list_empty(&xprt->bc_pa_list) ? "true" : "false");
> }
>=20
> -static struct rpc_rqst *xprt_alloc_bc_request(struct rpc_xprt *xprt, =
__be32 xid)
> +static struct rpc_rqst *xprt_get_bc_request(struct rpc_xprt *xprt, =
__be32 xid,
> +		struct rpc_rqst *new)
> {
> 	struct rpc_rqst *req =3D NULL;
>=20
> @@ -243,10 +244,9 @@ static struct rpc_rqst =
*xprt_alloc_bc_request(struct rpc_xprt *xprt, __be32 xid)
> 	if (atomic_read(&xprt->bc_free_slots) <=3D 0)
> 		goto not_found;
> 	if (list_empty(&xprt->bc_pa_list)) {
> -		req =3D xprt_alloc_bc_req(xprt, GFP_ATOMIC);
> -		if (!req)
> +		if (!new)
> 			goto not_found;
> -		list_add_tail(&req->rq_bc_pa_list, &xprt->bc_pa_list);
> +		list_add_tail(&new->rq_bc_pa_list, &xprt->bc_pa_list);
> 		xprt->bc_alloc_count++;
> 	}
> 	req =3D list_first_entry(&xprt->bc_pa_list, struct rpc_rqst,
> @@ -256,8 +256,8 @@ static struct rpc_rqst =
*xprt_alloc_bc_request(struct rpc_xprt *xprt, __be32 xid)
> 			sizeof(req->rq_private_buf));
> 	req->rq_xid =3D xid;
> 	req->rq_connect_cookie =3D xprt->connect_cookie;
> -not_found:
> 	dprintk("RPC:       backchannel req=3D%p\n", req);
> +not_found:
> 	return req;
> }
>=20
> @@ -320,18 +320,27 @@ void xprt_free_bc_rqst(struct rpc_rqst *req)
>  */
> struct rpc_rqst *xprt_lookup_bc_request(struct rpc_xprt *xprt, __be32 =
xid)
> {
> -	struct rpc_rqst *req;
> -
> -	spin_lock(&xprt->bc_pa_lock);
> -	list_for_each_entry(req, &xprt->bc_pa_list, rq_bc_pa_list) {
> -		if (req->rq_connect_cookie !=3D xprt->connect_cookie)
> -			continue;
> -		if (req->rq_xid =3D=3D xid)
> -			goto found;
> -	}
> -	req =3D xprt_alloc_bc_request(xprt, xid);
> +	struct rpc_rqst *req, *new =3D NULL;
> +
> +	do {
> +		spin_lock(&xprt->bc_pa_lock);
> +		list_for_each_entry(req, &xprt->bc_pa_list, =
rq_bc_pa_list) {
> +			if (req->rq_connect_cookie !=3D =
xprt->connect_cookie)
> +				continue;
> +			if (req->rq_xid =3D=3D xid)
> +				goto found;
> +		}
> +		req =3D xprt_get_bc_request(xprt, xid, new);
> found:
> -	spin_unlock(&xprt->bc_pa_lock);
> +		spin_unlock(&xprt->bc_pa_lock);
> +		if (new) {
> +			if (req !=3D new)
> +				xprt_free_bc_rqst(new);
> +			break;
> +		} else if (req)
> +			break;
> +		new =3D xprt_alloc_bc_req(xprt, GFP_KERNEL);
> +	} while (new);
> 	return req;
> }
>=20
> --=20
> 2.20.1
>=20

--
Chuck Lever