diff mbox series

[rdma-next,v1] irdma: Use list_last_entry/list_first_entry

Message ID 20210608211415.680-1-shiraz.saleem@intel.com (mailing list archive)
State Accepted
Delegated to: Jason Gunthorpe
Headers show
Series [rdma-next,v1] irdma: Use list_last_entry/list_first_entry | expand

Commit Message

Saleem, Shiraz June 8, 2021, 9:14 p.m. UTC
Use list_last_entry and list_first_entry instead of using prev and next
pointers.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
---
v0->v1: create patch on more recent git version

 drivers/infiniband/hw/irdma/puda.c  | 2 +-
 drivers/infiniband/hw/irdma/utils.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Jason Gunthorpe June 8, 2021, 11:04 p.m. UTC | #1
On Tue, Jun 08, 2021 at 04:14:16PM -0500, Shiraz Saleem wrote:
> Use list_last_entry and list_first_entry instead of using prev and next
> pointers.
> 
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> ---
> v0->v1: create patch on more recent git version
> 
>  drivers/infiniband/hw/irdma/puda.c  | 2 +-
>  drivers/infiniband/hw/irdma/utils.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

This still doesn't apply to the rdma tree.. You need to use the exact
rdma tree, not wherever this came from.

Anyhow I fixed it up by hand.

Jason
Saleem, Shiraz June 8, 2021, 11:19 p.m. UTC | #2
> Subject: Re: [PATCH rdma-next v1] irdma: Use list_last_entry/list_first_entry
> 
> On Tue, Jun 08, 2021 at 04:14:16PM -0500, Shiraz Saleem wrote:
> > Use list_last_entry and list_first_entry instead of using prev and
> > next pointers.
> >
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> > ---
> > v0->v1: create patch on more recent git version
> >
> >  drivers/infiniband/hw/irdma/puda.c  | 2 +-
> > drivers/infiniband/hw/irdma/utils.c | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> This still doesn't apply to the rdma tree.. You need to use the exact rdma tree, not
> wherever this came from.
> 
> Anyhow I fixed it up by hand.
> 

I am not really sure what is going on. This applies cleanly for me to the tip of for-next. Here is snippet of git log -p.

commit fc7f645ca3c1b7a7e9f8eabc83a6ee462b277194 (HEAD -> rdma-for-next)
Author: Shiraz Saleem <shiraz.saleem@intel.com>
Date:   Tue Jun 8 12:02:30 2021 -0500

    irdma: Use list_last_entry/list_first_entry
    
    Use list_last_entry and list_first_entry instead of using prev and next
    pointers.
    
    Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
    Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>

