2023-11-29 09:48:28

by Junxian Huang

[permalink] [raw]
Subject: [PATCH for-rc 0/6] Bugfixes and improvements for hns RoCE

Here are several bugfixes and improvements for hns RoCE.

Chengchang Tang (4):
RDMA/hns: Rename the interrupts
RDMA/hns: Remove unnecessary checks for NULL in mtr_alloc_bufs()
RDMA/hns: Fix memory leak in free_mr_init()
RDMA/hns: Improve the readability of free mr uninit

Junxian Huang (2):
RDMA/hns: Response dmac to userspace
RDMA/hns: Add a max length of gid table

drivers/infiniband/hw/hns/hns_roce_ah.c | 7 ++
drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 87 +++++++++++++++-------
drivers/infiniband/hw/hns/hns_roce_mr.c | 2 +-
include/uapi/rdma/hns-abi.h | 5 ++
4 files changed, 73 insertions(+), 28 deletions(-)

--
2.30.0


2023-11-29 09:48:31

by Junxian Huang

[permalink] [raw]
Subject: [PATCH for-rc 5/6] RDMA/hns: Fix memory leak in free_mr_init()

From: Chengchang Tang <[email protected]>

When a reserved QP fails to be created, the memory of the remaining
created reserved QPs is leaked.

Fixes: 70f92521584f ("RDMA/hns: Use the reserved loopback QPs to free MR before destroying MPT")
Signed-off-by: Chengchang Tang <[email protected]>
Signed-off-by: Junxian Huang <[email protected]>
---
drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 8126922b4e21..538f3e8949fc 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -2705,6 +2705,10 @@ static int free_mr_alloc_res(struct hns_roce_dev *hr_dev)
return 0;

create_failed_qp:
+ for (i--; i >= 0; i--) {
+ hns_roce_v2_destroy_qp(&free_mr->rsv_qp[i]->ibqp, NULL);
+ kfree(free_mr->rsv_qp[i]);
+ }
hns_roce_destroy_cq(cq, NULL);
kfree(cq);

--
2.30.0

2023-11-29 09:48:35

by Junxian Huang

[permalink] [raw]
Subject: [PATCH for-rc 1/6] RDMA/hns: Rename the interrupts

From: Chengchang Tang <[email protected]>

Now, different devices may have the same interrupt name, which
makes it difficult for users to distinguish between these
interrupts.

Modify the naming style to be consistent with our network devices.
Before:
"hns-aeq-0"
"hns-ceq-0"
...

Now:
"hns-0000:35:00.0-aeq-0"
"hns-0000:35:00.0-ceq-0"
...

Signed-off-by: Chengchang Tang <[email protected]>
Signed-off-by: Junxian Huang <[email protected]>
---
drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 2bca9560f32d..1ceeedfa225f 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -6457,15 +6457,16 @@ static int __hns_roce_request_irq(struct hns_roce_dev *hr_dev, int irq_num,
/* irq contains: abnormal + AEQ + CEQ */
for (j = 0; j < other_num; j++)
snprintf((char *)hr_dev->irq_names[j], HNS_ROCE_INT_NAME_LEN,
- "hns-abn-%d", j);
+ "hns-%s-abn-%d", pci_name(hr_dev->pci_dev), j);

for (j = other_num; j < (other_num + aeq_num); j++)
snprintf((char *)hr_dev->irq_names[j], HNS_ROCE_INT_NAME_LEN,
- "hns-aeq-%d", j - other_num);
+ "hns-%s-aeq-%d", pci_name(hr_dev->pci_dev), j - other_num);

for (j = (other_num + aeq_num); j < irq_num; j++)
snprintf((char *)hr_dev->irq_names[j], HNS_ROCE_INT_NAME_LEN,
- "hns-ceq-%d", j - other_num - aeq_num);
+ "hns-%s-ceq-%d", pci_name(hr_dev->pci_dev),
+ j - other_num - aeq_num);

