by Anna Schumaker

[permalink] [raw]

Subject: Re: [PATCH v1 05/19] xprtrdma: Clean up rpcrdma_create_rep() and rpcrdma_destroy_rep()

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 <[email protected]>
> ---
> 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;
> }
>