[v2,04/13] xprtrdma: Rename rpcrdma_req::rl_free
diff mbox

Message ID 20170608155212.18945.37327.stgit@manet.1015granger.net
State New
Headers show

Commit Message

Chuck Lever June 8, 2017, 3:52 p.m. UTC
Clean up: I'm about to use the rl_free field for purposes other than
a free list. So use a more generic name.

This is a refactoring change only.

BugLink: https://bugzilla.linux-nfs.org/show_bug.cgi?id=305
Fixes: 68791649a725 ('xprtrdma: Invalidate in the RPC reply ... ')
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/xprtrdma/verbs.c     |    9 ++++-----
 net/sunrpc/xprtrdma/xprt_rdma.h |    2 +-
 2 files changed, 5 insertions(+), 6 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Anna Schumaker June 9, 2017, 6:58 p.m. UTC | #1
Hi Chuck,

On 06/08/2017 11:52 AM, Chuck Lever wrote:
> Clean up: I'm about to use the rl_free field for purposes other than
> a free list. So use a more generic name.
> 
> This is a refactoring change only.
> 
> BugLink: https://bugzilla.linux-nfs.org/show_bug.cgi?id=305
> Fixes: 68791649a725 ('xprtrdma: Invalidate in the RPC reply ... ')
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  net/sunrpc/xprtrdma/verbs.c     |    9 ++++-----
>  net/sunrpc/xprtrdma/xprt_rdma.h |    2 +-
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index a8be66d..df72224 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -971,7 +971,6 @@ struct rpcrdma_req *
>  	if (req == NULL)
>  		return ERR_PTR(-ENOMEM);
>  
> -	INIT_LIST_HEAD(&req->rl_free);

Does rl_list still need to get initialized somewhere?

Thanks,
Anna

