[PATCH 00/17] Drivers: hv: vmbus: Add tracing to VMBus

[PATCH 00/17] Drivers: hv: vmbus: Add tracing to VMBus

[kys]

From: "K. Y. Srinivasan" <kys@microsoft.com>

Reply-To: kys@microsoft.com

Messages between guest and host are used in Hyper-V as control flow. To
simplify debugging various issues which are often hard to reproduce add
tracepoints to all message senders and handlers. This is not a
performance
critical path and tracing overhead should be negligible.

The example usage and output is:

Enable all tracing events:
# echo 1 > /sys/kernel/debug/tracing/events/hyperv/enable 

Do something which causes messages to be sent between host and guest,
e.g.
hot remove a VMBus device.

Check events:
# cat /sys/kernel/debug/tracing/trace 

# tracer: nop
#
#                              _-----=> irqs-off
#                             / _----=> need-resched
#                            | / _---=> hardirq/softirq
#                            || / _--=> preempt-depth
#                            ||| /     delay
#           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
#              | |       |   ||||       |         |
          <idle>-0     [011] ..s.   122.981583: vmbus_on_msg_dpc:
msgtype=1
    kworker/11:7-1506  [011] ....   122.981597: vmbus_on_message:
msgtype=1
    kworker/11:7-1506  [011] ....   122.981598: vmbus_onoffer:
child_relid 0x10, monitorid 0x2, is_dedicated 1, connection_id 0x10010,
if_type f8615163-df3e-46c5-913f-f2d2f965ed0e, if_instance
6676e078-e4b3-44da-8a7d-12eafb577d31, chn_flags 0x0, mmio_megabytes 0,
sub_channel_index 0
    kworker/11:7-1506  [011] ....   122.982130:
vmbus_establish_gpadl_header: sending child_relid 0x10, gpadl 0xe1e34,
range_buflen 2056 rangecount 1, ret 0
    kworker/11:7-1506  [011] ....   122.982133:
vmbus_establish_gpadl_body: sending msgnumber 0, gpadl 0xe1e34, ret 0
    kworker/11:7-1506  [011] ....   122.982136:
vmbus_establish_gpadl_body: sending msgnumber 0, gpadl 0xe1e34, ret 0
    kworker/11:7-1506  [011] ....   122.982137:
vmbus_establish_gpadl_body: sending msgnumber 0, gpadl 0xe1e34, ret 0
    kworker/11:7-1506  [011] ....   122.982139:
vmbus_establish_gpadl_body: sending msgnumber 0, gpadl 0xe1e34, ret 0
    kworker/11:7-1506  [011] ....   122.982141:
vmbus_establish_gpadl_body: sending msgnumber 0, gpadl 0xe1e34, ret 0
    kworker/11:7-1506  [011] ....   122.982142:
vmbus_establish_gpadl_body: sending msgnumber 0, gpadl 0xe1e34, ret 0
    kworker/11:7-1506  [011] ....   122.982144:
vmbus_establish_gpadl_body: sending msgnumber 0, gpadl 0xe1e34, ret 0
    kworker/11:7-1506  [011] ....   122.982146:
vmbus_establish_gpadl_body: sending msgnumber 0, gpadl 0xe1e34, ret 0
    kworker/11:7-1506  [011] ....   122.982148:
vmbus_establish_gpadl_body: sending msgnumber 0, gpadl 0xe1e34, ret 0
          <idle>-0     [011] ..s.   122.982336: vmbus_on_msg_dpc:
msgtype=10
          <idle>-0     [011] ..s.   122.982337: vmbus_ongpadl_created:
child_relid 0x10, gpadl 0xe1e34, creation_status 0
    kworker/11:7-1506  [011] ....   122.982351: vmbus_open: sending
child_relid 0x10, openid 16, gpadlhandle 0xe1e34, target_vp 0xb, offset
0x80, ret 0
     kworker/3:1-214   [003] ....   123.015007: vmbus_setevent: relid
0x5
          <idle>-0     [011] ..s.   123.029467: vmbus_on_msg_dpc:
msgtype=6
          <idle>-0     [011] ..s.   123.029470: vmbus_onopen_result:
child_relid 0x10, openid 16, status 0
    kworker/11:7-1506  [011] ....   123.029492: vmbus_setevent: relid
0x10
          <idle>-0     [011] d.h.   123.029533: vmbus_chan_sched: relid
0x10
    kworker/11:7-1506  [011] ....   123.029539: vmbus_setevent: relid
0x10

CHANNELMSG_UNLOAD/CHANNELMSG_UNLOAD_RESPONSE are not traced as these are
mostly used on crash.


Vitaly Kuznetsov (17):
  hyper-v: trace vmbus_on_msg_dpc()
  hyper-v: trace vmbus_on_message()
  hyper-v: trace vmbus_onoffer()
  hyper-v: trace vmbus_onoffer_rescind()
  hyper-v: trace vmbus_onopen_result()
  hyper-v: trace vmbus_ongpadl_created()
  hyper-v: trace vmbus_ongpadl_torndown()
  hyper-v: trace vmbus_onversion_response()
  hyper-v: trace vmbus_request_offers()
  hyper-v: trace vmbus_open()
  hyper-v: trace vmbus_close_internal()
  hyper-v: trace vmbus_establish_gpadl()
  hyper-v: trace vmbus_teardown_gpadl()
  hyper-v: trace vmbus_negotiate_version()
  hyper-v: trace vmbus_release_relid()
  hyper-v: trace vmbus_send_tl_connect_request()
  hyper-v: trace channel events

 drivers/hv/Makefile       |   4 +-
 drivers/hv/channel.c      |  21 ++-
 drivers/hv/channel_mgmt.c |  26 +++-
 drivers/hv/connection.c   |   5 +
 drivers/hv/hv_trace.c     |   4 +
 drivers/hv/hv_trace.h     | 327 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/hv/hyperv_vmbus.h |   2 +
 drivers/hv/vmbus_drv.c    |   4 +
 8 files changed, 388 insertions(+), 5 deletions(-)
 create mode 100644 drivers/hv/hv_trace.c
 create mode 100644 drivers/hv/hv_trace.h

-- 
2.14.1

[PATCH 01/17] hyper-v: trace vmbus_on_msg_dpc()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracing subsystem to Hyper-V VMBus module and add tracepoint
to vmbus_on_msg_dpc() which is called when we receive a message from host.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/Makefile       |  4 +++-
 drivers/hv/hv_trace.c     |  4 ++++
 drivers/hv/hv_trace.h     | 29 +++++++++++++++++++++++++++++
 drivers/hv/hyperv_vmbus.h |  2 ++
 drivers/hv/vmbus_drv.c    |  2 ++
 5 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 drivers/hv/hv_trace.c
 create mode 100644 drivers/hv/hv_trace.h

diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index 39c9b2c08d33..ad791e00230f 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -2,7 +2,9 @@ obj-$(CONFIG_HYPERV)		+= hv_vmbus.o
 obj-$(CONFIG_HYPERV_UTILS)	+= hv_utils.o
 obj-$(CONFIG_HYPERV_BALLOON)	+= hv_balloon.o
 
+CFLAGS_hv_trace.o = -I$(src)
+
 hv_vmbus-y := vmbus_drv.o \
 		 hv.o connection.o channel.o \
-		 channel_mgmt.o ring_buffer.o
+		 channel_mgmt.o ring_buffer.o hv_trace.o
 hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_fcopy.o hv_utils_transport.o
diff --git a/drivers/hv/hv_trace.c b/drivers/hv/hv_trace.c
new file mode 100644
index 000000000000..df47acd01a81
--- /dev/null
+++ b/drivers/hv/hv_trace.c
@@ -0,0 +1,4 @@
+#include "hyperv_vmbus.h"
+
+#define CREATE_TRACE_POINTS
+#include "hv_trace.h"
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
new file mode 100644
index 000000000000..9c2772922c76
--- /dev/null
+++ b/drivers/hv/hv_trace.h
@@ -0,0 +1,29 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM hyperv
+
+#if !defined(_HV_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _HV_TRACE_H
+
+#include <linux/tracepoint.h>
+
+DECLARE_EVENT_CLASS(vmbus_hdr_msg,
+	TP_PROTO(const struct vmbus_channel_message_header *hdr),
+	TP_ARGS(hdr),
+	TP_STRUCT__entry(__field(unsigned int, msgtype)),
+	TP_fast_assign(__entry->msgtype = hdr->msgtype;),
+	TP_printk("msgtype=%u", __entry->msgtype)
+);
+
+DEFINE_EVENT(vmbus_hdr_msg, vmbus_on_msg_dpc,
+	TP_PROTO(const struct vmbus_channel_message_header *hdr),
+	TP_ARGS(hdr)
+);
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE hv_trace
+#endif /* _HV_TRACE_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index de6f01df9592..22300ec7b556 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -31,6 +31,8 @@
 #include <linux/hyperv.h>
 #include <linux/interrupt.h>
 
