Re: [PATCH v4 for-next] RDMA/hns: dump detailed driver-specific CQ information for hip08

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Leon:

Thanks a lot for your reply.

On 2018/11/29 15:42, Leon Romanovsky wrote:
> On Thu, Nov 29, 2018 at 03:30:14PM +0800, liyangyang (M) wrote:
>> Hi Leon:
>>
>> Thanks a lot for your reply.
>>
>> On 2018/11/28 20:55, Leon Romanovsky wrote:
>>> On Wed, Nov 28, 2018 at 08:21:29PM +0800, Tao Tian wrote:
>>>> This patch adds support of resource track for hip08 and take
>>>> dumping cq context state used for debugging as an example.
>>>> More resources track supports for hns driver will be added in future.
>>>>
>>>> The output should be as follows.
>>>> $ ./rdma res show cq dev hnseth0 -d
>>>> dev hnseth5 cqe 5555 users 2 poll-ctx WORKQUEUE pid 0 comm [ib_core] state 2 ceq
>>>> n 0 cqn 0 hopnum 1 pi 0 ci 0 maxcnt 0 period 0 cqe 0
>>>
>>> You have "cqe" twice, one from IB/core and another from your driver.
>>> It will blow mind to everyone who will see it.
>> "cqe" in my driver means "cqe_cnt", but I mistakenly abbreviated it as "cqe".
>> This name will be changed to "cnt" in patch v5.
>>
>> The output should be as follows.
>> $ ./rdma res show cq dev hnseth0 -d
>> dev hnseth5 cqe 1023 users 2 poll-ctx WORKQUEUE pid 0 comm [ib_core] state 2 ceq
>> n 0 cqn 0 hopnum 1 pi 0 ci 0 maxcnt 0 period 0 cnt 0
> 
> So what does it mean to see maxcnt? Why is it zero?

"maxcnt" is the number of cqe aggregations. If it is 0, it means that cqe is not
aggregated. This name has existed for a long time, and it really makes users
confused. Perhaps "coalesce_num" or "coalesce" is a more appropriate name.

if "maxcnt" is changed to "coalesce_num", the output should be as follows.
$ ./rdma res show cq dev hnseth5 -d
dev hnseth5 cqe 1023 users 2 poll-ctx WORKQUEUE pid 0 comm [ib_core] state 2 ceq
n 0 cqn 0 hop_num 1 pi 0 ci 0 coalesce_num 0 period 0 cnt 0

Thanks.