diff --git a/drivers/infiniband/hw/irdma/puda.c b/drivers/infiniband/hw/irdma/puda.c
index 18057139817d..e09d3be90771 100644
--- a/drivers/infiniband/hw/irdma/puda.c
+++ b/drivers/infiniband/hw/irdma/puda.c
@@ -1419,7 +1419,7 @@ irdma_ieq_handle_partial(struct irdma_puda_rsrc *ieq, struct irdma_pfpdu *pfpdu,
 
 error:
        while (!list_empty(&pbufl)) {
-               buf = (struct irdma_puda_buf *)(pbufl.prev);
+               buf = list_last_entry(&pbufl, struct irdma_puda_buf, list);
                list_del(&buf->list);
                list_add(&buf->list, rxlist);
        }
diff --git a/drivers/infiniband/hw/irdma/utils.c b/drivers/infiniband/hw/irdma/utils.c
index 8f04347be52c..b4b91cb81cca 100644
--- a/drivers/infiniband/hw/irdma/utils.c
+++ b/drivers/infiniband/hw/irdma/utils.c
@@ -425,8 +425,8 @@ struct irdma_cqp_request *irdma_alloc_and_get_cqp_request(struct irdma_cqp *cqp,
 
        spin_lock_irqsave(&cqp->req_lock, flags);
        if (!list_empty(&cqp->cqp_avail_reqs)) {
-               cqp_request = list_entry(cqp->cqp_avail_reqs.next,
-                                        struct irdma_cqp_request, list);
+               cqp_request = list_first_entry(&cqp->cqp_avail_reqs,
+                                              struct irdma_cqp_request, list);
                list_del_init(&cqp_request->list);
        }
        spin_unlock_irqrestore(&cqp->req_lock, flags);

commit 61c7d826b81769ea57d094305c900f903768f322 (upstream_v2/for-next)
Author: Kamal Heib <kamalheib1@gmail.com>
Date:   Tue Jun 8 01:15:43 2021 +0300

    RDMA/irdma: Fix return error sign from irdma_modify_qp
    
    There is a typo in the returned error code sign from irdma_modify_qp()
    when the attr_mask is not supported - Fix it.

[....]
Jason Gunthorpe June 8, 2021, 11:25 p.m. UTC | #3
On Tue, Jun 08, 2021 at 11:19:58PM +0000, Saleem, Shiraz wrote:
> > Subject: Re: [PATCH rdma-next v1] irdma: Use list_last_entry/list_first_entry
> > 
> > On Tue, Jun 08, 2021 at 04:14:16PM -0500, Shiraz Saleem wrote:
> > > Use list_last_entry and list_first_entry instead of using prev and
> > > next pointers.
> > >
> > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> > > v0->v1: create patch on more recent git version
> > >
> > >  drivers/infiniband/hw/irdma/puda.c  | 2 +-
> > > drivers/infiniband/hw/irdma/utils.c | 4 ++--
> > >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > This still doesn't apply to the rdma tree.. You need to use the exact rdma tree, not
> > wherever this came from.
> > 
> > Anyhow I fixed it up by hand.
> > 
> 
> I am not really sure what is going on. This applies cleanly for me
> to the tip of for-next.

It is git magic because you have the right blobs and I don't

> diff --git a/drivers/infiniband/hw/irdma/utils.c b/drivers/infiniband/hw/irdma/utils.c
> index 8f04347be52c..b4b91cb81cca 100644
        ^^^^^^^^^^^^

This is the right blob, but the v1 from before had this:

index 8ce3535cdc21..81e590fb77b1 100644

And I don't have 8ce3535cdc21 at all. So git gives up if things aren't
a perfect match.

You have 8ce3535cdc21 so when you tried to apply your own patch it
would have done a 3 way merge and fixed it.

This is why it is important to actually generate diffs against common
commits that other people have because it allows git magic to work
properly. Even if the diffs are nearly identical having the blob be
correct covers alot of sins.

What I tell other people to do is publish everything they send in a
branch in a git repo and I then just keep a local copy of it up to
date and get the needed stuff from there.

Jason
Dan Carpenter June 9, 2021, 6:45 a.m. UTC | #4
On Tue, Jun 08, 2021 at 04:14:16PM -0500, Shiraz Saleem wrote:
> Use list_last_entry and list_first_entry instead of using prev and next
> pointers.
> 
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> ---
> v0->v1: create patch on more recent git version
> 
>  drivers/infiniband/hw/irdma/puda.c  | 2 +-
>  drivers/infiniband/hw/irdma/utils.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/irdma/puda.c b/drivers/infiniband/hw/irdma/puda.c
> index 18057139817d..e09d3be90771 100644
> --- a/drivers/infiniband/hw/irdma/puda.c
> +++ b/drivers/infiniband/hw/irdma/puda.c
> @@ -1419,7 +1419,7 @@ irdma_ieq_handle_partial(struct irdma_puda_rsrc *ieq, struct irdma_pfpdu *pfpdu,
>  
>  error:
>  	while (!list_empty(&pbufl)) {
> -		buf = (struct irdma_puda_buf *)(pbufl.prev);
> +		buf = list_last_entry(&pbufl, struct irdma_puda_buf, list);
>  		list_del(&buf->list);
>  		list_add(&buf->list, rxlist);
>  	}
> diff --git a/drivers/infiniband/hw/irdma/utils.c b/drivers/infiniband/hw/irdma/utils.c
> index 8ce3535cdc21..81e590fb77b1 100644
> --- a/drivers/infiniband/hw/irdma/utils.c
> +++ b/drivers/infiniband/hw/irdma/utils.c
> @@ -425,8 +425,8 @@ struct irdma_cqp_request *irdma_alloc_and_get_cqp_request(struct irdma_cqp *cqp,
>  
>  	spin_lock_irqsave(&cqp->req_lock, flags);
>  	if (!list_empty(&cqp->cqp_avail_reqs)) {
> -		cqp_request = list_entry(cqp->cqp_avail_reqs.next,
> -					 struct irdma_cqp_request, list);
> +		cqp_request = list_first_entry(&cqp->cqp_avail_reqs,
> +					       struct irdma_cqp_request, list);
>  		list_del_init(&cqp_request->list);

Thanks for doing this one too.  :)

regards,
dan carpenter
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/irdma/puda.c b/drivers/infiniband/hw/irdma/puda.c
index 18057139817d..e09d3be90771 100644
--- a/drivers/infiniband/hw/irdma/puda.c
+++ b/drivers/infiniband/hw/irdma/puda.c
@@ -1419,7 +1419,7 @@  irdma_ieq_handle_partial(struct irdma_puda_rsrc *ieq, struct irdma_pfpdu *pfpdu,
 
 error:
 	while (!list_empty(&pbufl)) {
-		buf = (struct irdma_puda_buf *)(pbufl.prev);
+		buf = list_last_entry(&pbufl, struct irdma_puda_buf, list);
 		list_del(&buf->list);
 		list_add(&buf->list, rxlist);
 	}
diff --git a/drivers/infiniband/hw/irdma/utils.c b/drivers/infiniband/hw/irdma/utils.c
index 8ce3535cdc21..81e590fb77b1 100644
--- a/drivers/infiniband/hw/irdma/utils.c
+++ b/drivers/infiniband/hw/irdma/utils.c
@@ -425,8 +425,8 @@  struct irdma_cqp_request *irdma_alloc_and_get_cqp_request(struct irdma_cqp *cqp,
 
 	spin_lock_irqsave(&cqp->req_lock, flags);
 	if (!list_empty(&cqp->cqp_avail_reqs)) {
-		cqp_request = list_entry(cqp->cqp_avail_reqs.next,
-					 struct irdma_cqp_request, list);
+		cqp_request = list_first_entry(&cqp->cqp_avail_reqs,
+					       struct irdma_cqp_request, list);
 		list_del_init(&cqp_request->list);
 	}
 	spin_unlock_irqrestore(&cqp->req_lock, flags);