for (j = 0; j < irq_num; j++) {
if (j < other_num)
--
2.30.0

2023-11-29 09:48:44

by Junxian Huang

[permalink] [raw]
Subject: [PATCH for-rc 3/6] RDMA/hns: Add a max length of gid table

IB-core and rdma-core restrict the sgid_index specified by users,
which is uint8_t/u8 data type, to only be within the range of 0-255,
so it's meaningless to support excessively large gid_table_len.

On the other hand, ib-core creates as many sysfs gid files as
gid_table_len, most of which are not only useless because of the
reason above, but also greatly increase the traversal time of
the sysfs gid files for applications.

This patch limits the maximum length of gid table to 256.

Signed-off-by: Junxian Huang <[email protected]>
---
drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 1ceeedfa225f..8126922b4e21 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -2055,6 +2055,7 @@ static void set_hem_page_size(struct hns_roce_dev *hr_dev)
/* Apply all loaded caps before setting to hardware */
static void apply_func_caps(struct hns_roce_dev *hr_dev)
{
+#define MAX_GID_TBL_LEN 256
struct hns_roce_caps *caps = &hr_dev->caps;
struct hns_roce_v2_priv *priv = hr_dev->priv;

@@ -2090,8 +2091,14 @@ static void apply_func_caps(struct hns_roce_dev *hr_dev)
caps->gmv_entry_sz = HNS_ROCE_V3_GMV_ENTRY_SZ;

caps->gmv_hop_num = HNS_ROCE_HOP_NUM_0;
- caps->gid_table_len[0] = caps->gmv_bt_num *
- (HNS_HW_PAGE_SIZE / caps->gmv_entry_sz);
+
+ /* It's meaningless to support excessively large gid_table_len,
+ * as the type of sgid_index in kernel struct ib_global_route
+ * and userspace struct ibv_global_route are u8/uint8_t (0-255).
+ */
+ caps->gid_table_len[0] = min_t(u32, MAX_GID_TBL_LEN,
+ caps->gmv_bt_num *
+ (HNS_HW_PAGE_SIZE / caps->gmv_entry_sz));

caps->gmv_entry_num = caps->gmv_bt_num * (PAGE_SIZE /
caps->gmv_entry_sz);
--
2.30.0

2023-11-29 09:48:45

by Junxian Huang

[permalink] [raw]
Subject: [PATCH for-rc 4/6] RDMA/hns: Remove unnecessary checks for NULL in mtr_alloc_bufs()

From: Chengchang Tang <[email protected]>

ib_umem_get() never return NULL.

Fixes: 3c873161a0d7 ("RDMA/hns: Add support for addressing when hopnum is 0")
Signed-off-by: Chengchang Tang <[email protected]>
Signed-off-by: Junxian Huang <[email protected]>
---
drivers/infiniband/hw/hns/hns_roce_mr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_mr.c b/drivers/infiniband/hw/hns/hns_roce_mr.c
index 14376490ac22..e71988a6c277 100644
--- a/drivers/infiniband/hw/hns/hns_roce_mr.c
+++ b/drivers/infiniband/hw/hns/hns_roce_mr.c
@@ -674,7 +674,7 @@ static int mtr_alloc_bufs(struct hns_roce_dev *hr_dev, struct hns_roce_mtr *mtr,
mtr->kmem = NULL;
mtr->umem = ib_umem_get(ibdev, user_addr, total_size,
buf_attr->user_access);
- if (IS_ERR_OR_NULL(mtr->umem)) {
+ if (IS_ERR(mtr->umem)) {
ibdev_err(ibdev, "failed to get umem, ret = %ld.\n",
PTR_ERR(mtr->umem));
return -ENOMEM;
--
2.30.0

2023-11-29 09:48:48

by Junxian Huang

[permalink] [raw]
Subject: [PATCH for-rc 6/6] RDMA/hns: Improve the readability of free mr uninit

From: Chengchang Tang <[email protected]>

Extract uninit functions of free mr qp, cq and pd to improve
readability.

Signed-off-by: Chengchang Tang <[email protected]>
Signed-off-by: Junxian Huang <[email protected]>
---
drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 73 ++++++++++++++--------
1 file changed, 47 insertions(+), 26 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 538f3e8949fc..be02034a8818 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -2573,6 +2573,19 @@ static struct ib_pd *free_mr_init_pd(struct hns_roce_dev *hr_dev)
return pd;
}

+static void free_mr_uninit_pd(struct hns_roce_dev *hr_dev)
+{
+ struct hns_roce_v2_priv *priv = hr_dev->priv;
+ struct hns_roce_v2_free_mr *free_mr = &priv->free_mr;
+
+ if (!free_mr->rsv_pd)
+ return;
+
+ hns_roce_dealloc_pd(&free_mr->rsv_pd->ibpd, NULL);
+ kfree(free_mr->rsv_pd);
+ free_mr->rsv_pd = NULL;
+}
+
static struct ib_cq *free_mr_init_cq(struct hns_roce_dev *hr_dev)
{
struct hns_roce_v2_priv *priv = hr_dev->priv;
@@ -2607,6 +2620,19 @@ static struct ib_cq *free_mr_init_cq(struct hns_roce_dev *hr_dev)
return cq;
}

+static void free_mr_uninit_cq(struct hns_roce_dev *hr_dev)
+{
+ struct hns_roce_v2_priv *priv = hr_dev->priv;
+ struct hns_roce_v2_free_mr *free_mr = &priv->free_mr;
+
+ if (!free_mr->rsv_cq)
+ return;
+
+ hns_roce_destroy_cq(&free_mr->rsv_cq->ib_cq, NULL);
+ kfree(free_mr->rsv_cq);
+ free_mr->rsv_cq = NULL;
+}
+
static int free_mr_init_qp(struct hns_roce_dev *hr_dev, struct ib_cq *cq,
struct ib_qp_init_attr *init_attr, int i)
{
@@ -2638,6 +2664,19 @@ static int free_mr_init_qp(struct hns_roce_dev *hr_dev, struct ib_cq *cq,
return 0;
}

+static void free_mr_uninit_qp(struct hns_roce_dev *hr_dev, int i)
+{
+ struct hns_roce_v2_priv *priv = hr_dev->priv;
+ struct hns_roce_v2_free_mr *free_mr = &priv->free_mr;
+
+ if (!free_mr->rsv_qp[i])
+ return;
+
+ hns_roce_v2_destroy_qp(&free_mr->rsv_qp[i]->ibqp, NULL);
+ kfree(free_mr->rsv_qp[i]);
+ free_mr->rsv_qp[i] = NULL;
+}
+
static void free_mr_exit(struct hns_roce_dev *hr_dev)
{
struct hns_roce_v2_priv *priv = hr_dev->priv;
@@ -2645,26 +2684,12 @@ static void free_mr_exit(struct hns_roce_dev *hr_dev)
struct ib_qp *qp;
int i;

- for (i = 0; i < ARRAY_SIZE(free_mr->rsv_qp); i++) {
- if (free_mr->rsv_qp[i]) {
- qp = &free_mr->rsv_qp[i]->ibqp;
- hns_roce_v2_destroy_qp(qp, NULL);
- kfree(free_mr->rsv_qp[i]);
- free_mr->rsv_qp[i] = NULL;
- }
- }
+ for (i = 0; i < ARRAY_SIZE(free_mr->rsv_qp); i++)
+ free_mr_uninit_qp(hr_dev, i);

- if (free_mr->rsv_cq) {
- hns_roce_destroy_cq(&free_mr->rsv_cq->ib_cq, NULL);
- kfree(free_mr->rsv_cq);
- free_mr->rsv_cq = NULL;
- }
+ free_mr_uninit_cq(hr_dev);

- if (free_mr->rsv_pd) {
- hns_roce_dealloc_pd(&free_mr->rsv_pd->ibpd, NULL);
- kfree(free_mr->rsv_pd);
- free_mr->rsv_pd = NULL;
- }
+ free_mr_uninit_pd(hr_dev);
}

static int free_mr_alloc_res(struct hns_roce_dev *hr_dev)
@@ -2705,16 +2730,12 @@ static int free_mr_alloc_res(struct hns_roce_dev *hr_dev)
return 0;

create_failed_qp:
- for (i--; i >= 0; i--) {
- hns_roce_v2_destroy_qp(&free_mr->rsv_qp[i]->ibqp, NULL);
- kfree(free_mr->rsv_qp[i]);
- }
- hns_roce_destroy_cq(cq, NULL);
- kfree(cq);
+ for (i--; i >= 0; i--)
+ free_mr_uninit_qp(hr_dev, i);
+ free_mr_uninit_cq(hr_dev);

create_failed_cq:
- hns_roce_dealloc_pd(pd, NULL);
- kfree(pd);
+ free_mr_uninit_pd(hr_dev);

return ret;
}
--
2.30.0

2023-11-29 09:49:08

by Junxian Huang

[permalink] [raw]
Subject: [PATCH for-rc 2/6] RDMA/hns: Response dmac to userspace

While creating AH, dmac is already resolved in kernel. Response dmac
to userspace so that userspace doesn't need to resolve dmac repeatedly.

Signed-off-by: Junxian Huang <[email protected]>
---
drivers/infiniband/hw/hns/hns_roce_ah.c | 7 +++++++
include/uapi/rdma/hns-abi.h | 5 +++++
2 files changed, 12 insertions(+)

diff --git a/drivers/infiniband/hw/hns/hns_roce_ah.c b/drivers/infiniband/hw/hns/hns_roce_ah.c
index 3df032ddda18..e839a83c4b8f 100644
--- a/drivers/infiniband/hw/hns/hns_roce_ah.c
+++ b/drivers/infiniband/hw/hns/hns_roce_ah.c
@@ -57,6 +57,7 @@ int hns_roce_create_ah(struct ib_ah *ibah, struct rdma_ah_init_attr *init_attr,
struct rdma_ah_attr *ah_attr = init_attr->ah_attr;
const struct ib_global_route *grh = rdma_ah_read_grh(ah_attr);
struct hns_roce_dev *hr_dev = to_hr_dev(ibah->device);
+ struct hns_roce_ib_create_ah_resp resp = {};
struct hns_roce_ah *ah = to_hr_ah(ibah);
int ret = 0;
u32 max_sl;
@@ -97,6 +98,12 @@ int hns_roce_create_ah(struct ib_ah *ibah, struct rdma_ah_init_attr *init_attr,
ah->av.vlan_en = ah->av.vlan_id < VLAN_N_VID;
}

+ if (udata) {
+ memcpy(resp.dmac, ah_attr->roce.dmac, ETH_ALEN);
+ ret = ib_copy_to_udata(udata, &resp,
+ min(udata->outlen, sizeof(resp)));
+ }
+
return ret;
}

diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
index ce0f37f83416..c996e151081e 100644
--- a/include/uapi/rdma/hns-abi.h
+++ b/include/uapi/rdma/hns-abi.h
@@ -125,4 +125,9 @@ struct hns_roce_ib_alloc_pd_resp {
__u32 pdn;
};

+struct hns_roce_ib_create_ah_resp {
+ __u8 dmac[6];
+ __u8 reserved[2];
+};
+
#endif /* HNS_ABI_USER_H */
--
2.30.0

2023-12-04 14:29:52

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [PATCH for-rc 0/6] Bugfixes and improvements for hns RoCE

On Wed, Nov 29, 2023 at 05:44:28PM +0800, Junxian Huang wrote:
> Here are several bugfixes and improvements for hns RoCE.
>
> Chengchang Tang (4):
> RDMA/hns: Rename the interrupts
> RDMA/hns: Remove unnecessary checks for NULL in mtr_alloc_bufs()
> RDMA/hns: Fix memory leak in free_mr_init()
> RDMA/hns: Improve the readability of free mr uninit

1. The series doesn't apply.
➜ kernel git:(wip/leon-for-next) ~/src/b4/b4.sh shazam -l -s https://lore.kernel.org/all/[email protected] -P 1-5
2. Please drop patch #6 as you are deleting the code which you added in
first patches without actual gain.

Thanks

>
> Junxian Huang (2):
> RDMA/hns: Response dmac to userspace
> RDMA/hns: Add a max length of gid table
>
> drivers/infiniband/hw/hns/hns_roce_ah.c | 7 ++
> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 87 +++++++++++++++-------
> drivers/infiniband/hw/hns/hns_roce_mr.c | 2 +-
> include/uapi/rdma/hns-abi.h | 5 ++
> 4 files changed, 73 insertions(+), 28 deletions(-)
>
> --
> 2.30.0
>
>

2023-12-05 02:06:04

by Junxian Huang

[permalink] [raw]
Subject: Re: [PATCH for-rc 0/6] Bugfixes and improvements for hns RoCE



On 2023/12/4 22:24, Leon Romanovsky wrote:
> On Wed, Nov 29, 2023 at 05:44:28PM +0800, Junxian Huang wrote:
>> Here are several bugfixes and improvements for hns RoCE.
>>
>> Chengchang Tang (4):
>> RDMA/hns: Rename the interrupts
>> RDMA/hns: Remove unnecessary checks for NULL in mtr_alloc_bufs()
>> RDMA/hns: Fix memory leak in free_mr_init()
>> RDMA/hns: Improve the readability of free mr uninit
>
> 1. The series doesn't apply.
> ➜ kernel git:(wip/leon-for-next) ~/src/b4/b4.sh shazam -l -s https://lore.kernel.org/all/[email protected] -P 1-5

Is this series going to be applied to -next?

> 2. Please drop patch #6 as you are deleting the code which you added in
> first patches without actual gain.

Is it better to drop it directly or merge it with the previous patch?

Thanks,
Junxian

>
> Thanks
>
>>
>> Junxian Huang (2):
>> RDMA/hns: Response dmac to userspace
>> RDMA/hns: Add a max length of gid table
>>
>> drivers/infiniband/hw/hns/hns_roce_ah.c | 7 ++
>> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 87 +++++++++++++++-------
>> drivers/infiniband/hw/hns/hns_roce_mr.c | 2 +-
>> include/uapi/rdma/hns-abi.h | 5 ++
>> 4 files changed, 73 insertions(+), 28 deletions(-)
>>
>> --
>> 2.30.0
>>
>>
>

2023-12-06 05:06:37

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH for-rc 6/6] RDMA/hns: Improve the readability of free mr uninit

Hi Junxian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.7-rc4]
[cannot apply to rdma/for-next next-20231205]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Junxian-Huang/RDMA-hns-Rename-the-interrupts/20231129-183932
base: linus/master
patch link: https://lore.kernel.org/r/20231129094434.134528-7-huangjunxian6%40hisilicon.com
patch subject: [PATCH for-rc 6/6] RDMA/hns: Improve the readability of free mr uninit
config: sparc64-allmodconfig (https://download.01.org/0day-ci/archive/20231206/[email protected]/config)
compiler: sparc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231206/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

drivers/infiniband/hw/hns/hns_roce_hw_v2.c: In function 'free_mr_exit':
>> drivers/infiniband/hw/hns/hns_roce_hw_v2.c:2684:23: warning: unused variable 'qp' [-Wunused-variable]
2684 | struct ib_qp *qp;
| ^~


vim +/qp +2684 drivers/infiniband/hw/hns/hns_roce_hw_v2.c

2679
2680 static void free_mr_exit(struct hns_roce_dev *hr_dev)
2681 {
2682 struct hns_roce_v2_priv *priv = hr_dev->priv;
2683 struct hns_roce_v2_free_mr *free_mr = &priv->free_mr;
> 2684 struct ib_qp *qp;
2685 int i;
2686
2687 for (i = 0; i < ARRAY_SIZE(free_mr->rsv_qp); i++)
2688 free_mr_uninit_qp(hr_dev, i);
2689
2690 free_mr_uninit_cq(hr_dev);
2691
2692 free_mr_uninit_pd(hr_dev);
2693 }
2694

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-12-06 14:27:17

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [PATCH for-rc 0/6] Bugfixes and improvements for hns RoCE

On Tue, Dec 05, 2023 at 10:05:46AM +0800, Junxian Huang wrote:
>
>
> On 2023/12/4 22:24, Leon Romanovsky wrote:
> > On Wed, Nov 29, 2023 at 05:44:28PM +0800, Junxian Huang wrote:
> >> Here are several bugfixes and improvements for hns RoCE.
> >>
> >> Chengchang Tang (4):
> >> RDMA/hns: Rename the interrupts
> >> RDMA/hns: Remove unnecessary checks for NULL in mtr_alloc_bufs()
> >> RDMA/hns: Fix memory leak in free_mr_init()
> >> RDMA/hns: Improve the readability of free mr uninit
> >
> > 1. The series doesn't apply.
> > ➜ kernel git:(wip/leon-for-next) ~/src/b4/b4.sh shazam -l -s https://lore.kernel.org/all/[email protected] -P 1-5
>
> Is this series going to be applied to -next?

Yes, I planned to apply them to -next, they don't really important Fixes for -rc4.

>
> > 2. Please drop patch #6 as you are deleting the code which you added in
> > first patches without actual gain.
>
> Is it better to drop it directly or merge it with the previous patch?

Please drop.

>
> Thanks,
> Junxian
>
> >
> > Thanks
> >
> >>
> >> Junxian Huang (2):
> >> RDMA/hns: Response dmac to userspace
> >> RDMA/hns: Add a max length of gid table
> >>
> >> drivers/infiniband/hw/hns/hns_roce_ah.c | 7 ++
> >> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 87 +++++++++++++++-------
> >> drivers/infiniband/hw/hns/hns_roce_mr.c | 2 +-
> >> include/uapi/rdma/hns-abi.h | 5 ++
> >> 4 files changed, 73 insertions(+), 28 deletions(-)
> >>
> >> --
> >> 2.30.0
> >>
> >>
> >