>  	spin_lock(&buffer->rb_reqslock);
>  	list_add(&req->rl_all, &buffer->rb_allreqs);
>  	spin_unlock(&buffer->rb_reqslock);
> @@ -1055,7 +1054,7 @@ struct rpcrdma_rep *
>  			goto out;
>  		}
>  		req->rl_backchannel = false;
> -		list_add(&req->rl_free, &buf->rb_send_bufs);
> +		list_add(&req->rl_list, &buf->rb_send_bufs);
>  	}
>  
>  	INIT_LIST_HEAD(&buf->rb_recv_bufs);
> @@ -1084,8 +1083,8 @@ struct rpcrdma_rep *
>  	struct rpcrdma_req *req;
>  
>  	req = list_first_entry(&buf->rb_send_bufs,
> -			       struct rpcrdma_req, rl_free);
> -	list_del(&req->rl_free);
> +			       struct rpcrdma_req, rl_list);
> +	list_del(&req->rl_list);
>  	return req;
>  }
>  
> @@ -1268,7 +1267,7 @@ struct rpcrdma_req *
>  
>  	spin_lock(&buffers->rb_lock);
>  	buffers->rb_send_count--;
> -	list_add_tail(&req->rl_free, &buffers->rb_send_bufs);
> +	list_add_tail(&req->rl_list, &buffers->rb_send_bufs);
>  	if (rep) {
>  		buffers->rb_recv_count--;
>  		list_add_tail(&rep->rr_list, &buffers->rb_recv_bufs);
> diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
> index 1c23117..ad918c8 100644
> --- a/net/sunrpc/xprtrdma/xprt_rdma.h
> +++ b/net/sunrpc/xprtrdma/xprt_rdma.h
> @@ -340,7 +340,7 @@ enum {
>  
>  struct rpcrdma_buffer;
>  struct rpcrdma_req {
> -	struct list_head	rl_free;
> +	struct list_head	rl_list;
>  	unsigned int		rl_mapped_sges;
>  	unsigned int		rl_connect_cookie;
>  	struct rpcrdma_buffer	*rl_buffer;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever June 9, 2017, 7:03 p.m. UTC | #2
> On Jun 9, 2017, at 2:58 PM, Anna Schumaker <Anna.Schumaker@Netapp.com> wrote:
> 
> Hi Chuck,
> 
> On 06/08/2017 11:52 AM, Chuck Lever wrote:
>> Clean up: I'm about to use the rl_free field for purposes other than
>> a free list. So use a more generic name.
>> 
>> This is a refactoring change only.
>> 
>> BugLink: https://bugzilla.linux-nfs.org/show_bug.cgi?id=305
>> Fixes: 68791649a725 ('xprtrdma: Invalidate in the RPC reply ... ')
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> net/sunrpc/xprtrdma/verbs.c     |    9 ++++-----
>> net/sunrpc/xprtrdma/xprt_rdma.h |    2 +-
>> 2 files changed, 5 insertions(+), 6 deletions(-)
>> 
>> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
>> index a8be66d..df72224 100644
>> --- a/net/sunrpc/xprtrdma/verbs.c
>> +++ b/net/sunrpc/xprtrdma/verbs.c
>> @@ -971,7 +971,6 @@ struct rpcrdma_req *
>> 	if (req == NULL)
>> 		return ERR_PTR(-ENOMEM);
>> 
>> -	INIT_LIST_HEAD(&req->rl_free);
> 
> Does rl_list still need to get initialized somewhere?

rl_free/rl_list isn't the anchor of a list. It is only
ever a member of a list. So this INIT_LIST_HEAD is actually
unnecessary.


> Thanks,
> Anna
> 
>> 	spin_lock(&buffer->rb_reqslock);
>> 	list_add(&req->rl_all, &buffer->rb_allreqs);
>> 	spin_unlock(&buffer->rb_reqslock);
>> @@ -1055,7 +1054,7 @@ struct rpcrdma_rep *
>> 			goto out;
>> 		}
>> 		req->rl_backchannel = false;
>> -		list_add(&req->rl_free, &buf->rb_send_bufs);
>> +		list_add(&req->rl_list, &buf->rb_send_bufs);
>> 	}
>> 
>> 	INIT_LIST_HEAD(&buf->rb_recv_bufs);
>> @@ -1084,8 +1083,8 @@ struct rpcrdma_rep *
>> 	struct rpcrdma_req *req;
>> 
>> 	req = list_first_entry(&buf->rb_send_bufs,
>> -			       struct rpcrdma_req, rl_free);
>> -	list_del(&req->rl_free);
>> +			       struct rpcrdma_req, rl_list);
>> +	list_del(&req->rl_list);
>> 	return req;
>> }
>> 
>> @@ -1268,7 +1267,7 @@ struct rpcrdma_req *
>> 
>> 	spin_lock(&buffers->rb_lock);
>> 	buffers->rb_send_count--;
>> -	list_add_tail(&req->rl_free, &buffers->rb_send_bufs);
>> +	list_add_tail(&req->rl_list, &buffers->rb_send_bufs);
>> 	if (rep) {
>> 		buffers->rb_recv_count--;
>> 		list_add_tail(&rep->rr_list, &buffers->rb_recv_bufs);
>> diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
>> index 1c23117..ad918c8 100644
>> --- a/net/sunrpc/xprtrdma/xprt_rdma.h
>> +++ b/net/sunrpc/xprtrdma/xprt_rdma.h
>> @@ -340,7 +340,7 @@ enum {
>> 
>> struct rpcrdma_buffer;
>> struct rpcrdma_req {
>> -	struct list_head	rl_free;
>> +	struct list_head	rl_list;
>> 	unsigned int		rl_mapped_sges;
>> 	unsigned int		rl_connect_cookie;
>> 	struct rpcrdma_buffer	*rl_buffer;
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever June 9, 2017, 7:12 p.m. UTC | #3
> On Jun 9, 2017, at 3:03 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
> 
>> 
>> On Jun 9, 2017, at 2:58 PM, Anna Schumaker <Anna.Schumaker@Netapp.com> wrote:
>> 
>> Hi Chuck,
>> 
>> On 06/08/2017 11:52 AM, Chuck Lever wrote:
>>> Clean up: I'm about to use the rl_free field for purposes other than
>>> a free list. So use a more generic name.
>>> 
>>> This is a refactoring change only.
>>> 
>>> BugLink: https://bugzilla.linux-nfs.org/show_bug.cgi?id=305
>>> Fixes: 68791649a725 ('xprtrdma: Invalidate in the RPC reply ... ')
>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>> ---
>>> net/sunrpc/xprtrdma/verbs.c     |    9 ++++-----
>>> net/sunrpc/xprtrdma/xprt_rdma.h |    2 +-
>>> 2 files changed, 5 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
>>> index a8be66d..df72224 100644
>>> --- a/net/sunrpc/xprtrdma/verbs.c
>>> +++ b/net/sunrpc/xprtrdma/verbs.c
>>> @@ -971,7 +971,6 @@ struct rpcrdma_req *
>>> 	if (req == NULL)
>>> 		return ERR_PTR(-ENOMEM);
>>> 
>>> -	INIT_LIST_HEAD(&req->rl_free);
>> 
>> Does rl_list still need to get initialized somewhere?
> 
> rl_free/rl_list isn't the anchor of a list. It is only
> ever a member of a list. So this INIT_LIST_HEAD is actually
> unnecessary.

Or, put another way: since list_empty() is not used
on this field anywhere, it shouldn't be necessary to
ensure it is initialized.


>> Thanks,
>> Anna
>> 
>>> 	spin_lock(&buffer->rb_reqslock);
>>> 	list_add(&req->rl_all, &buffer->rb_allreqs);
>>> 	spin_unlock(&buffer->rb_reqslock);
>>> @@ -1055,7 +1054,7 @@ struct rpcrdma_rep *
>>> 			goto out;
>>> 		}
>>> 		req->rl_backchannel = false;
>>> -		list_add(&req->rl_free, &buf->rb_send_bufs);
>>> +		list_add(&req->rl_list, &buf->rb_send_bufs);
>>> 	}
>>> 
>>> 	INIT_LIST_HEAD(&buf->rb_recv_bufs);
>>> @@ -1084,8 +1083,8 @@ struct rpcrdma_rep *
>>> 	struct rpcrdma_req *req;
>>> 
>>> 	req = list_first_entry(&buf->rb_send_bufs,
>>> -			       struct rpcrdma_req, rl_free);
>>> -	list_del(&req->rl_free);
>>> +			       struct rpcrdma_req, rl_list);
>>> +	list_del(&req->rl_list);
>>> 	return req;
>>> }
>>> 
>>> @@ -1268,7 +1267,7 @@ struct rpcrdma_req *
>>> 
>>> 	spin_lock(&buffers->rb_lock);
>>> 	buffers->rb_send_count--;
>>> -	list_add_tail(&req->rl_free, &buffers->rb_send_bufs);
>>> +	list_add_tail(&req->rl_list, &buffers->rb_send_bufs);
>>> 	if (rep) {
>>> 		buffers->rb_recv_count--;
>>> 		list_add_tail(&rep->rr_list, &buffers->rb_recv_bufs);
>>> diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
>>> index 1c23117..ad918c8 100644
>>> --- a/net/sunrpc/xprtrdma/xprt_rdma.h
>>> +++ b/net/sunrpc/xprtrdma/xprt_rdma.h
>>> @@ -340,7 +340,7 @@ enum {
>>> 
>>> struct rpcrdma_buffer;
>>> struct rpcrdma_req {
>>> -	struct list_head	rl_free;
>>> +	struct list_head	rl_list;
>>> 	unsigned int		rl_mapped_sges;
>>> 	unsigned int		rl_connect_cookie;
>>> 	struct rpcrdma_buffer	*rl_buffer;
>>> 
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> Chuck Lever
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anna Schumaker June 9, 2017, 7:15 p.m. UTC | #4
On 06/09/2017 03:12 PM, Chuck Lever wrote:
> 
>> On Jun 9, 2017, at 3:03 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
>>
>>>
>>> On Jun 9, 2017, at 2:58 PM, Anna Schumaker <Anna.Schumaker@Netapp.com> wrote:
>>>
>>> Hi Chuck,
>>>
>>> On 06/08/2017 11:52 AM, Chuck Lever wrote:
>>>> Clean up: I'm about to use the rl_free field for purposes other than
>>>> a free list. So use a more generic name.
>>>>
>>>> This is a refactoring change only.
>>>>
>>>> BugLink: https://bugzilla.linux-nfs.org/show_bug.cgi?id=305
>>>> Fixes: 68791649a725 ('xprtrdma: Invalidate in the RPC reply ... ')
>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>>> ---
>>>> net/sunrpc/xprtrdma/verbs.c     |    9 ++++-----
>>>> net/sunrpc/xprtrdma/xprt_rdma.h |    2 +-
>>>> 2 files changed, 5 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
>>>> index a8be66d..df72224 100644
>>>> --- a/net/sunrpc/xprtrdma/verbs.c
>>>> +++ b/net/sunrpc/xprtrdma/verbs.c
>>>> @@ -971,7 +971,6 @@ struct rpcrdma_req *
>>>> 	if (req == NULL)
>>>> 		return ERR_PTR(-ENOMEM);
>>>>
>>>> -	INIT_LIST_HEAD(&req->rl_free);
>>>
>>> Does rl_list still need to get initialized somewhere?
>>
>> rl_free/rl_list isn't the anchor of a list. It is only
>> ever a member of a list. So this INIT_LIST_HEAD is actually
>> unnecessary.
> 
> Or, put another way: since list_empty() is not used
> on this field anywhere, it shouldn't be necessary to
> ensure it is initialized.

Thanks for the explanation!  I took a second look, and that all sounds right :)

> 
> 
>>> Thanks,
>>> Anna
>>>
>>>> 	spin_lock(&buffer->rb_reqslock);
>>>> 	list_add(&req->rl_all, &buffer->rb_allreqs);
>>>> 	spin_unlock(&buffer->rb_reqslock);
>>>> @@ -1055,7 +1054,7 @@ struct rpcrdma_rep *
>>>> 			goto out;
>>>> 		}
>>>> 		req->rl_backchannel = false;
>>>> -		list_add(&req->rl_free, &buf->rb_send_bufs);
>>>> +		list_add(&req->rl_list, &buf->rb_send_bufs);
>>>> 	}
>>>>
>>>> 	INIT_LIST_HEAD(&buf->rb_recv_bufs);
>>>> @@ -1084,8 +1083,8 @@ struct rpcrdma_rep *
>>>> 	struct rpcrdma_req *req;
>>>>
>>>> 	req = list_first_entry(&buf->rb_send_bufs,
>>>> -			       struct rpcrdma_req, rl_free);
>>>> -	list_del(&req->rl_free);
>>>> +			       struct rpcrdma_req, rl_list);
>>>> +	list_del(&req->rl_list);
>>>> 	return req;
>>>> }
>>>>
>>>> @@ -1268,7 +1267,7 @@ struct rpcrdma_req *
>>>>
>>>> 	spin_lock(&buffers->rb_lock);
>>>> 	buffers->rb_send_count--;
>>>> -	list_add_tail(&req->rl_free, &buffers->rb_send_bufs);
>>>> +	list_add_tail(&req->rl_list, &buffers->rb_send_bufs);
>>>> 	if (rep) {
>>>> 		buffers->rb_recv_count--;
>>>> 		list_add_tail(&rep->rr_list, &buffers->rb_recv_bufs);
>>>> diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
>>>> index 1c23117..ad918c8 100644
>>>> --- a/net/sunrpc/xprtrdma/xprt_rdma.h
>>>> +++ b/net/sunrpc/xprtrdma/xprt_rdma.h
>>>> @@ -340,7 +340,7 @@ enum {
>>>>
>>>> struct rpcrdma_buffer;
>>>> struct rpcrdma_req {
>>>> -	struct list_head	rl_free;
>>>> +	struct list_head	rl_list;
>>>> 	unsigned int		rl_mapped_sges;
>>>> 	unsigned int		rl_connect_cookie;
>>>> 	struct rpcrdma_buffer	*rl_buffer;
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> --
>> Chuck Lever
>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> Chuck Lever
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index a8be66d..df72224 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -971,7 +971,6 @@  struct rpcrdma_req *
 	if (req == NULL)
 		return ERR_PTR(-ENOMEM);
 
-	INIT_LIST_HEAD(&req->rl_free);
 	spin_lock(&buffer->rb_reqslock);
 	list_add(&req->rl_all, &buffer->rb_allreqs);
 	spin_unlock(&buffer->rb_reqslock);
@@ -1055,7 +1054,7 @@  struct rpcrdma_rep *
 			goto out;
 		}
 		req->rl_backchannel = false;
-		list_add(&req->rl_free, &buf->rb_send_bufs);
+		list_add(&req->rl_list, &buf->rb_send_bufs);
 	}
 
 	INIT_LIST_HEAD(&buf->rb_recv_bufs);
@@ -1084,8 +1083,8 @@  struct rpcrdma_rep *
 	struct rpcrdma_req *req;
 
 	req = list_first_entry(&buf->rb_send_bufs,
-			       struct rpcrdma_req, rl_free);
-	list_del(&req->rl_free);
+			       struct rpcrdma_req, rl_list);
+	list_del(&req->rl_list);
 	return req;
 }
 
@@ -1268,7 +1267,7 @@  struct rpcrdma_req *
 
 	spin_lock(&buffers->rb_lock);
 	buffers->rb_send_count--;
-	list_add_tail(&req->rl_free, &buffers->rb_send_bufs);
+	list_add_tail(&req->rl_list, &buffers->rb_send_bufs);
 	if (rep) {
 		buffers->rb_recv_count--;
 		list_add_tail(&rep->rr_list, &buffers->rb_recv_bufs);
diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
index 1c23117..ad918c8 100644
--- a/net/sunrpc/xprtrdma/xprt_rdma.h
+++ b/net/sunrpc/xprtrdma/xprt_rdma.h
@@ -340,7 +340,7 @@  enum {
 
 struct rpcrdma_buffer;
 struct rpcrdma_req {
-	struct list_head	rl_free;
+	struct list_head	rl_list;
 	unsigned int		rl_mapped_sges;
 	unsigned int		rl_connect_cookie;
 	struct rpcrdma_buffer	*rl_buffer;