diff mbox series

[V6,4/6] IB/UMAD: Add umad trace points

Message ID 20190317195950.2991-5-ira.weiny@intel.com (mailing list archive)
State Superseded
Headers show
Series Add MAD stack trace points | expand

Commit Message

Ira Weiny March 17, 2019, 7:59 p.m. UTC
From: Ira Weiny <ira.weiny@intel.com>

Trace MADs going to/from user space.

CC: Hal Rosenstock <hal@dev.mellanox.co.il>
CC: Alexei Starovoitov <ast@kernel.org>
CC: Leon Romanovsky <leon@kernel.org>
CC: Jason Gunthorpe <jgg@ziepe.ca>
CC: "Ruhl, Michael J" <michael.j.ruhl@intel.com>
CC: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>

---
Changes from v4
	Clean up checkpatch

Changes from v3
	Change dev_name to dev_inde

Changes from v2
	Rebased on current RDMA for next
	convert dev_name to string
	convert license to SPDX
	Reorder fields for better ring buffer utilization

Changes from v1
	Update MAINTAINERS file

 MAINTAINERS                        |   1 +
 drivers/infiniband/core/user_mad.c |   7 ++
 include/trace/events/ib_umad.h     | 122 +++++++++++++++++++++++++++++
 3 files changed, 130 insertions(+)
 create mode 100644 include/trace/events/ib_umad.h

Comments

Ira Weiny March 18, 2019, 9:10 a.m. UTC | #1
On Mon, Mar 18, 2019 at 11:45:43AM -0400, Steven Rostedt wrote:
> On Mon, 18 Mar 2019 11:44:00 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Sun, 17 Mar 2019 12:59:48 -0700
> > ira.weiny@intel.com wrote:
> > 
> > > From: Ira Weiny <ira.weiny@intel.com>
> > > 
> > > Trace MADs going to/from user space.
> > > 
> > > CC: Hal Rosenstock <hal@dev.mellanox.co.il>
> > > CC: Alexei Starovoitov <ast@kernel.org>
> > > CC: Leon Romanovsky <leon@kernel.org>
> > > CC: Jason Gunthorpe <jgg@ziepe.ca>
> > > CC: "Ruhl, Michael J" <michael.j.ruhl@intel.com>
> > > CC: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > > Signed-off-by: Ira Weiny <ira.weiny@intel.com>  
> > 
> > For the tracing aspect.
> > 
> > Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > 
> 
> Note, it needs to have the __user fixed. So I take this back.

Yea I was not sure about that and how best to fix it...

Ira