> Again, you need to try to think like user who will see the output and
> immediately understand the issues.
> 
> Thanks
> 
> 
>>
>> Thanks.
>>>
>>>>
>>>> Signed-off-by: Tao Tian <tiantao6@xxxxxxxxxx>
>>>> Signed-off-by: Yangyang Li <liyangyang20@xxxxxxxxxx>
>>>> ---
>>>> v3->v4:
>>>> 1.print values in the granularity of parameterable fields.
>>>>
>>>> v2->v3:
>>>> 1.Modify the output message.
>>>>
>>>> v1->v2:
>>>> 1.Fixed the warning about the SPDX tag.
>>>> 2.Add the output of rdmatool to the commit message.
>>>> 3.Modify the function pointer to register res.
>>>> ---
>>>>  drivers/infiniband/hw/hns/Makefile             |   4 +-
>>>>  drivers/infiniband/hw/hns/hns_roce_cmd.h       |   1 +
>>>>  drivers/infiniband/hw/hns/hns_roce_device.h    |   8 ++
>>>>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c     |  15 ++-
>>>>  drivers/infiniband/hw/hns/hns_roce_hw_v2.h     |   8 ++
>>>>  drivers/infiniband/hw/hns/hns_roce_hw_v2_dfx.c |  36 ++++++++
>>>>  drivers/infiniband/hw/hns/hns_roce_main.c      |   1 +
>>>>  drivers/infiniband/hw/hns/hns_roce_restrack.c  | 123 +++++++++++++++++++++++++
>>>>  8 files changed, 189 insertions(+), 7 deletions(-)
>>>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_hw_v2_dfx.c
>>>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_restrack.c
>>>>
>>>> diff --git a/drivers/infiniband/hw/hns/Makefile b/drivers/infiniband/hw/hns/Makefile
>>>> index 004c88b..e9c5d11 100644
>>>> --- a/drivers/infiniband/hw/hns/Makefile
>>>> +++ b/drivers/infiniband/hw/hns/Makefile
>>>> @@ -7,8 +7,8 @@ ccflags-y :=  -Idrivers/net/ethernet/hisilicon/hns3
>>>>  obj-$(CONFIG_INFINIBAND_HNS) += hns-roce.o
>>>>  hns-roce-objs := hns_roce_main.o hns_roce_cmd.o hns_roce_pd.o \
>>>>  	hns_roce_ah.o hns_roce_hem.o hns_roce_mr.o hns_roce_qp.o \
>>>> -	hns_roce_cq.o hns_roce_alloc.o hns_roce_db.o hns_roce_srq.o
>>>> +	hns_roce_cq.o hns_roce_alloc.o hns_roce_db.o hns_roce_srq.o hns_roce_restrack.o
>>>>  obj-$(CONFIG_INFINIBAND_HNS_HIP06) += hns-roce-hw-v1.o
>>>>  hns-roce-hw-v1-objs := hns_roce_hw_v1.o
>>>>  obj-$(CONFIG_INFINIBAND_HNS_HIP08) += hns-roce-hw-v2.o
>>>> -hns-roce-hw-v2-objs := hns_roce_hw_v2.o
>>>> +hns-roce-hw-v2-objs := hns_roce_hw_v2.o hns_roce_hw_v2_dfx.o
>>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_cmd.h b/drivers/infiniband/hw/hns/hns_roce_cmd.h
>>>> index 97c5f2c..dfa96ba 100644
>>>> --- a/drivers/infiniband/hw/hns/hns_roce_cmd.h
>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_cmd.h
>>>> @@ -53,6 +53,7 @@ enum {
>>>>  	HNS_ROCE_CMD_QUERY_QPC		= 0x42,
>>>>
>>>>  	HNS_ROCE_CMD_MODIFY_CQC		= 0x52,
>>>> +	HNS_ROCE_CMD_QUERY_CQC		= 0x53,
>>>>  	/* CQC BT commands */
>>>>  	HNS_ROCE_CMD_WRITE_CQC_BT0	= 0x10,
>>>>  	HNS_ROCE_CMD_WRITE_CQC_BT1	= 0x11,
>>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
>>>> index 2a66929..cb2486e 100644
>>>> --- a/drivers/infiniband/hw/hns/hns_roce_device.h
>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
>>>> @@ -856,6 +856,11 @@ struct hns_roce_work {
>>>>  	int sub_type;
>>>>  };
>>>>
>>>> +struct hns_roce_dfx_hw {
>>>> +	int (*query_cqc_info)(struct hns_roce_dev *hr_dev, u32 cqn,
>>>> +			      int *buffer);
>>>> +};
>>>> +
>>>>  struct hns_roce_hw {
>>>>  	int (*reset)(struct hns_roce_dev *hr_dev, bool enable);
>>>>  	int (*cmq_init)(struct hns_roce_dev *hr_dev);
>>>> @@ -967,6 +972,7 @@ struct hns_roce_dev {
>>>>  	const struct hns_roce_hw *hw;
>>>>  	void			*priv;
>>>>  	struct workqueue_struct *irq_workq;
>>>> +	const struct hns_roce_dfx_hw *dfx;
>>>>  };
>>>>
>>>>  static inline struct hns_roce_dev *to_hr_dev(struct ib_device *ib_dev)
>>>> @@ -1183,4 +1189,6 @@ int hns_get_gid_index(struct hns_roce_dev *hr_dev, u8 port, int gid_index);
>>>>  int hns_roce_init(struct hns_roce_dev *hr_dev);
>>>>  void hns_roce_exit(struct hns_roce_dev *hr_dev);
>>>>
>>>> +int hns_roce_fill_res_entry(struct sk_buff *msg,
>>>> +			     struct rdma_restrack_entry *res);
>>>>  #endif /* _HNS_ROCE_DEVICE_H */
>>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
>>>> index 82e2ace..7e5a7ba 100644
>>>> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
>>>> @@ -846,9 +846,9 @@ static void hns_roce_v2_cmq_exit(struct hns_roce_dev *hr_dev)
>>>>  	hns_roce_free_cmq_desc(hr_dev, &priv->cmq.crq);
>>>>  }
>>>>
>>>> -static void hns_roce_cmq_setup_basic_desc(struct hns_roce_cmq_desc *desc,
>>>> -					  enum hns_roce_opcode_type opcode,
>>>> -					  bool is_read)
>>>> +void hns_roce_cmq_setup_basic_desc(struct hns_roce_cmq_desc *desc,
>>>> +				    enum hns_roce_opcode_type opcode,
>>>> +				    bool is_read)
>>>>  {
>>>>  	memset((void *)desc, 0, sizeof(struct hns_roce_cmq_desc));
>>>>  	desc->opcode = cpu_to_le16(opcode);
>>>> @@ -892,8 +892,8 @@ static int hns_roce_cmq_csq_clean(struct hns_roce_dev *hr_dev)
>>>>  	return clean;
>>>>  }
>>>>
>>>> -static int hns_roce_cmq_send(struct hns_roce_dev *hr_dev,
>>>> -			     struct hns_roce_cmq_desc *desc, int num)
>>>> +int hns_roce_cmq_send(struct hns_roce_dev *hr_dev,
>>>> +		       struct hns_roce_cmq_desc *desc, int num)
>>>>  {
>>>>  	struct hns_roce_v2_priv *priv = (struct hns_roce_v2_priv *)hr_dev->priv;
>>>>  	struct hns_roce_v2_cmq_ring *csq = &priv->cmq.csq;
>>>> @@ -5877,6 +5877,10 @@ static int hns_roce_v2_post_srq_recv(struct ib_srq *ibsrq,
>>>>  	return ret;
>>>>  }
>>>>
>>>> +static const struct hns_roce_dfx_hw hns_roce_dfx_hw_v2 = {
>>>> +	.query_cqc_info = hns_roce_v2_query_cqc_info,
>>>> +};
>>>> +
>>>>  static const struct hns_roce_hw hns_roce_hw_v2 = {
>>>>  	.cmq_init = hns_roce_v2_cmq_init,
>>>>  	.cmq_exit = hns_roce_v2_cmq_exit,
>>>> @@ -5936,6 +5940,7 @@ static int hns_roce_hw_v2_get_cfg(struct hns_roce_dev *hr_dev,
>>>>  	}
>>>>
>>>>  	hr_dev->hw = &hns_roce_hw_v2;
>>>> +	hr_dev->dfx = &hns_roce_dfx_hw_v2;
>>>>  	hr_dev->sdb_offset = ROCEE_DB_SQ_L_0_REG;
>>>>  	hr_dev->odb_offset = hr_dev->sdb_offset;
>>>>
>>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
>>>> index e6e3d8fb..247f29a 100644
>>>> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
>>>> @@ -1767,4 +1767,12 @@ struct hns_roce_scc_ctx_clr_done {
>>>>  	__le32 rsv[5];
>>>>  };
>>>>
>>>> +int hns_roce_v2_query_cqc_info(struct hns_roce_dev *hr_dev, u32 cqn,
>>>> +				int *buffer);
>>>> +void hns_roce_cmq_setup_basic_desc(struct hns_roce_cmq_desc *desc,
>>>> +				    enum hns_roce_opcode_type opcode,
>>>> +				    bool is_read);
>>>> +int hns_roce_cmq_send(struct hns_roce_dev *hr_dev,
>>>> +		       struct hns_roce_cmq_desc *desc, int num);
>>>> +
>>>>  #endif
>>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2_dfx.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2_dfx.c
>>>> new file mode 100644
>>>> index 0000000..ef6412b
>>>> --- /dev/null
>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2_dfx.c
>>>> @@ -0,0 +1,36 @@
>>>> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +// Copyright (c) 2018 Hisilicon Limited.
>>>> +
>>>> +#include "hns_roce_device.h"
>>>> +#include "hns_roce_cmd.h"
>>>> +#include "hns_roce_hw_v2.h"
>>>> +
>>>> +int hns_roce_v2_query_cqc_info(struct hns_roce_dev *hr_dev, u32 cqn,
>>>> +				int *buffer)
>>>> +{
>>>> +	struct hns_roce_v2_cq_context *cq_context;
>>>> +	struct hns_roce_cmd_mailbox *mailbox;
>>>> +	int ret;
>>>> +
>>>> +	mailbox = hns_roce_alloc_cmd_mailbox(hr_dev);
>>>> +	if (IS_ERR(mailbox))
>>>> +		return PTR_ERR(mailbox);
>>>> +
>>>> +	cq_context = mailbox->buf;
>>>> +	ret = hns_roce_cmd_mbox(hr_dev, 0, mailbox->dma,
>>>> +		cqn, 0,
>>>> +		HNS_ROCE_CMD_QUERY_CQC,
>>>> +		HNS_ROCE_CMD_TIMEOUT_MSECS);
>>>> +	if (ret) {
>>>> +		dev_err(hr_dev->dev, "QUERY cqc cmd process error\n");
>>>> +		goto err_mailbox;
>>>> +	}
>>>> +
>>>> +	memcpy(buffer, cq_context, sizeof(*cq_context));
>>>> +
>>>> +err_mailbox:
>>>> +	hns_roce_free_cmd_mailbox(hr_dev, mailbox);
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c
>>>> index 68f22b8..228e759 100644
>>>> --- a/drivers/infiniband/hw/hns/hns_roce_main.c
>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_main.c
>>>> @@ -564,6 +564,7 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
>>>>  	/* OTHERS */
>>>>  	ib_dev->get_port_immutable	= hns_roce_port_immutable;
>>>>  	ib_dev->disassociate_ucontext	= hns_roce_disassociate_ucontext;
>>>> +	ib_dev->res.fill_res_entry	= hns_roce_fill_res_entry;
>>>>
>>>>  	ib_dev->driver_id = RDMA_DRIVER_HNS;
>>>>  	ret = ib_register_device(ib_dev, "hns_%d", NULL);
>>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_restrack.c b/drivers/infiniband/hw/hns/hns_roce_restrack.c
>>>> new file mode 100644
>>>> index 0000000..ca35289
>>>> --- /dev/null
>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_restrack.c
>>>> @@ -0,0 +1,123 @@
>>>> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +// Copyright (c) 2018 Hisilicon Limited.
>>>> +
>>>> +#include <rdma/rdma_cm.h>
>>>> +#include <rdma/restrack.h>
>>>> +#include <uapi/rdma/rdma_netlink.h>
>>>> +#include "hns_roce_common.h"
>>>> +#include "hns_roce_device.h"
>>>> +#include "hns_roce_hw_v2.h"
>>>> +
>>>> +static int hns_roce_fill_cq(struct sk_buff *msg,
>>>> +			     struct hns_roce_v2_cq_context *context)
>>>> +{
>>>> +	if (rdma_nl_put_driver_u32(msg, "state",
>>>> +				   roce_get_field(context->byte_4_pg_ceqn,
>>>> +				   V2_CQC_BYTE_4_ARM_ST_M,
>>>> +				   V2_CQC_BYTE_4_ARM_ST_S)))
>>>> +		goto err;
>>>> +
>>>> +	if (rdma_nl_put_driver_u32(msg, "ceqn",
>>>> +				   roce_get_field(context->byte_4_pg_ceqn,
>>>> +				   V2_CQC_BYTE_4_CEQN_M,
>>>> +				   V2_CQC_BYTE_4_CEQN_S)))
>>>> +		goto err;
>>>> +
>>>> +	if (rdma_nl_put_driver_u32(msg, "cqn",
>>>> +				   roce_get_field(context->byte_8_cqn,
>>>> +				   V2_CQC_BYTE_8_CQN_M,
>>>> +				   V2_CQC_BYTE_8_CQN_S)))
>>>> +		goto err;
>>>> +
>>>> +	if (rdma_nl_put_driver_u32(msg, "hopnum",
>>>> +				   roce_get_field(context->byte_16_hop_addr,
>>>> +				   V2_CQC_BYTE_16_CQE_HOP_NUM_M,
>>>> +				   V2_CQC_BYTE_16_CQE_HOP_NUM_S)))
>>>> +		goto err;
>>>> +
>>>> +	if (rdma_nl_put_driver_u32(msg, "pi",
>>>> +				   roce_get_field(context->byte_28_cq_pi,
>>>> +				   V2_CQC_BYTE_28_CQ_PRODUCER_IDX_M,
>>>> +				   V2_CQC_BYTE_28_CQ_PRODUCER_IDX_S)))
>>>> +		goto err;
>>>> +
>>>> +	if (rdma_nl_put_driver_u32(msg, "ci",
>>>> +				   roce_get_field(context->byte_32_cq_ci,
>>>> +				   V2_CQC_BYTE_16_CQE_HOP_NUM_M,
>>>> +				   V2_CQC_BYTE_16_CQE_HOP_NUM_S)))
>>>> +		goto err;
>>>> +
>>>> +	if (rdma_nl_put_driver_u32(msg, "maxcnt",
>>>> +				   roce_get_field(
>>>> +				   context->byte_56_cqe_period_maxcnt,
>>>> +				   V2_CQC_BYTE_56_CQ_MAX_CNT_M,
>>>> +				   V2_CQC_BYTE_56_CQ_MAX_CNT_S)))
>>>> +		goto err;
>>>> +
>>>> +	if (rdma_nl_put_driver_u32(msg, "period",
>>>> +				   roce_get_field(
>>>> +				   context->byte_56_cqe_period_maxcnt,
>>>> +				   V2_CQC_BYTE_56_CQ_PERIOD_M,
>>>> +				   V2_CQC_BYTE_56_CQ_PERIOD_S)))
>>>> +		goto err;
>>>> +
>>>> +	if (rdma_nl_put_driver_u32(msg, "cqe",
>>>> +				   roce_get_field(context->byte_52_cqe_cnt,
>>>> +				   V2_CQC_BYTE_52_CQE_CNT_M,
>>>> +				   V2_CQC_BYTE_52_CQE_CNT_S)))
>>>> +		goto err;
>>>> +
>>>> +	return 0;
>>>> +
>>>> +err:
>>>> +	return -EMSGSIZE;
>>>> +}
>>>> +
>>>> +static int hns_roce_fill_res_cq_entry(struct sk_buff *msg,
>>>> +				       struct rdma_restrack_entry *res)
>>>> +{
>>>> +	struct ib_cq  *ib_cq = container_of(res, struct ib_cq, res);
>>>> +	struct hns_roce_dev *hr_dev = to_hr_dev(ib_cq->device);
>>>> +	struct hns_roce_cq *hr_cq = to_hr_cq(ib_cq);
>>>> +	struct hns_roce_v2_cq_context *context;
>>>> +	struct nlattr *table_attr;
>>>> +	int ret;
>>>> +
>>>> +	if (!hr_dev->dfx->query_cqc_info)
>>>> +		return -EINVAL;
>>>> +
>>>> +	context = kzalloc(sizeof(struct hns_roce_v2_cq_context), GFP_KERNEL);
>>>> +	if (!context)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	ret = hr_dev->dfx->query_cqc_info(hr_dev, hr_cq->cqn, (int *)context);
>>>> +	if (ret)
>>>> +		goto err;
>>>> +
>>>> +	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
>>>> +	if (!table_attr)
>>>> +		goto err;
>>>> +
>>>> +	if (hns_roce_fill_cq(msg, context))
>>>> +		goto err_cancel_table;
>>>> +
>>>> +	nla_nest_end(msg, table_attr);
>>>> +	kfree(context);
>>>> +
>>>> +	return 0;
>>>> +
>>>> +err_cancel_table:
>>>> +	nla_nest_cancel(msg, table_attr);
>>>> +err:
>>>> +	kfree(context);
>>>> +	return -EMSGSIZE;
>>>> +}
>>>> +
>>>> +int hns_roce_fill_res_entry(struct sk_buff *msg,
>>>> +			     struct rdma_restrack_entry *res)
>>>> +{
>>>> +	if (res->type == RDMA_RESTRACK_CQ)
>>>> +		return hns_roce_fill_res_cq_entry(msg, res);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> --
>>>> 2.7.4
>>>>
>>




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux