| Message ID | 20170608155212.18945.37327.stgit@manet.1015granger.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
> 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
> 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
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
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;
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