+#include "hv_trace.h"
+
 /*
  * Timeout for services such as KVP and fcopy.
  */
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 8fe13cd76acf..8f9293a87c6b 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -835,6 +835,8 @@ void vmbus_on_msg_dpc(unsigned long data)
 
 	hdr = (struct vmbus_channel_message_header *)msg->u.payload;
 
+	trace_vmbus_on_msg_dpc(hdr);
+
 	if (hdr->msgtype >= CHANNELMSG_COUNT) {
 		WARN_ONCE(1, "unknown msgtype=%d\n", hdr->msgtype);
 		goto msg_handled;
-- 
2.14.1

[PATCH 02/17] hyper-v: trace vmbus_on_message()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to vmbus_on_message() which is called when we start
processing a blocking from work context.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c | 2 ++
 drivers/hv/hv_trace.h     | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index a8dc9a164631..71d4d68488d4 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1176,6 +1176,8 @@ void vmbus_onmessage(void *context)
 	hdr = (struct vmbus_channel_message_header *)msg->u.payload;
 	size = msg->header.payload_size;
 
+	trace_vmbus_on_message(hdr);
+
 	if (hdr->msgtype >= CHANNELMSG_COUNT) {
 		pr_err("Received invalid channel message type %d size %d\n",
 			   hdr->msgtype, size);
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 9c2772922c76..d432aba5df8a 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -19,6 +19,11 @@ DEFINE_EVENT(vmbus_hdr_msg, vmbus_on_msg_dpc,
 	TP_ARGS(hdr)
 );
 
+DEFINE_EVENT(vmbus_hdr_msg, vmbus_on_message,
+	TP_PROTO(const struct vmbus_channel_message_header *hdr),
+	TP_ARGS(hdr)
+);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 03/17] hyper-v: trace vmbus_onoffer()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to CHANNELMSG_OFFERCHANNEL handler.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c |  2 ++
 drivers/hv/hv_trace.h     | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 71d4d68488d4..7c11e17ad295 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -813,6 +813,8 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
 
 	offer = (struct vmbus_channel_offer_channel *)hdr;
 
+	trace_vmbus_onoffer(offer);
+
 	/* Allocate the channel object and save this offer. */
 	newchannel = alloc_channel();
 	if (!newchannel) {
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index d432aba5df8a..488b873b563e 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -24,6 +24,43 @@ DEFINE_EVENT(vmbus_hdr_msg, vmbus_on_message,
 	TP_ARGS(hdr)
 );
 
+TRACE_EVENT(vmbus_onoffer,
+	    TP_PROTO(const struct vmbus_channel_offer_channel *offer),
+	    TP_ARGS(offer),
+	    TP_STRUCT__entry(
+		    __field(u32, child_relid)
+		    __field(u8, monitorid)
+		    __field(u16, is_ddc_int)
+		    __field(u32, connection_id)
+		    __array(char, if_type, 16)
+		    __array(char, if_instance, 16)
+		    __field(u16, chn_flags)
+		    __field(u16, mmio_mb)
+		    __field(u16, sub_idx)
+		    ),
+	    TP_fast_assign(__entry->child_relid = offer->child_relid;
+			   __entry->monitorid = offer->monitorid;
+			   __entry->is_ddc_int = offer->is_dedicated_interrupt;
+			   __entry->connection_id = offer->connection_id;
+			   memcpy(__entry->if_type,
+				  &offer->offer.if_type.b, 16);
+			   memcpy(__entry->if_instance,
+				  &offer->offer.if_instance.b, 16);
+			   __entry->chn_flags = offer->offer.chn_flags;
+			   __entry->mmio_mb = offer->offer.mmio_megabytes;
+			   __entry->sub_idx = offer->offer.sub_channel_index;
+		    ),
+	    TP_printk("child_relid 0x%x, monitorid 0x%x, is_dedicated %d, "
+		      "connection_id 0x%x, if_type %pUl, if_instance %pUl, "
+		      "chn_flags 0x%x, mmio_megabytes %d, sub_channel_index %d",
+		      __entry->child_relid, __entry->monitorid,
+		      __entry->is_ddc_int, __entry->connection_id,
+		      __entry->if_type, __entry->if_instance,
+		      __entry->chn_flags, __entry->mmio_mb,
+		      __entry->sub_idx
+		    )
+	);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 04/17] hyper-v: trace vmbus_onoffer_rescind()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to CHANNELMSG_RESCIND_CHANNELOFFER handler.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c | 2 ++
 drivers/hv/hv_trace.h     | 8 ++++++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 7c11e17ad295..cfe5c9c6a87c 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -856,6 +856,8 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
 
 	rescind = (struct vmbus_channel_rescind_offer *)hdr;
 
+	trace_vmbus_onoffer_rescind(rescind);
+
 	/*
 	 * The offer msg and the corresponding rescind msg
 	 * from the host are guranteed to be ordered -
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 488b873b563e..dbbed1d1f327 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -61,6 +61,14 @@ TRACE_EVENT(vmbus_onoffer,
 		    )
 	);
 
+TRACE_EVENT(vmbus_onoffer_rescind,
+	    TP_PROTO(const struct vmbus_channel_rescind_offer *offer),
+	    TP_ARGS(offer),
+	    TP_STRUCT__entry(__field(u32, child_relid)),
+	    TP_fast_assign(__entry->child_relid = offer->child_relid),
+	    TP_printk("child_relid 0x%x", __entry->child_relid)
+	);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 05/17] hyper-v: trace vmbus_onopen_result()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to CHANNELMSG_OPENCHANNEL_RESULT handler.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c |  2 ++
 drivers/hv/hv_trace.h     | 17 +++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index cfe5c9c6a87c..c735abe01712 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -986,6 +986,8 @@ static void vmbus_onopen_result(struct vmbus_channel_message_header *hdr)
 
 	result = (struct vmbus_channel_open_result *)hdr;
 
+	trace_vmbus_onopen_result(result);
+
 	/*
 	 * Find the open msg, copy the result and signal/unblock the wait event
 	 */
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index dbbed1d1f327..9757c19d1c08 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -69,6 +69,23 @@ TRACE_EVENT(vmbus_onoffer_rescind,
 	    TP_printk("child_relid 0x%x", __entry->child_relid)
 	);
 
+TRACE_EVENT(vmbus_onopen_result,
+	    TP_PROTO(const struct vmbus_channel_open_result *result),
+	    TP_ARGS(result),
+	    TP_STRUCT__entry(
+		    __field(u32, child_relid)
+		    __field(u32, openid)
+		    __field(u32, status)
+		    ),
+	    TP_fast_assign(__entry->child_relid = result->child_relid;
+			   __entry->openid = result->openid;
+			   __entry->status = result->status;
+		    ),
+	    TP_printk("child_relid 0x%x, openid %d, status %d",
+		      __entry->child_relid,  __entry->openid,  __entry->status
+		    )
+	);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 06/17] hyper-v: trace vmbus_ongpadl_created()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to CHANNELMSG_GPADL_CREATED handler.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c |  2 ++
 drivers/hv/hv_trace.h     | 17 +++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index c735abe01712..4bff75449afb 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1032,6 +1032,8 @@ static void vmbus_ongpadl_created(struct vmbus_channel_message_header *hdr)
 
 	gpadlcreated = (struct vmbus_channel_gpadl_created *)hdr;
 
