diff mbox series

[for-rc,1/8] RDMA/hns: Bugfix for posting multiple srq work request

Message ID 1548405030-124329-2-git-send-email-oulijun@huawei.com (mailing list archive)
State Changes Requested
Headers show
Series Some hns bugfixes for 5.0-rc3 | expand

Commit Message

Lijun Ou Jan. 25, 2019, 8:30 a.m. UTC
When the user submits more than 32 work request to a srq queue
at a time, it needs to find the corresponding number of entries
in the bitmap in the idx queue. However, the original lookup
function named ffs only processes 32 bits of the array element,
When the number of srq wqe issued exceeds 32, the ffs will only
process the lower 32 bits of the elements, it will not be able
to get the correct wqe index for srq wqe.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_device.h | 2 +-
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Dennis Dalessandro Jan. 25, 2019, 12:28 p.m. UTC | #1
On 1/25/2019 3:30 AM, Lijun Ou wrote:
> When the user submits more than 32 work request to a srq queue
> at a time, it needs to find the corresponding number of entries
> in the bitmap in the idx queue. However, the original lookup
> function named ffs only processes 32 bits of the array element,
> When the number of srq wqe issued exceeds 32, the ffs will only
> process the lower 32 bits of the elements, it will not be able
> to get the correct wqe index for srq wqe.
> 
> Signed-off-by: Lijun Ou <oulijun@huawei.com>

None of the patches in this series have a Fixes: line.

-Denny
Jason Gunthorpe Jan. 25, 2019, 7:25 p.m. UTC | #2
On Fri, Jan 25, 2019 at 04:30:23PM +0800, Lijun Ou wrote:
> When the user submits more than 32 work request to a srq queue
> at a time, it needs to find the corresponding number of entries
> in the bitmap in the idx queue. However, the original lookup
> function named ffs only processes 32 bits of the array element,
> When the number of srq wqe issued exceeds 32, the ffs will only
> process the lower 32 bits of the elements, it will not be able
> to get the correct wqe index for srq wqe.
> 
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>  drivers/infiniband/hw/hns/hns_roce_device.h | 2 +-
>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

This should use the apis in bitmap.h, not open code them.

Jason
Jason Gunthorpe Jan. 25, 2019, 7:27 p.m. UTC | #3
On Fri, Jan 25, 2019 at 07:28:09AM -0500, Dennis Dalessandro wrote:
> On 1/25/2019 3:30 AM, Lijun Ou wrote:
> > When the user submits more than 32 work request to a srq queue
> > at a time, it needs to find the corresponding number of entries
> > in the bitmap in the idx queue. However, the original lookup
> > function named ffs only processes 32 bits of the array element,
> > When the number of srq wqe issued exceeds 32, the ffs will only
> > process the lower 32 bits of the elements, it will not be able
> > to get the correct wqe index for srq wqe.
> > 
> > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> 
> None of the patches in this series have a Fixes: line.

None of them have good enough commit messages for -rc.

If you want things in -rc you must do a much better job explaining why
it should be in -rc in the commit message.

I'm going to stop looking at your -rc patches at this rate.

Jason
Lijun Ou Jan. 26, 2019, 1:05 a.m. UTC | #4
在 2019/1/25 20:28, Dennis Dalessandro 写道:
> On 1/25/2019 3:30 AM, Lijun Ou wrote:
>> When the user submits more than 32 work request to a srq queue
>> at a time, it needs to find the corresponding number of entries
>> in the bitmap in the idx queue. However, the original lookup
>> function named ffs only processes 32 bits of the array element,
>> When the number of srq wqe issued exceeds 32, the ffs will only
>> process the lower 32 bits of the elements, it will not be able
>> to get the correct wqe index for srq wqe.
>>
>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>
> None of the patches in this series have a Fixes: line.
>
> -Denny
>
Thank you for pointing out. I will carefully rewrite my commit in the future  patch  more.
> .
>
Lijun Ou Jan. 26, 2019, 1:10 a.m. UTC | #5
在 2019/1/26 3:27, Jason Gunthorpe 写道:
> On Fri, Jan 25, 2019 at 07:28:09AM -0500, Dennis Dalessandro wrote:
>> On 1/25/2019 3:30 AM, Lijun Ou wrote:
>>> When the user submits more than 32 work request to a srq queue
>>> at a time, it needs to find the corresponding number of entries
>>> in the bitmap in the idx queue. However, the original lookup
>>> function named ffs only processes 32 bits of the array element,
>>> When the number of srq wqe issued exceeds 32, the ffs will only
>>> process the lower 32 bits of the elements, it will not be able
>>> to get the correct wqe index for srq wqe.
>>>
>>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>> None of the patches in this series have a Fixes: line.
> None of them have good enough commit messages for -rc.
>
> If you want things in -rc you must do a much better job explaining why
> it should be in -rc in the commit message.
>
> I'm going to stop looking at your -rc patches at this rate.
>
> Jason
Maybe it's my emergency neglected this series of rc patch. I will rewrite the commit more carefully with other people's rc patch.
>
Lijun Ou Jan. 26, 2019, 1:11 a.m. UTC | #6
在 2019/1/26 3:25, Jason Gunthorpe 写道:
> On Fri, Jan 25, 2019 at 04:30:23PM +0800, Lijun Ou wrote:
>> When the user submits more than 32 work request to a srq queue
>> at a time, it needs to find the corresponding number of entries
>> in the bitmap in the idx queue. However, the original lookup
>> function named ffs only processes 32 bits of the array element,
>> When the number of srq wqe issued exceeds 32, the ffs will only
>> process the lower 32 bits of the elements, it will not be able
>> to get the correct wqe index for srq wqe.
>>
>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>>  drivers/infiniband/hw/hns/hns_roce_device.h | 2 +-
>>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
> This should use the apis in bitmap.h, not open code them.
>
> Jason
>
yes. I will do it.
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 509e467..67d82dc 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -445,7 +445,7 @@  struct hns_roce_idx_que {
 	u32				buf_size;
 	struct ib_umem			*umem;
 	struct hns_roce_mtt		mtt;
-	u64				*bitmap;
+	unsigned long			*bitmap;
 };
 
 struct hns_roce_srq {
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 543fa15..07f2e7a 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -5644,7 +5644,7 @@  static int find_empty_entry(struct hns_roce_idx_que *idx_que)
 	/* bitmap[i] is set zero if all bits are allocated */
 	for (i = 0; idx_que->bitmap[i] == 0; ++i)
 		;
-	bit_num = ffs(idx_que->bitmap[i]);
+	bit_num = __ffs64(idx_que->bitmap[i]) + 1;
 	idx_que->bitmap[i] &= ~(1ULL << (bit_num - 1));
 
 	return i * sizeof(u64) * 8 + (bit_num - 1);