> 
> -- Steve
Michael J. Ruhl March 18, 2019, 12:50 p.m. UTC | #2
>-----Original Message-----
>From: Weiny, Ira
>Sent: Sunday, March 17, 2019 4:00 PM
>To: Jason Gunthorpe <jgg@ziepe.ca>; Steven Rostedt
><rostedt@goodmis.org>
>Cc: Ingo Molnar <mingo@redhat.com>; linux-rdma@vger.kernel.org; Weiny,
>Ira <ira.weiny@intel.com>; Hal Rosenstock <hal@dev.mellanox.co.il>; Alexei
>Starovoitov <ast@kernel.org>; Leon Romanovsky <leon@kernel.org>; Ruhl,
>Michael J <michael.j.ruhl@intel.com>
>Subject: [PATCH V6 4/6] IB/UMAD: Add umad trace points
>
>From: Ira Weiny <ira.weiny@intel.com>
>
>Trace MADs going to/from user space.
>
>CC: Hal Rosenstock <hal@dev.mellanox.co.il>
>CC: Alexei Starovoitov <ast@kernel.org>
>CC: Leon Romanovsky <leon@kernel.org>
>CC: Jason Gunthorpe <jgg@ziepe.ca>
>CC: "Ruhl, Michael J" <michael.j.ruhl@intel.com>
>CC: Steven Rostedt (VMware) <rostedt@goodmis.org>
>Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>
>---
>Changes from v4
>	Clean up checkpatch
>
>Changes from v3
>	Change dev_name to dev_inde
>
>Changes from v2
>	Rebased on current RDMA for next
>	convert dev_name to string
>	convert license to SPDX
>	Reorder fields for better ring buffer utilization
>
>Changes from v1
>	Update MAINTAINERS file
>
> MAINTAINERS                        |   1 +
> drivers/infiniband/core/user_mad.c |   7 ++
> include/trace/events/ib_umad.h     | 122
>+++++++++++++++++++++++++++++
> 3 files changed, 130 insertions(+)
> create mode 100644 include/trace/events/ib_umad.h
>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index 9254bd40f1ae..18d42f6521dc 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -7492,6 +7492,7 @@ F:	include/uapi/linux/if_infiniband.h
> F:	include/uapi/rdma/
> F:	include/rdma/
> F:	include/trace/events/ib_mad.h
>+F:	include/trace/events/ib_umad.h
>
> INGENIC JZ4780 DMA Driver
> M:	Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
>diff --git a/drivers/infiniband/core/user_mad.c
>b/drivers/infiniband/core/user_mad.c
>index 02b7947ab215..84158dab1163 100644
>--- a/drivers/infiniband/core/user_mad.c
>+++ b/drivers/infiniband/core/user_mad.c
>@@ -129,6 +129,9 @@ struct ib_umad_packet {
> 	struct ib_user_mad mad;
> };
>
>+#define CREATE_TRACE_POINTS
>+#include <trace/events/ib_umad.h>
>+
> static const dev_t base_umad_dev = MKDEV(IB_UMAD_MAJOR,
>IB_UMAD_MINOR_BASE);
> static const dev_t base_issm_dev = MKDEV(IB_UMAD_MAJOR,
>IB_UMAD_MINOR_BASE) +
> 				   IB_UMAD_NUM_FIXED_MINOR;
>@@ -391,6 +394,8 @@ static ssize_t ib_umad_read(struct file *filp, char
>__user *buf,
> 	else
> 		ret = copy_send_mad(file, buf, packet, count);
>
>+	trace_ib_umad_read(file, (struct ib_user_mad __user *)buf);

buf is marked as __user.  Doesn't that mean that you have to use
copy_from_user(), in order to be safe?

> 	if (ret < 0) {
> 		/* Requeue packet */
> 		mutex_lock(&file->mutex);
>@@ -508,6 +513,8 @@ static ssize_t ib_umad_write(struct file *filp, const char
>__user *buf,
>
> 	mutex_lock(&file->mutex);
>
>+	trace_ib_umad_write(file, &packet->mad);
>+
> 	agent = __get_agent(file, packet->mad.hdr.id);
> 	if (!agent) {
> 		ret = -EINVAL;
>diff --git a/include/trace/events/ib_umad.h
>b/include/trace/events/ib_umad.h
>new file mode 100644
>index 000000000000..e177c5c0ee7d
>--- /dev/null
>+++ b/include/trace/events/ib_umad.h
>@@ -0,0 +1,122 @@
>+/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
>+
>+/*
>+ * Copyright (c) 2018 Intel Corporation.  All rights reserved.
>+ *
>+ */
>+
>+#undef TRACE_SYSTEM
>+#define TRACE_SYSTEM ib_umad
>+
>+#if !defined(_TRACE_IB_UMAD_H) ||
>defined(TRACE_HEADER_MULTI_READ)
>+#define _TRACE_IB_UMAD_H
>+
>+#include <linux/tracepoint.h>
>+
>+DECLARE_EVENT_CLASS(ib_umad_template,
>+	TP_PROTO(struct ib_umad_file *file, struct ib_user_mad *mad),
>+	TP_ARGS(file, mad),
>+
>+	TP_STRUCT__entry(
>+		__field(u8,             port_num)
>+		__field(u8,             sl)
>+		__field(u8,             path_bits)
>+		__field(u8,             grh_present)
>+		__field(u32,            id)
>+		__field(u32,            status)
>+		__field(u32,            timeout_ms)
>+		__field(u32,            retires)
>+		__field(u32,            length)
>+		__field(u32,            qpn)
>+		__field(u32,            qkey)
>+		__field(u8,             gid_index)
>+		__field(u8,             hop_limit)
>+		__field(u16,            lid)
>+		__field(u16,            attr_id)
>+		__field(u16,            pkey_index)
>+		__field(u8,             base_version)
>+		__field(u8,             mgmt_class)
>+		__field(u8,             class_version)
>+		__field(u8,             method)
>+		__field(u32,            flow_label)
>+		__field(u16,            mad_status)
>+		__field(u16,            class_specific)
>+		__field(u32,            attr_mod)
>+		__field(u64,            tid)
>+		__array(u8,             gid, 16)
>+		__field(u32,            dev_index)
>+		__field(u8,             traffic_class)
>+	),
>+
>+	TP_fast_assign(
>+		__entry->dev_index = file->port->ib_dev->index;
>+		__entry->port_num = file->port->port_num;
>+
>+		__entry->id = mad->hdr.id;
>+		__entry->status = mad->hdr.status;
>+		__entry->timeout_ms = mad->hdr.timeout_ms;
>+		__entry->retires = mad->hdr.retries;
>+		__entry->length = mad->hdr.length;
>+		__entry->qpn = mad->hdr.qpn;
>+		__entry->qkey = mad->hdr.qkey;
>+		__entry->lid = mad->hdr.lid;
>+		__entry->sl = mad->hdr.sl;
>+		__entry->path_bits = mad->hdr.path_bits;
>+		__entry->grh_present = mad->hdr.grh_present;
>+		__entry->gid_index = mad->hdr.gid_index;
>+		__entry->hop_limit = mad->hdr.hop_limit;
>+		__entry->traffic_class = mad->hdr.traffic_class;
>+		memcpy(__entry->gid, mad->hdr.gid, sizeof(mad->hdr.gid));
>+		__entry->flow_label = mad->hdr.flow_label;
>+		__entry->pkey_index = mad->hdr.pkey_index;
>+
>+		__entry->base_version =
>+			((struct ib_mad_hdr *)mad->data)->base_version;
>+		__entry->mgmt_class =
>+			((struct ib_mad_hdr *)mad->data)->mgmt_class;
>+		__entry->class_version =
>+			((struct ib_mad_hdr *)mad->data)->class_version;
>+		__entry->method = ((struct ib_mad_hdr *)mad->data)-
>>method;
>+		__entry->mad_status = ((struct ib_mad_hdr *)mad->data)-
>>status;
>+		__entry->class_specific =
>+			((struct ib_mad_hdr *)mad->data)->class_specific;
>+		__entry->tid = ((struct ib_mad_hdr *)mad->data)->tid;
>+		__entry->attr_id = ((struct ib_mad_hdr *)mad->data)-
>>attr_id;
>+		__entry->attr_mod = ((struct ib_mad_hdr *)mad->data)-
>>attr_mod;
>+	),
>+
>+	TP_printk("%d:%d umad_hdr: id 0x%08x status 0x%08x ms %u ret %u
>" \
>+		  "len %u QP%u qkey 0x%08x lid 0x%04x sl %u path_bits 0x%x "
>\
>+		  "grh 0x%x gidi %u hop_lim %u traf_cl %u gid %pI6c " \
>+		  "flow 0x%08x pkeyi %u  MAD: base_ver 0x%x class 0x%x " \
>+		  "class_ver 0x%x method 0x%x status 0x%04x " \
>+		  "class_specific 0x%04x tid 0x%016llx attr_id 0x%04x " \
>+		  "attr_mod 0x%08x ",
>+		__entry->dev_index, __entry->port_num,
>+		__entry->id, __entry->status, __entry->timeout_ms,
>+		__entry->retires, __entry->length, be32_to_cpu(__entry-
>>qpn),
>+		be32_to_cpu(__entry->qkey), be16_to_cpu(__entry->lid),
>+		__entry->sl, __entry->path_bits, __entry->grh_present,
>+		__entry->gid_index, __entry->hop_limit,
>+		__entry->traffic_class, &__entry->gid,
>+		be32_to_cpu(__entry->flow_label), __entry->pkey_index,
>+		__entry->base_version, __entry->mgmt_class,
>+		__entry->class_version, __entry->method,
>+		be16_to_cpu(__entry->mad_status),
>+		be16_to_cpu(__entry->class_specific),
>+		be64_to_cpu(__entry->tid), be16_to_cpu(__entry->attr_id),
>+		be32_to_cpu(__entry->attr_mod)
>+	)
>+);
>+
>+DEFINE_EVENT(ib_umad_template, ib_umad_write,
>+	TP_PROTO(struct ib_umad_file *file, struct ib_user_mad *mad),
>+	TP_ARGS(file, mad));
>+
>+DEFINE_EVENT(ib_umad_template, ib_umad_read,
>+	TP_PROTO(struct ib_umad_file *file, struct ib_user_mad __user
>*mad),
>+	TP_ARGS(file, mad));
>+
>+#endif /* _TRACE_IB_UMAD_H */
>+
>+#include <trace/define_trace.h>
>--
>2.20.1
Steven Rostedt March 18, 2019, 1:41 p.m. UTC | #3
On Mon, 18 Mar 2019 12:50:37 +0000
"Ruhl, Michael J" <michael.j.ruhl@intel.com> wrote:

> >@@ -391,6 +394,8 @@ static ssize_t ib_umad_read(struct file *filp, char
> >__user *buf,
> > 	else
> > 		ret = copy_send_mad(file, buf, packet, count);
> >
> >+	trace_ib_umad_read(file, (struct ib_user_mad __user *)buf);  
> 
> buf is marked as __user.  Doesn't that mean that you have to use
> copy_from_user(), in order to be safe?

Good catch. I believe you are correct.

This patch looks to be broken.

-- Steve

> 
> > 	if (ret < 0) {
> > 		/* Requeue packet */
> > 		mutex_lock(&file->mutex);
> >@@ -508,6 +513,8 @@ static ssize_t ib_umad_write(struct file *filp, const char
> >__user *buf,
> >
Steven Rostedt March 18, 2019, 3:44 p.m. UTC | #4
On Sun, 17 Mar 2019 12:59:48 -0700
ira.weiny@intel.com wrote:

> From: Ira Weiny <ira.weiny@intel.com>
> 
> Trace MADs going to/from user space.
> 
> CC: Hal Rosenstock <hal@dev.mellanox.co.il>
> CC: Alexei Starovoitov <ast@kernel.org>
> CC: Leon Romanovsky <leon@kernel.org>
> CC: Jason Gunthorpe <jgg@ziepe.ca>
> CC: "Ruhl, Michael J" <michael.j.ruhl@intel.com>
> CC: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>

For the tracing aspect.

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve
Steven Rostedt March 18, 2019, 3:45 p.m. UTC | #5
On Mon, 18 Mar 2019 11:44:00 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Sun, 17 Mar 2019 12:59:48 -0700
> ira.weiny@intel.com wrote:
> 
> > From: Ira Weiny <ira.weiny@intel.com>
> > 
> > Trace MADs going to/from user space.
> > 
> > CC: Hal Rosenstock <hal@dev.mellanox.co.il>
> > CC: Alexei Starovoitov <ast@kernel.org>
> > CC: Leon Romanovsky <leon@kernel.org>
> > CC: Jason Gunthorpe <jgg@ziepe.ca>
> > CC: "Ruhl, Michael J" <michael.j.ruhl@intel.com>
> > CC: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>  
> 
> For the tracing aspect.
> 
> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> 

Note, it needs to have the __user fixed. So I take this back.

-- Steve
Steven Rostedt March 18, 2019, 5:38 p.m. UTC | #6
On Mon, 18 Mar 2019 02:10:07 -0700
Ira Weiny <ira.weiny@intel.com> wrote:

> On Mon, Mar 18, 2019 at 11:45:43AM -0400, Steven Rostedt wrote:
> > On Mon, 18 Mar 2019 11:44:00 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> >   
> > > On Sun, 17 Mar 2019 12:59:48 -0700
> > > ira.weiny@intel.com wrote:
> > >   
> > > > From: Ira Weiny <ira.weiny@intel.com>
> > > > 
> > > > Trace MADs going to/from user space.
> > > > 
> > > > CC: Hal Rosenstock <hal@dev.mellanox.co.il>
> > > > CC: Alexei Starovoitov <ast@kernel.org>
> > > > CC: Leon Romanovsky <leon@kernel.org>
> > > > CC: Jason Gunthorpe <jgg@ziepe.ca>
> > > > CC: "Ruhl, Michael J" <michael.j.ruhl@intel.com>
> > > > CC: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > > > Signed-off-by: Ira Weiny <ira.weiny@intel.com>    
> > > 
> > > For the tracing aspect.
> > > 
> > > Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > >   
> > 
> > Note, it needs to have the __user fixed. So I take this back.  
> 
> Yea I was not sure about that and how best to fix it...
>


	if (packet->recv_wc)
		ret = copy_recv_mad(file, buf, packet, count);
	else
		ret = copy_send_mad(file, buf, packet, count);

Looking at that code, in both cases buf contains the content of packet:

	if (copy_to_user(buf, &packet->mad, hdr_size(file)))
		return -EFAULT;

and the payload.

Why not use the contents of packet and payload?

I would also put in the ret into the tracepoint and make the tracepoint
a TRACE_EVENT_CONDITIONAL() and only record when ret is not negative.

Or just place the tracepoint in the copy_send/recv_mad() functions?

-- Steve
Ira Weiny March 19, 2019, 9:11 a.m. UTC | #7
On Mon, Mar 18, 2019 at 01:38:58PM -0400, Steven Rostedt wrote:
> On Mon, 18 Mar 2019 02:10:07 -0700
> Ira Weiny <ira.weiny@intel.com> wrote:
> 
> > On Mon, Mar 18, 2019 at 11:45:43AM -0400, Steven Rostedt wrote:
> > > On Mon, 18 Mar 2019 11:44:00 -0400
> > > Steven Rostedt <rostedt@goodmis.org> wrote:
> > >   
> > > > On Sun, 17 Mar 2019 12:59:48 -0700
> > > > ira.weiny@intel.com wrote:
> > > >   
> > > > > From: Ira Weiny <ira.weiny@intel.com>
> > > > > 
> > > > > Trace MADs going to/from user space.
> > > > > 
> > > > > CC: Hal Rosenstock <hal@dev.mellanox.co.il>
> > > > > CC: Alexei Starovoitov <ast@kernel.org>
> > > > > CC: Leon Romanovsky <leon@kernel.org>
> > > > > CC: Jason Gunthorpe <jgg@ziepe.ca>
> > > > > CC: "Ruhl, Michael J" <michael.j.ruhl@intel.com>
> > > > > CC: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > > > > Signed-off-by: Ira Weiny <ira.weiny@intel.com>    
> > > > 
> > > > For the tracing aspect.
> > > > 
> > > > Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > > >   
> > > 
> > > Note, it needs to have the __user fixed. So I take this back.  
> > 
> > Yea I was not sure about that and how best to fix it...
> >
> 
> 
> 	if (packet->recv_wc)
> 		ret = copy_recv_mad(file, buf, packet, count);
> 	else
> 		ret = copy_send_mad(file, buf, packet, count);
> 
> Looking at that code, in both cases buf contains the content of packet:
> 
> 	if (copy_to_user(buf, &packet->mad, hdr_size(file)))
> 		return -EFAULT;
> 
> and the payload.
> 
> Why not use the contents of packet and payload?

I want to say that is how I originally coded it.  But I don't know why I
changed it.  I just don't remember because the original code was written so
long ago.

I agree this is broken.  Sorry for being sloppy.  I just don't recall why I've
made this mistake and don't have the time right now to test it.  I should have
some time later this week.

> 
> I would also put in the ret into the tracepoint and make the tracepoint
> a TRACE_EVENT_CONDITIONAL() and only record when ret is not negative.
> 
> Or just place the tracepoint in the copy_send/recv_mad() functions?

Both good ideas.  If packet->mad is ok to use, which it should be, then I'd
like to record the info there as well as ret.  Again I think I need to spend
some time on this.

Jason, I'll throw up the other patches minus this one later today.

Thanks,
Ira

> 
> -- Steve
Ira Weiny March 19, 2019, 9:12 p.m. UTC | #8
On Mon, Mar 18, 2019 at 01:38:58PM -0400, Steven Rostedt wrote:
> On Mon, 18 Mar 2019 02:10:07 -0700
> Ira Weiny <ira.weiny@intel.com> wrote:
> 
> > On Mon, Mar 18, 2019 at 11:45:43AM -0400, Steven Rostedt wrote:
> > > On Mon, 18 Mar 2019 11:44:00 -0400
> > > Steven Rostedt <rostedt@goodmis.org> wrote:
> > >   
> > > > On Sun, 17 Mar 2019 12:59:48 -0700
> > > > ira.weiny@intel.com wrote:
> > > >   
> > > > > From: Ira Weiny <ira.weiny@intel.com>
> > > > > 
> > > > > Trace MADs going to/from user space.
> > > > > 
> > > > > CC: Hal Rosenstock <hal@dev.mellanox.co.il>
> > > > > CC: Alexei Starovoitov <ast@kernel.org>
> > > > > CC: Leon Romanovsky <leon@kernel.org>
> > > > > CC: Jason Gunthorpe <jgg@ziepe.ca>
> > > > > CC: "Ruhl, Michael J" <michael.j.ruhl@intel.com>
> > > > > CC: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > > > > Signed-off-by: Ira Weiny <ira.weiny@intel.com>    
> > > > 
> > > > For the tracing aspect.
> > > > 
> > > > Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > > >   
> > > 
> > > Note, it needs to have the __user fixed. So I take this back.  
> > 
> > Yea I was not sure about that and how best to fix it...
> >
> 
> 
> 	if (packet->recv_wc)
> 		ret = copy_recv_mad(file, buf, packet, count);
> 	else
> 		ret = copy_send_mad(file, buf, packet, count);
> 
> Looking at that code, in both cases buf contains the content of packet:
> 
> 	if (copy_to_user(buf, &packet->mad, hdr_size(file)))
> 		return -EFAULT;
> 
> and the payload.
> 
> Why not use the contents of packet and payload?
> 
> I would also put in the ret into the tracepoint and make the tracepoint
> a TRACE_EVENT_CONDITIONAL() and only record when ret is not negative.
> 
> Or just place the tracepoint in the copy_send/recv_mad() functions?

Yea that turned out to be easier because packet->mad was not all that was
needed.

V7 sent with this fix rather than splitting the series.  Just seemed more
reasonable to get it done...

Ira

> 
> -- Steve
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 9254bd40f1ae..18d42f6521dc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7492,6 +7492,7 @@  F:	include/uapi/linux/if_infiniband.h
 F:	include/uapi/rdma/
 F:	include/rdma/
 F:	include/trace/events/ib_mad.h
+F:	include/trace/events/ib_umad.h
 
 INGENIC JZ4780 DMA Driver
 M:	Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index 02b7947ab215..84158dab1163 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -129,6 +129,9 @@  struct ib_umad_packet {
 	struct ib_user_mad mad;
 };
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/ib_umad.h>
+
 static const dev_t base_umad_dev = MKDEV(IB_UMAD_MAJOR, IB_UMAD_MINOR_BASE);
 static const dev_t base_issm_dev = MKDEV(IB_UMAD_MAJOR, IB_UMAD_MINOR_BASE) +
 				   IB_UMAD_NUM_FIXED_MINOR;
@@ -391,6 +394,8 @@  static ssize_t ib_umad_read(struct file *filp, char __user *buf,
 	else
 		ret = copy_send_mad(file, buf, packet, count);
 
+	trace_ib_umad_read(file, (struct ib_user_mad __user *)buf);
+
 	if (ret < 0) {
 		/* Requeue packet */
 		mutex_lock(&file->mutex);
@@ -508,6 +513,8 @@  static ssize_t ib_umad_write(struct file *filp, const char __user *buf,
 
 	mutex_lock(&file->mutex);
 
+	trace_ib_umad_write(file, &packet->mad);
+
 	agent = __get_agent(file, packet->mad.hdr.id);
 	if (!agent) {
 		ret = -EINVAL;
diff --git a/include/trace/events/ib_umad.h b/include/trace/events/ib_umad.h
new file mode 100644
index 000000000000..e177c5c0ee7d
--- /dev/null
+++ b/include/trace/events/ib_umad.h
@@ -0,0 +1,122 @@ 
+/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
+
+/*
+ * Copyright (c) 2018 Intel Corporation.  All rights reserved.
+ *
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM ib_umad
+
+#if !defined(_TRACE_IB_UMAD_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_IB_UMAD_H
+
+#include <linux/tracepoint.h>
+
+DECLARE_EVENT_CLASS(ib_umad_template,
+	TP_PROTO(struct ib_umad_file *file, struct ib_user_mad *mad),
+	TP_ARGS(file, mad),
+
+	TP_STRUCT__entry(
+		__field(u8,             port_num)
+		__field(u8,             sl)
+		__field(u8,             path_bits)
+		__field(u8,             grh_present)
+		__field(u32,            id)
+		__field(u32,            status)
+		__field(u32,            timeout_ms)
+		__field(u32,            retires)
+		__field(u32,            length)
+		__field(u32,            qpn)
+		__field(u32,            qkey)
+		__field(u8,             gid_index)
+		__field(u8,             hop_limit)
+		__field(u16,            lid)
+		__field(u16,            attr_id)
+		__field(u16,            pkey_index)
+		__field(u8,             base_version)
+		__field(u8,             mgmt_class)
+		__field(u8,             class_version)
+		__field(u8,             method)
+		__field(u32,            flow_label)
+		__field(u16,            mad_status)
+		__field(u16,            class_specific)
+		__field(u32,            attr_mod)
+		__field(u64,            tid)
+		__array(u8,             gid, 16)
+		__field(u32,            dev_index)
+		__field(u8,             traffic_class)
+	),
+
+	TP_fast_assign(
+		__entry->dev_index = file->port->ib_dev->index;
+		__entry->port_num = file->port->port_num;
+
+		__entry->id = mad->hdr.id;
+		__entry->status = mad->hdr.status;
+		__entry->timeout_ms = mad->hdr.timeout_ms;
+		__entry->retires = mad->hdr.retries;
+		__entry->length = mad->hdr.length;
+		__entry->qpn = mad->hdr.qpn;
+		__entry->qkey = mad->hdr.qkey;
+		__entry->lid = mad->hdr.lid;
+		__entry->sl = mad->hdr.sl;
+		__entry->path_bits = mad->hdr.path_bits;
+		__entry->grh_present = mad->hdr.grh_present;
+		__entry->gid_index = mad->hdr.gid_index;
+		__entry->hop_limit = mad->hdr.hop_limit;
+		__entry->traffic_class = mad->hdr.traffic_class;
+		memcpy(__entry->gid, mad->hdr.gid, sizeof(mad->hdr.gid));
+		__entry->flow_label = mad->hdr.flow_label;
+		__entry->pkey_index = mad->hdr.pkey_index;
+
+		__entry->base_version =
+			((struct ib_mad_hdr *)mad->data)->base_version;
+		__entry->mgmt_class =
+			((struct ib_mad_hdr *)mad->data)->mgmt_class;
+		__entry->class_version =
+			((struct ib_mad_hdr *)mad->data)->class_version;
+		__entry->method = ((struct ib_mad_hdr *)mad->data)->method;
+		__entry->mad_status = ((struct ib_mad_hdr *)mad->data)->status;
+		__entry->class_specific =
+			((struct ib_mad_hdr *)mad->data)->class_specific;
+		__entry->tid = ((struct ib_mad_hdr *)mad->data)->tid;
+		__entry->attr_id = ((struct ib_mad_hdr *)mad->data)->attr_id;
+		__entry->attr_mod = ((struct ib_mad_hdr *)mad->data)->attr_mod;
+	),
+
+	TP_printk("%d:%d umad_hdr: id 0x%08x status 0x%08x ms %u ret %u " \
+		  "len %u QP%u qkey 0x%08x lid 0x%04x sl %u path_bits 0x%x " \
+		  "grh 0x%x gidi %u hop_lim %u traf_cl %u gid %pI6c " \
+		  "flow 0x%08x pkeyi %u  MAD: base_ver 0x%x class 0x%x " \
+		  "class_ver 0x%x method 0x%x status 0x%04x " \
+		  "class_specific 0x%04x tid 0x%016llx attr_id 0x%04x " \
+		  "attr_mod 0x%08x ",
+		__entry->dev_index, __entry->port_num,
+		__entry->id, __entry->status, __entry->timeout_ms,
+		__entry->retires, __entry->length, be32_to_cpu(__entry->qpn),
+		be32_to_cpu(__entry->qkey), be16_to_cpu(__entry->lid),
+		__entry->sl, __entry->path_bits, __entry->grh_present,
+		__entry->gid_index, __entry->hop_limit,
+		__entry->traffic_class, &__entry->gid,
+		be32_to_cpu(__entry->flow_label), __entry->pkey_index,
+		__entry->base_version, __entry->mgmt_class,
+		__entry->class_version, __entry->method,
+		be16_to_cpu(__entry->mad_status),
+		be16_to_cpu(__entry->class_specific),
+		be64_to_cpu(__entry->tid), be16_to_cpu(__entry->attr_id),
+		be32_to_cpu(__entry->attr_mod)
+	)
+);
+
+DEFINE_EVENT(ib_umad_template, ib_umad_write,
+	TP_PROTO(struct ib_umad_file *file, struct ib_user_mad *mad),
+	TP_ARGS(file, mad));
+
+DEFINE_EVENT(ib_umad_template, ib_umad_read,
+	TP_PROTO(struct ib_umad_file *file, struct ib_user_mad __user *mad),
+	TP_ARGS(file, mad));
+
+#endif /* _TRACE_IB_UMAD_H */
+
+#include <trace/define_trace.h>