| Message ID | 20231009071801.10210-14-guoqing.jiang@linux.dev (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Cleanup for siw | expand |
> -----Original Message----- > From: Guoqing Jiang <guoqing.jiang@linux.dev> > Sent: Monday, October 9, 2023 9:18 AM > To: Bernard Metzler <BMT@zurich.ibm.com>; jgg@ziepe.ca; leon@kernel.org > Cc: linux-rdma@vger.kernel.org > Subject: [EXTERNAL] [PATCH 13/19] RDMA/siw: Simplify siw_qp_id2obj > > Let's set qp and return it. > > Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev> > --- > drivers/infiniband/sw/siw/siw.h | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/infiniband/sw/siw/siw.h > b/drivers/infiniband/sw/siw/siw.h > index 44684b74550f..e127ef493296 100644 > --- a/drivers/infiniband/sw/siw/siw.h > +++ b/drivers/infiniband/sw/siw/siw.h > @@ -601,12 +601,10 @@ static inline struct siw_qp *siw_qp_id2obj(struct > siw_device *sdev, int id) > > rcu_read_lock(); > qp = xa_load(&sdev->qp_xa, id); > - if (likely(qp && kref_get_unless_zero(&qp->ref))) { > - rcu_read_unlock(); > - return qp; > - } > + if (likely(qp && !kref_get_unless_zero(&qp->ref))) > + qp = NULL; > rcu_read_unlock(); > - return NULL; > + return qp; > } > > static inline u32 qp_id(struct siw_qp *qp) > -- > 2.35.3 No let's keep it as is. It openly codes the likely case first. Your code makes the unlikely thing likely.
On 10/25/23 21:04, Bernard Metzler wrote: >> -----Original Message----- >> From: Guoqing Jiang<guoqing.jiang@linux.dev> >> Sent: Monday, October 9, 2023 9:18 AM >> To: Bernard Metzler<BMT@zurich.ibm.com>;jgg@ziepe.ca;leon@kernel.org >> Cc:linux-rdma@vger.kernel.org >> Subject: [EXTERNAL] [PATCH 13/19] RDMA/siw: Simplify siw_qp_id2obj >> >> Let's set qp and return it. >> >> Signed-off-by: Guoqing Jiang<guoqing.jiang@linux.dev> >> --- >> drivers/infiniband/sw/siw/siw.h | 8 +++----- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/infiniband/sw/siw/siw.h >> b/drivers/infiniband/sw/siw/siw.h >> index 44684b74550f..e127ef493296 100644 >> --- a/drivers/infiniband/sw/siw/siw.h >> +++ b/drivers/infiniband/sw/siw/siw.h >> @@ -601,12 +601,10 @@ static inline struct siw_qp *siw_qp_id2obj(struct >> siw_device *sdev, int id) >> >> rcu_read_lock(); >> qp = xa_load(&sdev->qp_xa, id); >> - if (likely(qp && kref_get_unless_zero(&qp->ref))) { >> - rcu_read_unlock(); >> - return qp; >> - } >> + if (likely(qp && !kref_get_unless_zero(&qp->ref))) >> + qp = NULL; >> rcu_read_unlock(); >> - return NULL; >> + return qp; >> } >> >> static inline u32 qp_id(struct siw_qp *qp) >> -- >> 2.35.3 > No let's keep it as is. It openly codes the likely case > first. > > Your code makes the unlikely thing likely. How about change likely to unlikely? If not, I will drop both 13 and 14. Thanks, Guoqing
> -----Original Message----- > From: Guoqing Jiang <guoqing.jiang@linux.dev> > Sent: Thursday, October 26, 2023 8:43 AM > To: Bernard Metzler <BMT@zurich.ibm.com>; jgg@ziepe.ca; leon@kernel.org > Cc: linux-rdma@vger.kernel.org > Subject: [EXTERNAL] Re: [PATCH 13/19] RDMA/siw: Simplify siw_qp_id2obj > > > > On 10/25/23 21:04, Bernard Metzler wrote: > >> -----Original Message----- > >> From: Guoqing Jiang<guoqing.jiang@linux.dev> > >> Sent: Monday, October 9, 2023 9:18 AM > >> To: Bernard Metzler<BMT@zurich.ibm.com>;jgg@ziepe.ca;leon@kernel.org > >> Cc:linux-rdma@vger.kernel.org > >> Subject: [EXTERNAL] [PATCH 13/19] RDMA/siw: Simplify siw_qp_id2obj > >> > >> Let's set qp and return it. > >> > >> Signed-off-by: Guoqing Jiang<guoqing.jiang@linux.dev> > >> --- > >> drivers/infiniband/sw/siw/siw.h | 8 +++----- > >> 1 file changed, 3 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/infiniband/sw/siw/siw.h > >> b/drivers/infiniband/sw/siw/siw.h > >> index 44684b74550f..e127ef493296 100644 > >> --- a/drivers/infiniband/sw/siw/siw.h > >> +++ b/drivers/infiniband/sw/siw/siw.h > >> @@ -601,12 +601,10 @@ static inline struct siw_qp *siw_qp_id2obj(struct > >> siw_device *sdev, int id) > >> > >> rcu_read_lock(); > >> qp = xa_load(&sdev->qp_xa, id); > >> - if (likely(qp && kref_get_unless_zero(&qp->ref))) { > >> - rcu_read_unlock(); > >> - return qp; > >> - } > >> + if (likely(qp && !kref_get_unless_zero(&qp->ref))) > >> + qp = NULL; > >> rcu_read_unlock(); > >> - return NULL; > >> + return qp; > >> } > >> > >> static inline u32 qp_id(struct siw_qp *qp) > >> -- > >> 2.35.3 > > No let's keep it as is. It openly codes the likely case > > first. > > > > Your code makes the unlikely thing likely. > > How about change likely to unlikely? If not, I will drop both 13 and 14. just drop these. I don't see much benefit in changing it.
diff --git a/drivers/infiniband/sw/siw/siw.h b/drivers/infiniband/sw/siw/siw.h index 44684b74550f..e127ef493296 100644 --- a/drivers/infiniband/sw/siw/siw.h +++ b/drivers/infiniband/sw/siw/siw.h @@ -601,12 +601,10 @@ static inline struct siw_qp *siw_qp_id2obj(struct siw_device *sdev, int id) rcu_read_lock(); qp = xa_load(&sdev->qp_xa, id); - if (likely(qp && kref_get_unless_zero(&qp->ref))) { - rcu_read_unlock(); - return qp; - } + if (likely(qp && !kref_get_unless_zero(&qp->ref))) + qp = NULL; rcu_read_unlock(); - return NULL; + return qp; } static inline u32 qp_id(struct siw_qp *qp)
Let's set qp and return it. Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev> --- drivers/infiniband/sw/siw/siw.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)