+	trace_vmbus_ongpadl_created(gpadlcreated);
+
 	/*
 	 * Find the establish msg, copy the result and signal/unblock the wait
 	 * event
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 9757c19d1c08..20734b7b341b 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -86,6 +86,23 @@ TRACE_EVENT(vmbus_onopen_result,
 		    )
 	);
 
+TRACE_EVENT(vmbus_ongpadl_created,
+	    TP_PROTO(const struct vmbus_channel_gpadl_created *gpadlcreated),
+	    TP_ARGS(gpadlcreated),
+	    TP_STRUCT__entry(
+		    __field(u32, child_relid)
+		    __field(u32, gpadl)
+		    __field(u32, status)
+		    ),
+	    TP_fast_assign(__entry->child_relid = gpadlcreated->child_relid;
+			   __entry->gpadl = gpadlcreated->gpadl;
+			   __entry->status = gpadlcreated->creation_status;
+		    ),
+	    TP_printk("child_relid 0x%x, gpadl 0x%x, creation_status %d",
+		      __entry->child_relid,  __entry->gpadl,  __entry->status
+		    )
+	);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 07/17] hyper-v: trace vmbus_ongpadl_torndown()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to CHANNELMSG_GPADL_TORNDOWN handler.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c | 2 ++
 drivers/hv/hv_trace.h     | 8 ++++++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 4bff75449afb..d1cb85257bf0 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1082,6 +1082,8 @@ static void vmbus_ongpadl_torndown(
 
 	gpadl_torndown = (struct vmbus_channel_gpadl_torndown *)hdr;
 
+	trace_vmbus_ongpadl_torndown(gpadl_torndown);
+
 	/*
 	 * Find the open msg, copy the result and signal/unblock the wait event
 	 */
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 20734b7b341b..84c08cdf7235 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -103,6 +103,14 @@ TRACE_EVENT(vmbus_ongpadl_created,
 		    )
 	);
 
+TRACE_EVENT(vmbus_ongpadl_torndown,
+	    TP_PROTO(const struct vmbus_channel_gpadl_torndown *gpadltorndown),
+	    TP_ARGS(gpadltorndown),
+	    TP_STRUCT__entry(__field(u32, gpadl)),
+	    TP_fast_assign(__entry->gpadl = gpadltorndown->gpadl),
+	    TP_printk("gpadl 0x%x", __entry->gpadl)
+	);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 08/17] hyper-v: trace vmbus_onversion_response()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to CHANNELMSG_VERSION_RESPONSE handler.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c |  3 +++
 drivers/hv/hv_trace.h     | 11 +++++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index d1cb85257bf0..c33b7412f303 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1127,6 +1127,9 @@ static void vmbus_onversion_response(
 	unsigned long flags;
 
 	version_response = (struct vmbus_channel_version_response *)hdr;
+
+	trace_vmbus_onversion_response(version_response);
+
 	spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
 
 	list_for_each_entry(msginfo, &vmbus_connection.chn_msg_list,
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 84c08cdf7235..2a046547107f 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -111,6 +111,17 @@ TRACE_EVENT(vmbus_ongpadl_torndown,
 	    TP_printk("gpadl 0x%x", __entry->gpadl)
 	);
 
+TRACE_EVENT(vmbus_onversion_response,
+	    TP_PROTO(const struct vmbus_channel_version_response *response),
+	    TP_ARGS(response),
+	    TP_STRUCT__entry(
+		    __field(u8, ver)
+		    ),
+	    TP_fast_assign(__entry->ver = response->version_supported;
+		    ),
+	    TP_printk("version_supported %d", __entry->ver)
+	);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 09/17] hyper-v: trace vmbus_request_offers()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to CHANNELMSG_REQUESTOFFERS sender.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c | 4 +++-
 drivers/hv/hv_trace.h     | 8 ++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index c33b7412f303..b27edff8796e 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1224,9 +1224,11 @@ int vmbus_request_offers(void)
 
 	msg->msgtype = CHANNELMSG_REQUESTOFFERS;
 
-
 	ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_message_header),
 			     true);
+
+	trace_vmbus_request_offers(ret);
+
 	if (ret != 0) {
 		pr_err("Unable to request offers - %d\n", ret);
 
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 2a046547107f..566ac0f2fe56 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -122,6 +122,14 @@ TRACE_EVENT(vmbus_onversion_response,
 	    TP_printk("version_supported %d", __entry->ver)
 	);
 
+TRACE_EVENT(vmbus_request_offers,
+	    TP_PROTO(int ret),
+	    TP_ARGS(ret),
+	    TP_STRUCT__entry(__field(int, ret)),
+	    TP_fast_assign(__entry->ret = ret),
+	    TP_printk("sending ret %d", __entry->ret)
+	);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 10/17] hyper-v: trace vmbus_open()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to CHANNELMSG_OPENCHANNEL sender.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel.c  |  2 ++
 drivers/hv/hv_trace.h | 27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index a406beb10dd0..739b3fe1e0fb 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -185,6 +185,8 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 	ret = vmbus_post_msg(open_msg,
 			     sizeof(struct vmbus_channel_open_channel), true);
 
+	trace_vmbus_open(open_msg, ret);
+
 	if (ret != 0) {
 		err = ret;
 		goto error_clean_msglist;
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 566ac0f2fe56..38fedb803bd8 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -130,6 +130,33 @@ TRACE_EVENT(vmbus_request_offers,
 	    TP_printk("sending ret %d", __entry->ret)
 	);
 
+TRACE_EVENT(vmbus_open,
+	    TP_PROTO(const struct vmbus_channel_open_channel *msg, int ret),
+	    TP_ARGS(msg, ret),
+	    TP_STRUCT__entry(
+		    __field(u32, child_relid)
+		    __field(u32, openid)
+		    __field(u32, gpadlhandle)
+		    __field(u32, target_vp)
+		    __field(u32, offset)
+		    __field(int, ret)
+		    ),
+	    TP_fast_assign(
+		    __entry->child_relid = msg->child_relid;
+		    __entry->openid = msg->openid;
+		    __entry->gpadlhandle = msg->ringbuffer_gpadlhandle;
+		    __entry->target_vp = msg->target_vp;
+		    __entry->offset = msg->downstream_ringbuffer_pageoffset;
+		    __entry->ret = ret;
+		    ),
+	    TP_printk("sending child_relid 0x%x, openid %d, "
+		      "gpadlhandle 0x%x, target_vp 0x%x, offset 0x%x, ret %d",
+		      __entry->child_relid,  __entry->openid,
+		      __entry->gpadlhandle, __entry->target_vp,
+		      __entry->offset, __entry->ret
+		    )
+	);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 11/17] hyper-v: trace vmbus_close_internal()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to CHANNELMSG_CLOSECHANNEL sender.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel.c  |  2 ++
 drivers/hv/hv_trace.h | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 739b3fe1e0fb..a827f43aa408 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -591,6 +591,8 @@ static int vmbus_close_internal(struct vmbus_channel *channel)
 	ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_close_channel),
 			     true);
 
