Message-ID: <0d66402eb20f5f6aa74572d2e15239aa1be04bef.camel@gmail.com>
Subject: Re: [PATCH v1 05/19] xprtrdma: Clean up rpcrdma_create_rep() and
 rpcrdma_destroy_rep()
From:   Anna Schumaker <schumaker.anna@gmail.com>
To:     Chuck Lever <chuck.lever@oracle.com>, linux-rdma@vger.kernel.org,
        linux-nfs@vger.kernel.org
Date:   Thu, 11 Apr 2019 16:47:24 -0400
In-Reply-To: <20190410200658.11522.59896.stgit@manet.1015granger.net>
References: <20190410200446.11522.21145.stgit@manet.1015granger.net>
         <20190410200658.11522.59896.stgit@manet.1015granger.net>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.32.1 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

Hi Chuck,

On Wed, 2019-04-10 at 16:06 -0400, Chuck Lever wrote:
> For code legibility, clean up the function names to be consistent
> with the pattern: "rpcrdma" _ object-type _ action
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  net/sunrpc/xprtrdma/verbs.c |   21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index 82ea298..3bc751e 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -76,7 +76,6 @@
>  static void rpcrdma_sendctx_put_locked(struct rpcrdma_sendctx *sc);
>  static void rpcrdma_mrs_create(struct rpcrdma_xprt *r_xprt);
>  static void rpcrdma_mrs_destroy(struct rpcrdma_buffer *buf);
> -static int rpcrdma_create_rep(struct rpcrdma_xprt *r_xprt, bool temp);
>  static void rpcrdma_dma_unmap_regbuf(struct rpcrdma_regbuf *rb);
>  static void rpcrdma_post_recvs(struct rpcrdma_xprt *r_xprt, bool temp);
>  
> @@ -1029,15 +1028,12 @@ struct rpcrdma_req *rpcrdma_req_create(struct
> rpcrdma_xprt *r_xprt, gfp_t flags)
>  	return req;
>  }
>  
> -static int
> -rpcrdma_create_rep(struct rpcrdma_xprt *r_xprt, bool temp)
> +static bool rpcrdma_rep_create(struct rpcrdma_xprt *r_xprt, bool temp)
>  {
>  	struct rpcrdma_create_data_internal *cdata = &r_xprt->rx_data;
>  	struct rpcrdma_buffer *buf = &r_xprt->rx_buf;
>  	struct rpcrdma_rep *rep;
> -	int rc;
>  
> -	rc = -ENOMEM;

rc is used again a little lower, in the IS_ERR(rep->rr_rdmabuf) check. Can you
remove it there too, please?

Thanks,
Anna

>  	rep = kzalloc(sizeof(*rep), GFP_KERNEL);
>  	if (rep == NULL)
>  		goto out;
> @@ -1063,12 +1059,12 @@ struct rpcrdma_req *rpcrdma_req_create(struct
> rpcrdma_xprt *r_xprt, gfp_t flags)
>  	spin_lock(&buf->rb_lock);
>  	list_add(&rep->rr_list, &buf->rb_recv_bufs);
>  	spin_unlock(&buf->rb_lock);
> -	return 0;
> +	return true;
>  
>  out_free:
>  	kfree(rep);
>  out:
> -	return rc;
> +	return false;
>  }
>  
>  int
> @@ -1124,8 +1120,7 @@ struct rpcrdma_req *rpcrdma_req_create(struct
> rpcrdma_xprt *r_xprt, gfp_t flags)
>  	return rc;
>  }
>  
> -static void
> -rpcrdma_destroy_rep(struct rpcrdma_rep *rep)
> +static void rpcrdma_rep_destroy(struct rpcrdma_rep *rep)
>  {
>  	rpcrdma_free_regbuf(rep->rr_rdmabuf);
>  	kfree(rep);
> @@ -1205,7 +1200,7 @@ struct rpcrdma_req *rpcrdma_req_create(struct
> rpcrdma_xprt *r_xprt, gfp_t flags)
>  		rep = list_first_entry(&buf->rb_recv_bufs,
>  				       struct rpcrdma_rep, rr_list);
>  		list_del(&rep->rr_list);
> -		rpcrdma_destroy_rep(rep);
> +		rpcrdma_rep_destroy(rep);
>  	}
>  
>  	while (!list_empty(&buf->rb_send_bufs)) {
> @@ -1334,7 +1329,7 @@ struct rpcrdma_req *
>  	}
>  	spin_unlock(&buffers->rb_lock);
>  	if (rep)
> -		rpcrdma_destroy_rep(rep);
> +		rpcrdma_rep_destroy(rep);
>  }
>  
>  /*
> @@ -1351,7 +1346,7 @@ struct rpcrdma_req *
>  		list_add(&rep->rr_list, &buffers->rb_recv_bufs);
>  		spin_unlock(&buffers->rb_lock);
>  	} else {
> -		rpcrdma_destroy_rep(rep);
> +		rpcrdma_rep_destroy(rep);
>  	}
>  }
>  
> @@ -1500,7 +1495,7 @@ struct rpcrdma_regbuf *
>  			list_del(&rep->rr_list);
>  		spin_unlock(&buf->rb_lock);
>  		if (!rep) {
> -			if (rpcrdma_create_rep(r_xprt, temp))
> +			if (!rpcrdma_rep_create(r_xprt, temp))
>  				break;
>  			continue;
>  		}
>