On Mon, Mar 18, 2019 at 05:34:33AM -0700, Michael Ruhl wrote:
> >-----Original Message-----
> >From: Weiny, Ira
> >Sent: Sunday, March 17, 2019 4:00 PM
> >To: Jason Gunthorpe <jgg@xxxxxxxx>; Steven Rostedt
> ><rostedt@xxxxxxxxxxx>
> >Cc: Ingo Molnar <mingo@xxxxxxxxxx>; linux-rdma@xxxxxxxxxxxxxxx; Weiny,
> >Ira <ira.weiny@xxxxxxxxx>; Hal Rosenstock <hal@xxxxxxxxxxxxxxxxxx>; Alexei
> >Starovoitov <ast@xxxxxxxxxx>; Leon Romanovsky <leon@xxxxxxxxxx>; Ruhl,
> >Michael J <michael.j.ruhl@xxxxxxxxx>
> >Subject: [PATCH V6 1/6] IB/MAD: Add send path trace points
> >
> >From: Ira Weiny <ira.weiny@xxxxxxxxx>
> >
> >Use the standard Linux trace mechanism to trace MADs being sent. 4 trace
> >points are added, when the MAD is posted to the qp, when the MAD is
> >completed, if a MAD is resent, and when the MAD completes in error.
> >
> >CC: Hal Rosenstock <hal@xxxxxxxxxxxxxxxxxx>
> >CC: Alexei Starovoitov <ast@xxxxxxxxxx>
> >CC: Leon Romanovsky <leon@xxxxxxxxxx>
> >CC: Jason Gunthorpe <jgg@xxxxxxxx>
> >CC: "Ruhl, Michael J" <michael.j.ruhl@xxxxxxxxx>
> >Suggested-by: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>
> >Signed-off-by: Ira Weiny <ira.weiny@xxxxxxxxx>
> >
> >---
> >Changes from V5:
> > Merge using the trace structe in create_mad_addr_info()
> > Fixed version for TRACEPOINTS off
> >
> >Changes from V1
> > Fix compilation when TRACEPOINTS are not configured
> >
> >Changes since v4:
> > Clean up for checkpatch
> > Remove helper function trace_create_mad_addr and if check
> >
> >Changes since v3:
> > Change dev_name to dev_index
> >
> >Changes since v2:
> > Change license text to SPDX tag
> > Change dev_name to string from array
> > Reorder fields for more compact ring buffer utilization
> > Use a defined roce address type for safer memcpy/memset
> >
> >Changes since v1:
> > Update MAINTAINERS with tracing file
> >
> > MAINTAINERS | 1 +
> > drivers/infiniband/core/mad.c | 46 ++++++++-
> > include/trace/events/ib_mad.h | 187
> >++++++++++++++++++++++++++++++++++
> > 3 files changed, 233 insertions(+), 1 deletion(-)
> > create mode 100644 include/trace/events/ib_mad.h
> >
> >diff --git a/MAINTAINERS b/MAINTAINERS
> >index 8c68de3cfd80..9254bd40f1ae 100644
> >--- a/MAINTAINERS
> >+++ b/MAINTAINERS
> >@@ -7491,6 +7491,7 @@ F: drivers/infiniband/
> > F: include/uapi/linux/if_infiniband.h
> > F: include/uapi/rdma/
> > F: include/rdma/
> >+F: include/trace/events/ib_mad.h
> >
> > INGENIC JZ4780 DMA Driver
> > M: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@xxxxxxxxxx>
> >diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
> >index e742a6a2c138..92133b2c5072 100644
> >--- a/drivers/infiniband/core/mad.c
> >+++ b/drivers/infiniband/core/mad.c
> >@@ -3,7 +3,7 @@
> > * Copyright (c) 2005 Intel Corporation. All rights reserved.
> > * Copyright (c) 2005 Mellanox Technologies Ltd. All rights reserved.
> > * Copyright (c) 2009 HNR Consulting. All rights reserved.
> >- * Copyright (c) 2014 Intel Corporation. All rights reserved.
> >+ * Copyright (c) 2014,2018 Intel Corporation. All rights reserved.
> > *
> > * This software is available to you under a choice of one of two
> > * licenses. You may choose to be licensed under the terms of the GNU
> >@@ -51,6 +51,45 @@
> > #include "opa_smi.h"
> > #include "agent.h"
> >
> >+#define CREATE_TRACE_POINTS
> >+#include <trace/events/ib_mad.h>
> >+
> >+#ifdef CONFIG_TRACEPOINTS
> >+static void create_mad_addr_info(struct ib_mad_send_wr_private
> >*mad_send_wr,
> >+ struct ib_mad_qp_info *qp_info,
> >+ struct trace_event_raw_ib_mad_send_template
> >*entry)
> >+{
> >+ u16 pkey;
> >+ struct ib_device *dev = qp_info->port_priv->device;
> >+ u8 pnum = qp_info->port_priv->port_num;
> >+ struct ib_ud_wr *wr = &mad_send_wr->send_wr;
> >+ struct rdma_ah_attr attr;
>
> attr = {};
> instead of:
>
> >+ memset(&attr, 0, sizeof(attr));
>
> ?
Not sure that matters.
>
> The rdmavt query_ah() effectively does a memcpy of the ah attr. Is the
> memset() even necessary? (not sure what other driver do).
I don't want to rely on the drivers.
>
> >+ rdma_query_ah(wr->ah, &attr);
> >+
> >+ /* These are common */
> >+ entry->sl = attr.sl;
> >+ ib_query_pkey(dev, pnum, wr->pkey_index, &pkey);
> >+ entry->pkey = pkey;
> >+ entry->rqpn = wr->remote_qpn;
> >+ entry->rqkey = wr->remote_qkey;
> >+
> >+ switch (attr.type) {
> >+ case RDMA_AH_ATTR_TYPE_IB:
> >+ entry->dlid = attr.ib.dlid;
> >+ break;
> >+ case RDMA_AH_ATTR_TYPE_OPA:
> >+ entry->dlid = attr.opa.dlid;
> >+ break;
> >+ case RDMA_AH_ATTR_TYPE_ROCE:
> >+ case RDMA_AH_ATTR_TYPE_UNDEFINED:
> >+ entry->dlid = 0;
> >+ break;
> >+ }
>
> rdma_ah_get_dlid()?
Sure that is cleaner.
Ira
>
> >+}
> >+#endif
> >+
> > static int mad_sendq_size = IB_MAD_QP_SEND_SIZE;
> > static int mad_recvq_size = IB_MAD_QP_RECV_SIZE;
> >
> >@@ -1223,6 +1262,7 @@ int ib_send_mad(struct ib_mad_send_wr_private
> >*mad_send_wr)
> >
> > spin_lock_irqsave(&qp_info->send_queue.lock, flags);
> > if (qp_info->send_queue.count < qp_info->send_queue.max_active)
> >{
> >+ trace_ib_mad_ib_send_mad(mad_send_wr, qp_info);
> > ret = ib_post_send(mad_agent->qp, &mad_send_wr-
> >>send_wr.wr,
> > NULL);
> > list = &qp_info->send_queue.list;
> >@@ -2496,6 +2536,8 @@ static void ib_mad_send_done(struct ib_cq *cq,
> >struct ib_wc *wc)
> > send_queue = mad_list->mad_queue;
> > qp_info = send_queue->qp_info;
> >
> >+ trace_ib_mad_send_done_handler(mad_send_wr, wc);
> >+
> > retry:
> > ib_dma_unmap_single(mad_send_wr->send_buf.mad_agent-
> >>device,
> > mad_send_wr->header_mapping,
> >@@ -2527,6 +2569,7 @@ static void ib_mad_send_done(struct ib_cq *cq,
> >struct ib_wc *wc)
> > ib_mad_complete_send_wr(mad_send_wr, &mad_send_wc);
> >
> > if (queued_send_wr) {
> >+ trace_ib_mad_send_done_resend(queued_send_wr,
> >qp_info);
> > ret = ib_post_send(qp_info->qp, &queued_send_wr-
> >>send_wr.wr,
> > NULL);
> > if (ret) {
> >@@ -2574,6 +2617,7 @@ static bool ib_mad_send_error(struct
> >ib_mad_port_private *port_priv,
> > if (mad_send_wr->retry) {
> > /* Repost send */
> > mad_send_wr->retry = 0;
> >+ trace_ib_mad_error_handler(mad_send_wr,
> >qp_info);
> > ret = ib_post_send(qp_info->qp, &mad_send_wr-
> >>send_wr.wr,
> > NULL);
> > if (!ret)
> >diff --git a/include/trace/events/ib_mad.h b/include/trace/events/ib_mad.h
> >new file mode 100644
> >index 000000000000..585028f17fa7
> >--- /dev/null
> >+++ b/include/trace/events/ib_mad.h
> >@@ -0,0 +1,187 @@
> >+/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
> >+
> >+/*
> >+ * Copyright (c) 2018 Intel Corporation. All rights reserved.
> >+ */
> >+
> >+#undef TRACE_SYSTEM
> >+#define TRACE_SYSTEM ib_mad
> >+
> >+#if !defined(_TRACE_IB_MAD_H) || defined(TRACE_HEADER_MULTI_READ)
> >+#define _TRACE_IB_MAD_H
> >+
> >+#include <linux/tracepoint.h>
> >+#include <rdma/ib_mad.h>
> >+
> >+#ifdef CONFIG_TRACEPOINTS
> >+struct trace_event_raw_ib_mad_send_template;
> >+static void create_mad_addr_info(struct ib_mad_send_wr_private
> >*mad_send_wr,
> >+ struct ib_mad_qp_info *qp_info,
> >+ struct trace_event_raw_ib_mad_send_template
> >*entry);
> >+#endif
> >+
> >+DECLARE_EVENT_CLASS(ib_mad_send_template,
> >+ TP_PROTO(struct ib_mad_send_wr_private *wr,
> >+ struct ib_mad_qp_info *qp_info),
> >+ TP_ARGS(wr, qp_info),
> >+
> >+ TP_STRUCT__entry(
> >+ __field(u8, base_version)
> >+ __field(u8, mgmt_class)
> >+ __field(u8, class_version)
> >+ __field(u8, port_num)
> >+ __field(u32, qp_num)
> >+ __field(u8, method)
> >+ __field(u8, sl)
> >+ __field(u16, attr_id)
> >+ __field(u32, attr_mod)
> >+ __field(u64, wrtid)
> >+ __field(u64, tid)
> >+ __field(u16, status)
> >+ __field(u16, class_specific)
> >+ __field(u32, length)
> >+ __field(u32, dlid)
> >+ __field(u32, rqpn)
> >+ __field(u32, rqkey)
> >+ __field(u32, dev_index)
> >+ __field(void *, agent_priv)
> >+ __field(unsigned long, timeout)
> >+ __field(int, retries_left)
> >+ __field(int, max_retries)
> >+ __field(int, retry)
> >+ __field(u16, pkey)
> >+ ),
> >+
> >+ TP_fast_assign(
> >+ __entry->dev_index = wr->mad_agent_priv->agent.device-
> >>index;
> >+ __entry->port_num = wr->mad_agent_priv-
> >>agent.port_num;
> >+ __entry->qp_num = wr->mad_agent_priv->qp_info->qp-
> >>qp_num;
> >+ __entry->agent_priv = wr->mad_agent_priv;
> >+ __entry->wrtid = wr->tid;
> >+ __entry->max_retries = wr->max_retries;
> >+ __entry->retries_left = wr->retries_left;
> >+ __entry->retry = wr->retry;
> >+ __entry->timeout = wr->timeout;
> >+ __entry->length = wr->send_buf.hdr_len +
> >+ wr->send_buf.data_len;
> >+ __entry->base_version =
> >+ ((struct ib_mad_hdr *)wr->send_buf.mad)-
> >>base_version;
> >+ __entry->mgmt_class =
> >+ ((struct ib_mad_hdr *)wr->send_buf.mad)-
> >>mgmt_class;
> >+ __entry->class_version =
> >+ ((struct ib_mad_hdr *)wr->send_buf.mad)-
> >>class_version;
> >+ __entry->method =
> >+ ((struct ib_mad_hdr *)wr->send_buf.mad)->method;
> >+ __entry->status =
> >+ ((struct ib_mad_hdr *)wr->send_buf.mad)->status;
> >+ __entry->class_specific =
> >+ ((struct ib_mad_hdr *)wr->send_buf.mad)-
> >>class_specific;
> >+ __entry->tid = ((struct ib_mad_hdr *)wr->send_buf.mad)-
> >>tid;
> >+ __entry->attr_id =
> >+ ((struct ib_mad_hdr *)wr->send_buf.mad)->attr_id;
> >+ __entry->attr_mod =
> >+ ((struct ib_mad_hdr *)wr->send_buf.mad)-
> >>attr_mod;
> >+ create_mad_addr_info(wr, qp_info, __entry);
> >+ ),
> >+
> >+ TP_printk("%d:%d QP%d agent %p: " \
> >+ "wrtid 0x%llx; %d/%d retries(%d); timeout %lu length %d : " \
> >+ "hdr : base_ver 0x%x class 0x%x class_ver 0x%x " \
> >+ "method 0x%x status 0x%x class_specific 0x%x tid 0x%llx " \
> >+ "attr_id 0x%x attr_mod 0x%x => dlid 0x%08x sl %d "\
> >+ "pkey 0x%x rpqn 0x%x rqpkey 0x%x",
> >+ __entry->dev_index, __entry->port_num, __entry-
> >>qp_num,
> >+ __entry->agent_priv, be64_to_cpu(__entry->wrtid),
> >+ __entry->retries_left, __entry->max_retries,
> >+ __entry->retry, __entry->timeout, __entry->length,
> >+ __entry->base_version, __entry->mgmt_class,
> >+ __entry->class_version,
> >+ __entry->method, be16_to_cpu(__entry->status),
> >+ be16_to_cpu(__entry->class_specific),
> >+ be64_to_cpu(__entry->tid), be16_to_cpu(__entry->attr_id),
> >+ be32_to_cpu(__entry->attr_mod),
> >+ be32_to_cpu(__entry->dlid), __entry->sl, __entry->pkey,
> >+ __entry->rqpn, __entry->rqkey
> >+ )
> >+);
> >+
> >+DEFINE_EVENT(ib_mad_send_template, ib_mad_error_handler,
> >+ TP_PROTO(struct ib_mad_send_wr_private *wr,
> >+ struct ib_mad_qp_info *qp_info),
> >+ TP_ARGS(wr, qp_info));
> >+DEFINE_EVENT(ib_mad_send_template, ib_mad_ib_send_mad,
> >+ TP_PROTO(struct ib_mad_send_wr_private *wr,
> >+ struct ib_mad_qp_info *qp_info),
> >+ TP_ARGS(wr, qp_info));
> >+DEFINE_EVENT(ib_mad_send_template, ib_mad_send_done_resend,
> >+ TP_PROTO(struct ib_mad_send_wr_private *wr,
> >+ struct ib_mad_qp_info *qp_info),
> >+ TP_ARGS(wr, qp_info));
> >+
> >+TRACE_EVENT(ib_mad_send_done_handler,
> >+ TP_PROTO(struct ib_mad_send_wr_private *wr, struct ib_wc *wc),
> >+ TP_ARGS(wr, wc),
> >+
> >+ TP_STRUCT__entry(
> >+ __field(u8, port_num)
> >+ __field(u8, base_version)
> >+ __field(u8, mgmt_class)
> >+ __field(u8, class_version)
> >+ __field(u32, qp_num)
> >+ __field(u64, wrtid)
> >+ __field(u16, status)
> >+ __field(u16, wc_status)
> >+ __field(u32, length)
> >+ __field(void *, agent_priv)
> >+ __field(unsigned long, timeout)
> >+ __field(u32, dev_index)
> >+ __field(int, retries_left)
> >+ __field(int, max_retries)
> >+ __field(int, retry)
> >+ __field(u8, method)
> >+ ),
> >+
> >+ TP_fast_assign(
> >+ __entry->dev_index = wr->mad_agent_priv->agent.device-
> >>index;
> >+ __entry->port_num = wr->mad_agent_priv-
> >>agent.port_num;
> >+ __entry->qp_num = wr->mad_agent_priv->qp_info->qp-
> >>qp_num;
> >+ __entry->agent_priv = wr->mad_agent_priv;
> >+ __entry->wrtid = wr->tid;
> >+ __entry->max_retries = wr->max_retries;
> >+ __entry->retries_left = wr->retries_left;
> >+ __entry->retry = wr->retry;
> >+ __entry->timeout = wr->timeout;
> >+ __entry->base_version =
> >+ ((struct ib_mad_hdr *)wr->send_buf.mad)-
> >>base_version;
> >+ __entry->mgmt_class =
> >+ ((struct ib_mad_hdr *)wr->send_buf.mad)-
> >>mgmt_class;
> >+ __entry->class_version =
> >+ ((struct ib_mad_hdr *)wr->send_buf.mad)-
> >>class_version;
> >+ __entry->method =
> >+ ((struct ib_mad_hdr *)wr->send_buf.mad)->method;
> >+ __entry->status =
> >+ ((struct ib_mad_hdr *)wr->send_buf.mad)->status;
> >+ __entry->wc_status = wc->status;
> >+ __entry->length = wc->byte_len;
> >+ ),
> >+
> >+ TP_printk("%d:%d QP%d : SEND WC Status %d : agent %p: " \
> >+ "wrtid 0x%llx %d/%d retries(%d) timeout %lu length %d: " \
> >+ "hdr : base_ver 0x%x class 0x%x class_ver 0x%x " \
> >+ "method 0x%x status 0x%x",
> >+ __entry->dev_index, __entry->port_num, __entry-
> >>qp_num,
> >+ __entry->wc_status,
> >+ __entry->agent_priv, be64_to_cpu(__entry->wrtid),
> >+ __entry->retries_left, __entry->max_retries,
> >+ __entry->retry, __entry->timeout,
> >+ __entry->length,
> >+ __entry->base_version, __entry->mgmt_class,
> >+ __entry->class_version, __entry->method,
> >+ be16_to_cpu(__entry->status)
> >+ )
> >+);
> >+
> >+
> >+#endif /* _TRACE_IB_MAD_H */
> >+
> >+#include <trace/define_trace.h>
> >--
> >2.20.1
>