+	trace_vmbus_close_internal(msg, ret);
+
 	if (ret) {
 		pr_err("Close failed: close post msg return is %d\n", ret);
 		/*
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 38fedb803bd8..302bd4e964f0 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -157,6 +157,21 @@ TRACE_EVENT(vmbus_open,
 		    )
 	);
 
+TRACE_EVENT(vmbus_close_internal,
+	    TP_PROTO(const struct vmbus_channel_close_channel *msg, int ret),
+	    TP_ARGS(msg, ret),
+	    TP_STRUCT__entry(
+		    __field(u32, child_relid)
+		    __field(int, ret)
+		    ),
+	    TP_fast_assign(
+		    __entry->child_relid = msg->child_relid;
+		    __entry->ret = ret;
+		    ),
+	    TP_printk("sending child_relid 0x%x, ret %d", __entry->child_relid,
+		    __entry->ret)
+	);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 12/17] hyper-v: trace vmbus_establish_gpadl()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to CHANNELMSG_GPADL_HEADER/CHANNELMSG_GPADL_BODY sender.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel.c  |  6 ++++++
 drivers/hv/hv_trace.h | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index a827f43aa408..823ddc714d56 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -435,6 +435,9 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
 
 	ret = vmbus_post_msg(gpadlmsg, msginfo->msgsize -
 			     sizeof(*msginfo), true);
+
+	trace_vmbus_establish_gpadl_header(gpadlmsg, ret);
+
 	if (ret != 0)
 		goto cleanup;
 
@@ -450,6 +453,9 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
 		ret = vmbus_post_msg(gpadl_body,
 				     submsginfo->msgsize - sizeof(*submsginfo),
 				     true);
+
+		trace_vmbus_establish_gpadl_body(gpadl_body, ret);
+
 		if (ret != 0)
 			goto cleanup;
 
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 302bd4e964f0..978e70bdc7c5 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -172,6 +172,48 @@ TRACE_EVENT(vmbus_close_internal,
 		    __entry->ret)
 	);
 
+TRACE_EVENT(vmbus_establish_gpadl_header,
+	    TP_PROTO(const struct vmbus_channel_gpadl_header *msg, int ret),
+	    TP_ARGS(msg, ret),
+	    TP_STRUCT__entry(
+		    __field(u32, child_relid)
+		    __field(u32, gpadl)
+		    __field(u16, range_buflen)
+		    __field(u16, rangecount)
+		    __field(int, ret)
+		    ),
+	    TP_fast_assign(
+		    __entry->child_relid = msg->child_relid;
+		    __entry->gpadl = msg->gpadl;
+		    __entry->range_buflen = msg->range_buflen;
+		    __entry->rangecount = msg->rangecount;
+		    __entry->ret = ret;
+		    ),
+	    TP_printk("sending child_relid 0x%x, gpadl 0x%x, range_buflen %d "
+		      "rangecount %d, ret %d",
+		      __entry->child_relid, __entry->gpadl,
+		      __entry->range_buflen, __entry->rangecount, __entry->ret
+		    )
+	);
+
+TRACE_EVENT(vmbus_establish_gpadl_body,
+	    TP_PROTO(const struct vmbus_channel_gpadl_body *msg, int ret),
+	    TP_ARGS(msg, ret),
+	    TP_STRUCT__entry(
+		    __field(u32, msgnumber)
+		    __field(u32, gpadl)
+		    __field(int, ret)
+		    ),
+	    TP_fast_assign(
+		    __entry->msgnumber = msg->msgnumber;
+		    __entry->gpadl = msg->gpadl;
+		    __entry->ret = ret;
+		    ),
+	    TP_printk("sending msgnumber %d, gpadl 0x%x, ret %d",
+		      __entry->msgnumber, __entry->gpadl, __entry->ret
+		    )
+	);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 13/17] hyper-v: trace vmbus_teardown_gpadl()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to CHANNELMSG_GPADL_TEARDOWN sender.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel.c  |  2 ++
 drivers/hv/hv_trace.h | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 823ddc714d56..0382d7b600a1 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -519,6 +519,8 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle)
 	ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_gpadl_teardown),
 			     true);
 
+	trace_vmbus_teardown_gpadl(msg, ret);
+
 	if (ret)
 		goto post_msg_err;
 
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 978e70bdc7c5..cd33a52ef27f 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -214,6 +214,24 @@ TRACE_EVENT(vmbus_establish_gpadl_body,
 		    )
 	);
 
+TRACE_EVENT(vmbus_teardown_gpadl,
+	    TP_PROTO(const struct vmbus_channel_gpadl_teardown *msg, int ret),
+	    TP_ARGS(msg, ret),
+	    TP_STRUCT__entry(
+		    __field(u32, child_relid)
+		    __field(u32, gpadl)
+		    __field(int, ret)
+		    ),
+	    TP_fast_assign(
+		    __entry->child_relid = msg->child_relid;
+		    __entry->gpadl = msg->gpadl;
+		    __entry->ret = ret;
+		    ),
+	    TP_printk("sending child_relid 0x%x, gpadl 0x%x, ret %d",
+		      __entry->child_relid, __entry->gpadl, __entry->ret
+		    )
+	);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 14/17] hyper-v: trace vmbus_negotiate_version()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to CHANNELMSG_INITIATE_CONTACT sender.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/connection.c |  3 +++
 drivers/hv/hv_trace.h   | 26 ++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index b06a6b796819..05f413c4fe3b 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -117,6 +117,9 @@ static int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo,
 	ret = vmbus_post_msg(msg,
 			     sizeof(struct vmbus_channel_initiate_contact),
 			     true);
+
+	trace_vmbus_negotiate_version(msg, ret);
+
 	if (ret != 0) {
 		spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
 		list_del(&msginfo->msglistentry);
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index cd33a52ef27f..f06284d64a8c 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -232,6 +232,32 @@ TRACE_EVENT(vmbus_teardown_gpadl,
 		    )
 	);
 
+TRACE_EVENT(vmbus_negotiate_version,
+	    TP_PROTO(const struct vmbus_channel_initiate_contact *msg, int ret),
+	    TP_ARGS(msg, ret),
+	    TP_STRUCT__entry(
+		    __field(u32, ver)
+		    __field(u32, target_vcpu)
+		    __field(int, ret)
+		    __field(u64, int_page)
+		    __field(u64, mon_page1)
+		    __field(u64, mon_page2)
+		    ),
+	    TP_fast_assign(
+		    __entry->ver = msg->vmbus_version_requested;
+		    __entry->target_vcpu = msg->target_vcpu;
+		    __entry->int_page = msg->interrupt_page;
+		    __entry->mon_page1 = msg->monitor_page1;
+		    __entry->mon_page2 = msg->monitor_page2;
+		    __entry->ret = ret;
+		    ),
+	    TP_printk("sending vmbus_version_requested %d, target_vcpu 0x%x, "
+		      "pages %llx:%llx:%llx, ret %d",
+		      __entry->ver, __entry->target_vcpu, __entry->int_page,
+		      __entry->mon_page1, __entry->mon_page2, __entry->ret
+		    )
+	);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 15/17] hyper-v: trace vmbus_release_relid()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to CHANNELMSG_RELID_RELEASED sender.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c |  7 +++++--
 drivers/hv/hv_trace.h     | 16 ++++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index b27edff8796e..ec5454f3f4a6 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -373,12 +373,15 @@ static void percpu_channel_deq(void *arg)
 static void vmbus_release_relid(u32 relid)
 {
 	struct vmbus_channel_relid_released msg;
+	int ret;
 
 	memset(&msg, 0, sizeof(struct vmbus_channel_relid_released));
 	msg.child_relid = relid;
 	msg.header.msgtype = CHANNELMSG_RELID_RELEASED;
-	vmbus_post_msg(&msg, sizeof(struct vmbus_channel_relid_released),
-		       true);
+	ret = vmbus_post_msg(&msg, sizeof(struct vmbus_channel_relid_released),
+			     true);
+
+	trace_vmbus_release_relid(&msg, ret);
 }
 
 void hv_process_channel_removal(u32 relid)
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index f06284d64a8c..f0e437c3522f 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -258,6 +258,22 @@ TRACE_EVENT(vmbus_negotiate_version,
 		    )
 	);
 
+TRACE_EVENT(vmbus_release_relid,
+	    TP_PROTO(const struct vmbus_channel_relid_released *msg, int ret),
+	    TP_ARGS(msg, ret),
+	    TP_STRUCT__entry(
+		    __field(u32, child_relid)
+		    __field(int, ret)
+		    ),
+	    TP_fast_assign(
+		    __entry->child_relid = msg->child_relid;
+		    __entry->ret = ret;
+		    ),
+	    TP_printk("sending child_relid 0x%x, ret %d",
+		      __entry->child_relid, __entry->ret
+		    )
+	);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 16/17] hyper-v: trace vmbus_send_tl_connect_request()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add tracepoint to CHANNELMSG_TL_CONNECT_REQUEST sender.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel.c  |  7 ++++++-
 drivers/hv/hv_trace.h | 20 ++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 0382d7b600a1..ed3634e53e15 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -236,13 +236,18 @@ int vmbus_send_tl_connect_request(const uuid_le *shv_guest_servie_id,
 				  const uuid_le *shv_host_servie_id)
 {
 	struct vmbus_channel_tl_connect_request conn_msg;
+	int ret;
 
 	memset(&conn_msg, 0, sizeof(conn_msg));
 	conn_msg.header.msgtype = CHANNELMSG_TL_CONNECT_REQUEST;
 	conn_msg.guest_endpoint_id = *shv_guest_servie_id;
 	conn_msg.host_service_id = *shv_host_servie_id;
 
-	return vmbus_post_msg(&conn_msg, sizeof(conn_msg), true);
+	ret = vmbus_post_msg(&conn_msg, sizeof(conn_msg), true);
+
+	trace_vmbus_send_tl_connect_request(&conn_msg, ret);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(vmbus_send_tl_connect_request);
 
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index f0e437c3522f..5382d9630306 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -274,6 +274,26 @@ TRACE_EVENT(vmbus_release_relid,
 		    )
 	);
 
+TRACE_EVENT(vmbus_send_tl_connect_request,
+	    TP_PROTO(const struct vmbus_channel_tl_connect_request *msg,
+		     int ret),
+	    TP_ARGS(msg, ret),
+	    TP_STRUCT__entry(
+		    __array(char, guest_id, 16)
+		    __array(char, host_id, 16)
+		    __field(int, ret)
+		    ),
+	    TP_fast_assign(
+		    memcpy(__entry->guest_id, &msg->guest_endpoint_id.b, 16);
+		    memcpy(__entry->host_id, &msg->host_service_id.b, 16);
+		    __entry->ret = ret;
+		    ),
+	    TP_printk("sending guest_endpoint_id %pUl, host_service_id %pUl, "
+		      "ret %d",
+		      __entry->guest_id, __entry->host_id, __entry->ret
+		    )
+	);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.14.1

[PATCH 17/17] hyper-v: trace channel events

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Added an additional set of trace points for when channel gets notified
or signals host.

Suggested-by: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel.c    |  2 ++
 drivers/hv/connection.c |  2 ++
 drivers/hv/hv_trace.h   | 23 +++++++++++++++++++++++
 drivers/hv/vmbus_drv.c  |  2 ++
 4 files changed, 29 insertions(+)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index ed3634e53e15..19f0cf37e0ed 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -43,6 +43,8 @@ void vmbus_setevent(struct vmbus_channel *channel)
 {
 	struct hv_monitor_page *monitorpage;
 
+	trace_vmbus_setevent(channel);
+
 	/*
 	 * For channels marked as in "low latency" mode
 	 * bypass the monitor page mechanism.
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 05f413c4fe3b..447371f4de56 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -322,6 +322,8 @@ void vmbus_on_event(unsigned long data)
 	struct vmbus_channel *channel = (void *) data;
 	unsigned long time_limit = jiffies + 2;
 
+	trace_vmbus_on_event(channel);
+
 	do {
 		void (*callback_fn)(void *);
 
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 5382d9630306..d635ee95b20d 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -294,6 +294,29 @@ TRACE_EVENT(vmbus_send_tl_connect_request,
 		    )
 	);
 
+DECLARE_EVENT_CLASS(vmbus_channel,
+	TP_PROTO(const struct vmbus_channel *channel),
+	TP_ARGS(channel),
+	TP_STRUCT__entry(__field(u32, relid)),
+	TP_fast_assign(__entry->relid = channel->offermsg.child_relid),
+	TP_printk("relid 0x%x", __entry->relid)
+);
+
+DEFINE_EVENT(vmbus_channel, vmbus_chan_sched,
+	    TP_PROTO(const struct vmbus_channel *channel),
+	    TP_ARGS(channel)
+);
+
+DEFINE_EVENT(vmbus_channel, vmbus_setevent,
+	    TP_PROTO(const struct vmbus_channel *channel),
+	    TP_ARGS(channel)
+);
+
+DEFINE_EVENT(vmbus_channel, vmbus_on_event,
+	    TP_PROTO(const struct vmbus_channel *channel),
+	    TP_ARGS(channel)
+);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 8f9293a87c6b..6a86746d4f60 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -946,6 +946,8 @@ static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
 			if (channel->rescind)
 				continue;
 
+			trace_vmbus_chan_sched(channel);
+
 			++channel->interrupts;
 
 			switch (channel->callback_mode) {
-- 
2.14.1

Re: [PATCH 10/17] hyper-v: trace vmbus_open()

[Greg KH]

On Sun, Oct 29, 2017 at 12:21:09PM -0700, kys@exchange.microsoft.com wrote:
> From: Vitaly Kuznetsov <vkuznets@redhat.com>
> 
> Add tracepoint to CHANNELMSG_OPENCHANNEL sender.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/hv/channel.c  |  2 ++
>  drivers/hv/hv_trace.h | 27 +++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index a406beb10dd0..739b3fe1e0fb 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -185,6 +185,8 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
>  	ret = vmbus_post_msg(open_msg,
>  			     sizeof(struct vmbus_channel_open_channel), true);
>  
> +	trace_vmbus_open(open_msg, ret);

Why add tracepoints for things that ftrace can handle automatically?

Re: [PATCH 10/17] hyper-v: trace vmbus_open()

[Vitaly Kuznetsov]

Greg KH <gregkh@linuxfoundation.org> writes:

> On Sun, Oct 29, 2017 at 12:21:09PM -0700, kys@exchange.microsoft.com wrote:
>> From: Vitaly Kuznetsov <vkuznets@redhat.com>
>> 
>> Add tracepoint to CHANNELMSG_OPENCHANNEL sender.
>> 
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
>> ---
>>  drivers/hv/channel.c  |  2 ++
>>  drivers/hv/hv_trace.h | 27 +++++++++++++++++++++++++++
>>  2 files changed, 29 insertions(+)
>> 
>> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
>> index a406beb10dd0..739b3fe1e0fb 100644
>> --- a/drivers/hv/channel.c
>> +++ b/drivers/hv/channel.c
>> @@ -185,6 +185,8 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
>>  	ret = vmbus_post_msg(open_msg,
>>  			     sizeof(struct vmbus_channel_open_channel), true);
>>  
>> +	trace_vmbus_open(open_msg, ret);
>
> Why add tracepoints for things that ftrace can handle automatically?

This series adds pretty prints for structures printing what is needed
and in the right format significantly simplifying debugging. And it
wouldn't make sense to add tracepoints to *some* messages-related
functions and skip others where parsing is more trivial.

-- 
  Vitaly

Re: [PATCH 10/17] hyper-v: trace vmbus_open()

[Greg KH]

On Mon, Oct 30, 2017 at 09:16:19AM +0100, Vitaly Kuznetsov wrote:
> Greg KH <gregkh@linuxfoundation.org> writes:
> 
> > On Sun, Oct 29, 2017 at 12:21:09PM -0700, kys@exchange.microsoft.com wrote:
> >> From: Vitaly Kuznetsov <vkuznets@redhat.com>
> >> 
> >> Add tracepoint to CHANNELMSG_OPENCHANNEL sender.
> >> 
> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> >> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> >> ---
> >>  drivers/hv/channel.c  |  2 ++
> >>  drivers/hv/hv_trace.h | 27 +++++++++++++++++++++++++++
> >>  2 files changed, 29 insertions(+)
> >> 
> >> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> >> index a406beb10dd0..739b3fe1e0fb 100644
> >> --- a/drivers/hv/channel.c
> >> +++ b/drivers/hv/channel.c
> >> @@ -185,6 +185,8 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
> >>  	ret = vmbus_post_msg(open_msg,
> >>  			     sizeof(struct vmbus_channel_open_channel), true);
> >>  
> >> +	trace_vmbus_open(open_msg, ret);
> >
> > Why add tracepoints for things that ftrace can handle automatically?
> 
> This series adds pretty prints for structures printing what is needed
> and in the right format significantly simplifying debugging. And it
> wouldn't make sense to add tracepoints to *some* messages-related
> functions and skip others where parsing is more trivial.

Tracepoints add memory usage and take up real space.  If you don't need
them for something, as there are other ways to already get the
information needed, why add new ones that you now need to drag around
for all time?

thanks

greg k-h

Re: [PATCH 10/17] hyper-v: trace vmbus_open()

[Vitaly Kuznetsov]

Greg KH <gregkh@linuxfoundation.org> writes:

> On Mon, Oct 30, 2017 at 09:16:19AM +0100, Vitaly Kuznetsov wrote:
>> Greg KH <gregkh@linuxfoundation.org> writes:
>> 
>> > On Sun, Oct 29, 2017 at 12:21:09PM -0700, kys@exchange.microsoft.com wrote:
>> >> From: Vitaly Kuznetsov <vkuznets@redhat.com>
>> >> 
>> >> Add tracepoint to CHANNELMSG_OPENCHANNEL sender.
>> >> 
>> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> >> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
>> >> ---
>> >>  drivers/hv/channel.c  |  2 ++
>> >>  drivers/hv/hv_trace.h | 27 +++++++++++++++++++++++++++
>> >>  2 files changed, 29 insertions(+)
>> >> 
>> >> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
>> >> index a406beb10dd0..739b3fe1e0fb 100644
>> >> --- a/drivers/hv/channel.c
>> >> +++ b/drivers/hv/channel.c
>> >> @@ -185,6 +185,8 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
>> >>  	ret = vmbus_post_msg(open_msg,
>> >>  			     sizeof(struct vmbus_channel_open_channel), true);
>> >>  
>> >> +	trace_vmbus_open(open_msg, ret);
>> >
>> > Why add tracepoints for things that ftrace can handle automatically?
>> 
>> This series adds pretty prints for structures printing what is needed
>> and in the right format significantly simplifying debugging. And it
>> wouldn't make sense to add tracepoints to *some* messages-related
>> functions and skip others where parsing is more trivial.
>
> Tracepoints add memory usage and take up real space.  If you don't need
> them for something, as there are other ways to already get the
> information needed, why add new ones that you now need to drag around
> for all time?
>

Are you opposed to the series as a whole (AKA 'no tracepoints in
drivers') or only to some tracepoints we add here?

Debugging stuff is always a tradeof between some memory overhead and
convenience (and CONFIG_TRACEPOINTS is the knob). Here I'd like to see
vmbus_* structures parsed in output. When things go wrong I can tell
someone "do echo 1 > /sys/kernel/debug/tracing/events/hyperv/enable and
show me the output" and I will have enough information to figure out
what's going on.

I'm probably missing something about easy 'alternative' ways to get what
I need and why people add tracepoints to drivers in general.

-- 
  Vitaly

Re: [PATCH 10/17] hyper-v: trace vmbus_open()

[Greg KH]

On Mon, Oct 30, 2017 at 11:07:01AM +0100, Vitaly Kuznetsov wrote:
> Greg KH <gregkh@linuxfoundation.org> writes:
> 
> > On Mon, Oct 30, 2017 at 09:16:19AM +0100, Vitaly Kuznetsov wrote:
> >> Greg KH <gregkh@linuxfoundation.org> writes:
> >> 
> >> > On Sun, Oct 29, 2017 at 12:21:09PM -0700, kys@exchange.microsoft.com wrote:
> >> >> From: Vitaly Kuznetsov <vkuznets@redhat.com>
> >> >> 
> >> >> Add tracepoint to CHANNELMSG_OPENCHANNEL sender.
> >> >> 
> >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> >> >> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> >> >> ---
> >> >>  drivers/hv/channel.c  |  2 ++
> >> >>  drivers/hv/hv_trace.h | 27 +++++++++++++++++++++++++++
> >> >>  2 files changed, 29 insertions(+)
> >> >> 
> >> >> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> >> >> index a406beb10dd0..739b3fe1e0fb 100644
> >> >> --- a/drivers/hv/channel.c
> >> >> +++ b/drivers/hv/channel.c
> >> >> @@ -185,6 +185,8 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
> >> >>  	ret = vmbus_post_msg(open_msg,
> >> >>  			     sizeof(struct vmbus_channel_open_channel), true);
> >> >>  
> >> >> +	trace_vmbus_open(open_msg, ret);
> >> >
> >> > Why add tracepoints for things that ftrace can handle automatically?
> >> 
> >> This series adds pretty prints for structures printing what is needed
> >> and in the right format significantly simplifying debugging. And it
> >> wouldn't make sense to add tracepoints to *some* messages-related
> >> functions and skip others where parsing is more trivial.
> >
> > Tracepoints add memory usage and take up real space.  If you don't need
> > them for something, as there are other ways to already get the
> > information needed, why add new ones that you now need to drag around
> > for all time?
> >
> 
> Are you opposed to the series as a whole (AKA 'no tracepoints in
> drivers') or only to some tracepoints we add here?

I'm opposed to adding tracepoints for things that are not needed as you
can get the same info already today without the tracepoint.

I'm not opposed to tracepoints in drivers as it's up to the maintainer
to have to manage them over the long-term and the issues that surround
them...

thanks,

greg k-h

Re: [PATCH 10/17] hyper-v: trace vmbus_open()

[Steven Rostedt]

On Mon, 30 Oct 2017 11:32:20 +0100
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Mon, Oct 30, 2017 at 11:07:01AM +0100, Vitaly Kuznetsov wrote:
> > Greg KH <gregkh@linuxfoundation.org> writes:
> >   
> > > On Mon, Oct 30, 2017 at 09:16:19AM +0100, Vitaly Kuznetsov wrote:  
> > >> Greg KH <gregkh@linuxfoundation.org> writes:
> > >>   
> > >> > On Sun, Oct 29, 2017 at 12:21:09PM -0700, kys@exchange.microsoft.com wrote:  
> > >> >> From: Vitaly Kuznetsov <vkuznets@redhat.com>
> > >> >> 
> > >> >> Add tracepoint to CHANNELMSG_OPENCHANNEL sender.
> > >> >> 
> > >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> > >> >> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > >> >> ---
> > >> >>  drivers/hv/channel.c  |  2 ++
> > >> >>  drivers/hv/hv_trace.h | 27 +++++++++++++++++++++++++++
> > >> >>  2 files changed, 29 insertions(+)
> > >> >> 
> > >> >> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> > >> >> index a406beb10dd0..739b3fe1e0fb 100644
> > >> >> --- a/drivers/hv/channel.c
> > >> >> +++ b/drivers/hv/channel.c
> > >> >> @@ -185,6 +185,8 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
> > >> >>  	ret = vmbus_post_msg(open_msg,
> > >> >>  			     sizeof(struct vmbus_channel_open_channel), true);
> > >> >>  
> > >> >> +	trace_vmbus_open(open_msg, ret);  
> > >> >
> > >> > Why add tracepoints for things that ftrace can handle automatically?  
> > >> 
> > >> This series adds pretty prints for structures printing what is needed
> > >> and in the right format significantly simplifying debugging. And it
> > >> wouldn't make sense to add tracepoints to *some* messages-related
> > >> functions and skip others where parsing is more trivial.  
> > >
> > > Tracepoints add memory usage and take up real space.  If you don't need
> > > them for something, as there are other ways to already get the
> > > information needed, why add new ones that you now need to drag around
> > > for all time?
> > >  
> > 
> > Are you opposed to the series as a whole (AKA 'no tracepoints in
> > drivers') or only to some tracepoints we add here?  
> 
> I'm opposed to adding tracepoints for things that are not needed as you
> can get the same info already today without the tracepoint.

I looked at this specific tracepoint, and I don't see how to get the
information from the current tracing infrastructure. Maybe an eBPF
program attached to a kprobe here might work. But the tracepoint data
looks like this:

+           TP_STRUCT__entry(
+                   __field(u32, child_relid)
+                   __field(u32, openid)
+                   __field(u32, gpadlhandle)
+                   __field(u32, target_vp)
+                   __field(u32, offset)
+                   __field(int, ret)
+                   ),
+           TP_fast_assign(
+                   __entry->child_relid = msg->child_relid;
+                   __entry->openid = msg->openid;
+                   __entry->gpadlhandle = msg->ringbuffer_gpadlhandle;
+                   __entry->target_vp = msg->target_vp;
+                   __entry->offset = msg->downstream_ringbuffer_pageoffset;
+                   __entry->ret = ret;
+                   ),

I don't see how that information can be extracted easily without a
tracepoint here. Am I missing something?

-- Steve


> 
> I'm not opposed to tracepoints in drivers as it's up to the maintainer
> to have to manage them over the long-term and the issues that surround
> them...
> 
>

Re: [PATCH 10/17] hyper-v: trace vmbus_open()

[Greg KH]

On Mon, Oct 30, 2017 at 10:31:34AM -0400, Steven Rostedt wrote:
> On Mon, 30 Oct 2017 11:32:20 +0100
> Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > On Mon, Oct 30, 2017 at 11:07:01AM +0100, Vitaly Kuznetsov wrote:
> > > Greg KH <gregkh@linuxfoundation.org> writes:
> > >   
> > > > On Mon, Oct 30, 2017 at 09:16:19AM +0100, Vitaly Kuznetsov wrote:  
> > > >> Greg KH <gregkh@linuxfoundation.org> writes:
> > > >>   
> > > >> > On Sun, Oct 29, 2017 at 12:21:09PM -0700, kys@exchange.microsoft.com wrote:  
> > > >> >> From: Vitaly Kuznetsov <vkuznets@redhat.com>
> > > >> >> 
> > > >> >> Add tracepoint to CHANNELMSG_OPENCHANNEL sender.
> > > >> >> 
> > > >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> > > >> >> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > >> >> ---
> > > >> >>  drivers/hv/channel.c  |  2 ++
> > > >> >>  drivers/hv/hv_trace.h | 27 +++++++++++++++++++++++++++
> > > >> >>  2 files changed, 29 insertions(+)
> > > >> >> 
> > > >> >> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> > > >> >> index a406beb10dd0..739b3fe1e0fb 100644
> > > >> >> --- a/drivers/hv/channel.c
> > > >> >> +++ b/drivers/hv/channel.c
> > > >> >> @@ -185,6 +185,8 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
> > > >> >>  	ret = vmbus_post_msg(open_msg,
> > > >> >>  			     sizeof(struct vmbus_channel_open_channel), true);
> > > >> >>  
> > > >> >> +	trace_vmbus_open(open_msg, ret);  
> > > >> >
> > > >> > Why add tracepoints for things that ftrace can handle automatically?  
> > > >> 
> > > >> This series adds pretty prints for structures printing what is needed
> > > >> and in the right format significantly simplifying debugging. And it
> > > >> wouldn't make sense to add tracepoints to *some* messages-related
> > > >> functions and skip others where parsing is more trivial.  
> > > >
> > > > Tracepoints add memory usage and take up real space.  If you don't need
> > > > them for something, as there are other ways to already get the
> > > > information needed, why add new ones that you now need to drag around
> > > > for all time?
> > > >  
> > > 
> > > Are you opposed to the series as a whole (AKA 'no tracepoints in
> > > drivers') or only to some tracepoints we add here?  
> > 
> > I'm opposed to adding tracepoints for things that are not needed as you
> > can get the same info already today without the tracepoint.
> 
> I looked at this specific tracepoint, and I don't see how to get the
> information from the current tracing infrastructure. Maybe an eBPF
> program attached to a kprobe here might work. But the tracepoint data
> looks like this:
> 
> +           TP_STRUCT__entry(
> +                   __field(u32, child_relid)
> +                   __field(u32, openid)
> +                   __field(u32, gpadlhandle)
> +                   __field(u32, target_vp)
> +                   __field(u32, offset)
> +                   __field(int, ret)
> +                   ),
> +           TP_fast_assign(
> +                   __entry->child_relid = msg->child_relid;
> +                   __entry->openid = msg->openid;
> +                   __entry->gpadlhandle = msg->ringbuffer_gpadlhandle;
> +                   __entry->target_vp = msg->target_vp;
> +                   __entry->offset = msg->downstream_ringbuffer_pageoffset;
> +                   __entry->ret = ret;
> +                   ),
> 
> I don't see how that information can be extracted easily without a
> tracepoint here. Am I missing something?

Wasn't one of the outcomes of the conference last week the fact that for
ftrace + ebpf we could get access to the structures of the function
parameters?  Or that work would soon be showing up?

It just feels "wrong" to add a tracepoint for a function call, like it
is a duplication of work/functionality we already have.

thanks,

greg k-h

Re: [PATCH 10/17] hyper-v: trace vmbus_open()

[Steven Rostedt]

On Tue, 31 Oct 2017 13:48:00 +0100
Greg KH <gregkh@linuxfoundation.org> wrote:

> > I don't see how that information can be extracted easily without a
> > tracepoint here. Am I missing something?  
> 
> Wasn't one of the outcomes of the conference last week the fact that for
> ftrace + ebpf we could get access to the structures of the function
> parameters?  Or that work would soon be showing up?

I told Linus that I'll start building an infrastructure on function
tracing to see what we can do. But it may be very limited in features.
I don't believe eBPF can follow arbitrary data structure pointers
without helper functions. Which doesn't exist for this type of code yet.

> 
> It just feels "wrong" to add a tracepoint for a function call, like it
> is a duplication of work/functionality we already have.

We don't already have it. We may have something in a year (or two) that
may be able to get all the data that is requested here. But it's going
to take lots of RFC patch sets and brain storming to come up with
something that everyone is satisfied with.

In other words, the functionality is currently in vaporware state.

-- Steve

Re: [PATCH 10/17] hyper-v: trace vmbus_open()

[Stephen Hemminger]

On Tue, 31 Oct 2017 13:48:00 +0100
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Mon, Oct 30, 2017 at 10:31:34AM -0400, Steven Rostedt wrote:
> > On Mon, 30 Oct 2017 11:32:20 +0100
> > Greg KH <gregkh@linuxfoundation.org> wrote:
> >   
> > > On Mon, Oct 30, 2017 at 11:07:01AM +0100, Vitaly Kuznetsov wrote:  
> > > > Greg KH <gregkh@linuxfoundation.org> writes:
> > > >     
> > > > > On Mon, Oct 30, 2017 at 09:16:19AM +0100, Vitaly Kuznetsov wrote:    
> > > > >> Greg KH <gregkh@linuxfoundation.org> writes:
> > > > >>     
> > > > >> > On Sun, Oct 29, 2017 at 12:21:09PM -0700, kys@exchange.microsoft.com wrote:    
> > > > >> >> From: Vitaly Kuznetsov <vkuznets@redhat.com>
> > > > >> >> 
> > > > >> >> Add tracepoint to CHANNELMSG_OPENCHANNEL sender.
> > > > >> >> 
> > > > >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> > > > >> >> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > >> >> ---
> > > > >> >>  drivers/hv/channel.c  |  2 ++
> > > > >> >>  drivers/hv/hv_trace.h | 27 +++++++++++++++++++++++++++
> > > > >> >>  2 files changed, 29 insertions(+)
> > > > >> >> 
> > > > >> >> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> > > > >> >> index a406beb10dd0..739b3fe1e0fb 100644
> > > > >> >> --- a/drivers/hv/channel.c
> > > > >> >> +++ b/drivers/hv/channel.c
> > > > >> >> @@ -185,6 +185,8 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
> > > > >> >>  	ret = vmbus_post_msg(open_msg,
> > > > >> >>  			     sizeof(struct vmbus_channel_open_channel), true);
> > > > >> >>  
> > > > >> >> +	trace_vmbus_open(open_msg, ret);    
> > > > >> >
> > > > >> > Why add tracepoints for things that ftrace can handle automatically?    
> > > > >> 
> > > > >> This series adds pretty prints for structures printing what is needed
> > > > >> and in the right format significantly simplifying debugging. And it
> > > > >> wouldn't make sense to add tracepoints to *some* messages-related
> > > > >> functions and skip others where parsing is more trivial.    
> > > > >
> > > > > Tracepoints add memory usage and take up real space.  If you don't need
> > > > > them for something, as there are other ways to already get the
> > > > > information needed, why add new ones that you now need to drag around
> > > > > for all time?
> > > > >    
> > > > 
> > > > Are you opposed to the series as a whole (AKA 'no tracepoints in
> > > > drivers') or only to some tracepoints we add here?    
> > > 
> > > I'm opposed to adding tracepoints for things that are not needed as you
> > > can get the same info already today without the tracepoint.  
> > 
> > I looked at this specific tracepoint, and I don't see how to get the
> > information from the current tracing infrastructure. Maybe an eBPF
> > program attached to a kprobe here might work. But the tracepoint data
> > looks like this:
> > 
> > +           TP_STRUCT__entry(
> > +                   __field(u32, child_relid)
> > +                   __field(u32, openid)
> > +                   __field(u32, gpadlhandle)
> > +                   __field(u32, target_vp)
> > +                   __field(u32, offset)
> > +                   __field(int, ret)
> > +                   ),
> > +           TP_fast_assign(
> > +                   __entry->child_relid = msg->child_relid;
> > +                   __entry->openid = msg->openid;
> > +                   __entry->gpadlhandle = msg->ringbuffer_gpadlhandle;
> > +                   __entry->target_vp = msg->target_vp;
> > +                   __entry->offset = msg->downstream_ringbuffer_pageoffset;
> > +                   __entry->ret = ret;
> > +                   ),
> > 
> > I don't see how that information can be extracted easily without a
> > tracepoint here. Am I missing something?  
> 
> Wasn't one of the outcomes of the conference last week the fact that for
> ftrace + ebpf we could get access to the structures of the function
> parameters?  Or that work would soon be showing up?
> 
> It just feels "wrong" to add a tracepoint for a function call, like it
> is a duplication of work/functionality we already have.
> 
> thanks,
> 
> greg k-h

Just to add some context. VMBus open/close etc are not in critical path.
The reason that tracing of these makes sense is that there have been bugs
in the past with teardown and restart of channels, and having some information
would help. Not sure if the detailed internals of the parameters matter that
much since it has been clear what the parameters were by context.

RE: [PATCH 10/17] hyper-v: trace vmbus_open()

[KY Srinivasan]

> -----Original Message-----
> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> Behalf Of Steven Rostedt
> Sent: Tuesday, October 31, 2017 6:10 AM
> To: Greg KH <gregkh@linuxfoundation.org>
> Cc: olaf@aepfle.de; Stephen Hemminger <sthemmin@microsoft.com>;
> jasowang@redhat.com; linux-kernel@vger.kernel.org; apw@canonical.com;
> marcelo.cerri@canonical.com; devel@linuxdriverproject.org; Vitaly
> Kuznetsov <vkuznets@redhat.com>; leann.ogasawara@canonical.com
> Subject: Re: [PATCH 10/17] hyper-v: trace vmbus_open()
> 
> On Tue, 31 Oct 2017 13:48:00 +0100
> Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > > I don't see how that information can be extracted easily without a
> > > tracepoint here. Am I missing something?
> >
> > Wasn't one of the outcomes of the conference last week the fact that for
> > ftrace + ebpf we could get access to the structures of the function
> > parameters?  Or that work would soon be showing up?
> 
> I told Linus that I'll start building an infrastructure on function
> tracing to see what we can do. But it may be very limited in features.
> I don't believe eBPF can follow arbitrary data structure pointers
> without helper functions. Which doesn't exist for this type of code yet.
> 
> >
> > It just feels "wrong" to add a tracepoint for a function call, like it
> > is a duplication of work/functionality we already have.
> 
> We don't already have it. We may have something in a year (or two) that
> may be able to get all the data that is requested here. But it's going
> to take lots of RFC patch sets and brain storming to come up with
> something that everyone is satisfied with.
> 
> In other words, the functionality is currently in vaporware state.

Greg,

The added memory overhead is very minimal, and the added tracing support is extremely useful for
our debugging. If you don't mind, I would like to have this tracing support.

Regards,

K. Y
> 
> -- Steve
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdriverd
> ev.linuxdriverproject.org%2Fmailman%2Flistinfo%2Fdriverdev-
> devel&data=02%7C01%7Ckys%40microsoft.com%7C93ac7eb43f9745d3ba5f0
> 8d52060c8a6%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6364505
> 22419439104&sdata=XT46EGhjw9yoenE8qrW0aTqfDJ0ONvAPPzFri4Udn%2BY
> %3D&reserved=0

Re: [PATCH 10/17] hyper-v: trace vmbus_open()

[Greg KH]

On Wed, Nov 01, 2017 at 05:43:08PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> > Behalf Of Steven Rostedt
> > Sent: Tuesday, October 31, 2017 6:10 AM
> > To: Greg KH <gregkh@linuxfoundation.org>
> > Cc: olaf@aepfle.de; Stephen Hemminger <sthemmin@microsoft.com>;
> > jasowang@redhat.com; linux-kernel@vger.kernel.org; apw@canonical.com;
> > marcelo.cerri@canonical.com; devel@linuxdriverproject.org; Vitaly
> > Kuznetsov <vkuznets@redhat.com>; leann.ogasawara@canonical.com
> > Subject: Re: [PATCH 10/17] hyper-v: trace vmbus_open()
> > 
> > On Tue, 31 Oct 2017 13:48:00 +0100
> > Greg KH <gregkh@linuxfoundation.org> wrote:
> > 
> > > > I don't see how that information can be extracted easily without a
> > > > tracepoint here. Am I missing something?
> > >
> > > Wasn't one of the outcomes of the conference last week the fact that for
> > > ftrace + ebpf we could get access to the structures of the function
> > > parameters?  Or that work would soon be showing up?
> > 
> > I told Linus that I'll start building an infrastructure on function
> > tracing to see what we can do. But it may be very limited in features.
> > I don't believe eBPF can follow arbitrary data structure pointers
> > without helper functions. Which doesn't exist for this type of code yet.
> > 
> > >
> > > It just feels "wrong" to add a tracepoint for a function call, like it
> > > is a duplication of work/functionality we already have.
> > 
> > We don't already have it. We may have something in a year (or two) that
> > may be able to get all the data that is requested here. But it's going
> > to take lots of RFC patch sets and brain storming to come up with
> > something that everyone is satisfied with.
> > 
> > In other words, the functionality is currently in vaporware state.
> 
> Greg,
> 
> The added memory overhead is very minimal, and the added tracing support is extremely useful for
> our debugging. If you don't mind, I would like to have this tracing support.

Ok, it's your subsystem, you get to handle the long-term maintenance of
these :)

Now merged,

greg k-h

RE: [PATCH 10/17] hyper-v: trace vmbus_open()

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Saturday, November 4, 2017 4:05 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>; olaf@aepfle.de; Stephen
> Hemminger <sthemmin@microsoft.com>; jasowang@redhat.com; linux-
> kernel@vger.kernel.org; apw@canonical.com; marcelo.cerri@canonical.com;
> devel@linuxdriverproject.org; Vitaly Kuznetsov <vkuznets@redhat.com>;
> leann.ogasawara@canonical.com
> Subject: Re: [PATCH 10/17] hyper-v: trace vmbus_open()
> 
> On Wed, Nov 01, 2017 at 05:43:08PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org]
> On
> > > Behalf Of Steven Rostedt
> > > Sent: Tuesday, October 31, 2017 6:10 AM
> > > To: Greg KH <gregkh@linuxfoundation.org>
> > > Cc: olaf@aepfle.de; Stephen Hemminger <sthemmin@microsoft.com>;
> > > jasowang@redhat.com; linux-kernel@vger.kernel.org;
> apw@canonical.com;
> > > marcelo.cerri@canonical.com; devel@linuxdriverproject.org; Vitaly
> > > Kuznetsov <vkuznets@redhat.com>; leann.ogasawara@canonical.com
> > > Subject: Re: [PATCH 10/17] hyper-v: trace vmbus_open()
> > >
> > > On Tue, 31 Oct 2017 13:48:00 +0100
> > > Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > > > I don't see how that information can be extracted easily without a
> > > > > tracepoint here. Am I missing something?
> > > >
> > > > Wasn't one of the outcomes of the conference last week the fact that
> for
> > > > ftrace + ebpf we could get access to the structures of the function
> > > > parameters?  Or that work would soon be showing up?
> > >
> > > I told Linus that I'll start building an infrastructure on function
> > > tracing to see what we can do. But it may be very limited in features.
> > > I don't believe eBPF can follow arbitrary data structure pointers
> > > without helper functions. Which doesn't exist for this type of code yet.
> > >
> > > >
> > > > It just feels "wrong" to add a tracepoint for a function call, like it
> > > > is a duplication of work/functionality we already have.
> > >
> > > We don't already have it. We may have something in a year (or two) that
> > > may be able to get all the data that is requested here. But it's going
> > > to take lots of RFC patch sets and brain storming to come up with
> > > something that everyone is satisfied with.
> > >
> > > In other words, the functionality is currently in vaporware state.
> >
> > Greg,
> >
> > The added memory overhead is very minimal, and the added tracing
> support is extremely useful for
> > our debugging. If you don't mind, I would like to have this tracing support.
> 
> Ok, it's your subsystem, you get to handle the long-term maintenance of
> these :)
> 
> Now merged,

Thanks Greg.

K. Y
> 
> greg k-h