[PATCH v2 0/4] vfio-ccw: A couple trace changes

[PATCH v2 0/4] vfio-ccw: A couple trace changes

[Eric Farman]

Here a couple updates to the vfio-ccw traces in the kernel,
based on things I've been using locally.  Perhaps they'll
be useful for future debugging.

Steffen's comments earlier today (thank you!) were simple enough
that here's a quick turnaround on a v2:

v1/RFC -> v2:
 - Convert state/event=%x to %d
 - Use individual fields for cssid/ssid/sch_no, to enable
   filtering by device
 - Add 0x prefix to remaining %x substitution in existing trace

Eric Farman (4):
  vfio-ccw: Refactor how the traces are built
  vfio-ccw: Trace the FSM jumptable
  vfio-ccw: Add a trace for asynchronous requests
  vfio-ccw: Rework the io_fctl trace

 drivers/s390/cio/Makefile           |  4 +-
 drivers/s390/cio/vfio_ccw_cp.h      |  1 +
 drivers/s390/cio/vfio_ccw_fsm.c     | 11 +++--
 drivers/s390/cio/vfio_ccw_private.h |  1 +
 drivers/s390/cio/vfio_ccw_trace.c   | 14 ++++++
 drivers/s390/cio/vfio_ccw_trace.h   | 76 ++++++++++++++++++++++++++---
 6 files changed, 93 insertions(+), 14 deletions(-)
 create mode 100644 drivers/s390/cio/vfio_ccw_trace.c

-- 
2.17.1

[PATCH v2 1/4] vfio-ccw: Refactor how the traces are built

[Eric Farman]

Commit 3cd90214b70f ("vfio: ccw: add tracepoints for interesting error
paths") added a quick trace point to determine where a channel program
failed while being processed.  It's a great addition, but adding more
traces to vfio-ccw is more cumbersome than it needs to be.

Let's refactor how this is done, so that additional traces are easier
to add and can exist outside of the FSM if we ever desire.

Signed-off-by: Eric Farman <farman@linux.ibm.com>
---
 drivers/s390/cio/Makefile         |  4 ++--
 drivers/s390/cio/vfio_ccw_cp.h    |  1 +
 drivers/s390/cio/vfio_ccw_fsm.c   |  3 ---
 drivers/s390/cio/vfio_ccw_trace.c | 12 ++++++++++++
 drivers/s390/cio/vfio_ccw_trace.h |  2 ++
 5 files changed, 17 insertions(+), 5 deletions(-)
 create mode 100644 drivers/s390/cio/vfio_ccw_trace.c

diff --git a/drivers/s390/cio/Makefile b/drivers/s390/cio/Makefile
index f6a8db04177c..23eae4188876 100644
--- a/drivers/s390/cio/Makefile
+++ b/drivers/s390/cio/Makefile
@@ -5,7 +5,7 @@
 
 # The following is required for define_trace.h to find ./trace.h
 CFLAGS_trace.o := -I$(src)
-CFLAGS_vfio_ccw_fsm.o := -I$(src)
+CFLAGS_vfio_ccw_trace.o := -I$(src)
 
 obj-y += airq.o blacklist.o chsc.o cio.o css.o chp.o idset.o isc.o \
 	fcx.o itcw.o crw.o ccwreq.o trace.o ioasm.o
@@ -21,5 +21,5 @@ qdio-objs := qdio_main.o qdio_thinint.o qdio_debug.o qdio_setup.o
 obj-$(CONFIG_QDIO) += qdio.o
 
 vfio_ccw-objs += vfio_ccw_drv.o vfio_ccw_cp.o vfio_ccw_ops.o vfio_ccw_fsm.o \
-	vfio_ccw_async.o
+	vfio_ccw_async.o vfio_ccw_trace.o
 obj-$(CONFIG_VFIO_CCW) += vfio_ccw.o
diff --git a/drivers/s390/cio/vfio_ccw_cp.h b/drivers/s390/cio/vfio_ccw_cp.h
index 7cdc38049033..ba31240ce965 100644
--- a/drivers/s390/cio/vfio_ccw_cp.h
+++ b/drivers/s390/cio/vfio_ccw_cp.h
@@ -15,6 +15,7 @@
 #include <asm/scsw.h>
 
 #include "orb.h"
+#include "vfio_ccw_trace.h"
 
 /*
  * Max length for ccw chain.
diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c
index 4a1e727c62d9..d4119e4c4a8c 100644
--- a/drivers/s390/cio/vfio_ccw_fsm.c
+++ b/drivers/s390/cio/vfio_ccw_fsm.c
@@ -15,9 +15,6 @@
 #include "ioasm.h"
 #include "vfio_ccw_private.h"
 
-#define CREATE_TRACE_POINTS
-#include "vfio_ccw_trace.h"
-
 static int fsm_io_helper(struct vfio_ccw_private *private)
 {
 	struct subchannel *sch;
diff --git a/drivers/s390/cio/vfio_ccw_trace.c b/drivers/s390/cio/vfio_ccw_trace.c
new file mode 100644
index 000000000000..d5cc943c6864
--- /dev/null
+++ b/drivers/s390/cio/vfio_ccw_trace.c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Tracepoint definitions for vfio_ccw
+ *
+ * Copyright IBM Corp. 2019
+ * Author(s): Eric Farman <farman@linux.ibm.com>
+ */
+
+#define CREATE_TRACE_POINTS
+#include "vfio_ccw_trace.h"
+
+EXPORT_TRACEPOINT_SYMBOL(vfio_ccw_io_fctl);
diff --git a/drivers/s390/cio/vfio_ccw_trace.h b/drivers/s390/cio/vfio_ccw_trace.h
index b1da53ddec1f..2a2937a40124 100644
--- a/drivers/s390/cio/vfio_ccw_trace.h
+++ b/drivers/s390/cio/vfio_ccw_trace.h
@@ -7,6 +7,8 @@
  *            Halil Pasic <pasic@linux.vnet.ibm.com>
  */
 
+#include "cio.h"
+
 #undef TRACE_SYSTEM
 #define TRACE_SYSTEM vfio_ccw
 
-- 
2.17.1

Re: [PATCH v2 1/4] vfio-ccw: Refactor how the traces are built

[Cornelia Huck]

On Wed, 16 Oct 2019 03:58:19 +0200
Eric Farman <farman@linux.ibm.com> wrote:

> Commit 3cd90214b70f ("vfio: ccw: add tracepoints for interesting error
> paths") added a quick trace point to determine where a channel program
> failed while being processed.  It's a great addition, but adding more
> traces to vfio-ccw is more cumbersome than it needs to be.
> 
> Let's refactor how this is done, so that additional traces are easier
> to add and can exist outside of the FSM if we ever desire.
> 
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
>  drivers/s390/cio/Makefile         |  4 ++--
>  drivers/s390/cio/vfio_ccw_cp.h    |  1 +
>  drivers/s390/cio/vfio_ccw_fsm.c   |  3 ---
>  drivers/s390/cio/vfio_ccw_trace.c | 12 ++++++++++++
>  drivers/s390/cio/vfio_ccw_trace.h |  2 ++
>  5 files changed, 17 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/s390/cio/vfio_ccw_trace.c

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

[PATCH v2 2/4] vfio-ccw: Trace the FSM jumptable

[Eric Farman]

It would be nice if we could track the sequence of events within
vfio-ccw, based on the state of the device/FSM and our calling
sequence within it.  So let's add a simple trace here so we can
watch the states change as things go, and allow it to be folded
into the rest of the other cio traces.

Signed-off-by: Eric Farman <farman@linux.ibm.com>
---
 drivers/s390/cio/vfio_ccw_private.h |  1 +
 drivers/s390/cio/vfio_ccw_trace.c   |  1 +
 drivers/s390/cio/vfio_ccw_trace.h   | 26 ++++++++++++++++++++++++++
 3 files changed, 28 insertions(+)

diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h
index bbe9babf767b..9b9bb4982972 100644
--- a/drivers/s390/cio/vfio_ccw_private.h
+++ b/drivers/s390/cio/vfio_ccw_private.h
@@ -135,6 +135,7 @@ extern fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS];
 static inline void vfio_ccw_fsm_event(struct vfio_ccw_private *private,
 				     int event)
 {
+	trace_vfio_ccw_fsm_event(private->sch->schid, private->state, event);
 	vfio_ccw_jumptable[private->state][event](private, event);
 }
 
diff --git a/drivers/s390/cio/vfio_ccw_trace.c b/drivers/s390/cio/vfio_ccw_trace.c
index d5cc943c6864..b37bc68e7f18 100644
--- a/drivers/s390/cio/vfio_ccw_trace.c
+++ b/drivers/s390/cio/vfio_ccw_trace.c
@@ -9,4 +9,5 @@
 #define CREATE_TRACE_POINTS
 #include "vfio_ccw_trace.h"
 
+EXPORT_TRACEPOINT_SYMBOL(vfio_ccw_fsm_event);
 EXPORT_TRACEPOINT_SYMBOL(vfio_ccw_io_fctl);
diff --git a/drivers/s390/cio/vfio_ccw_trace.h b/drivers/s390/cio/vfio_ccw_trace.h
index 2a2937a40124..5005d57901b4 100644
--- a/drivers/s390/cio/vfio_ccw_trace.h
+++ b/drivers/s390/cio/vfio_ccw_trace.h
@@ -17,6 +17,32 @@
 
 #include <linux/tracepoint.h>
 
+TRACE_EVENT(vfio_ccw_fsm_event,
+	TP_PROTO(struct subchannel_id schid, int state, int event),
+	TP_ARGS(schid, state, event),
+
+	TP_STRUCT__entry(
+		__field(u8, cssid)
+		__field(u8, ssid)
+		__field(u16, schno)
+		__field(int, state)
+		__field(int, event)
+	),
+
+	TP_fast_assign(
+		__entry->cssid = schid.cssid;
+		__entry->ssid = schid.ssid;
+		__entry->schno = schid.sch_no;
+		__entry->state = state;
+		__entry->event = event;
+	),
+
+	TP_printk("schid=%x.%x.%04x state=%d event=%d",
+		__entry->cssid, __entry->ssid, __entry->schno,
+		__entry->state,
+		__entry->event)
+);
+
 TRACE_EVENT(vfio_ccw_io_fctl,
 	TP_PROTO(int fctl, struct subchannel_id schid, int errno, char *errstr),
 	TP_ARGS(fctl, schid, errno, errstr),
-- 
2.17.1

Re: [PATCH v2 2/4] vfio-ccw: Trace the FSM jumptable

[Cornelia Huck]

On Wed, 16 Oct 2019 03:58:20 +0200
Eric Farman <farman@linux.ibm.com> wrote:

> It would be nice if we could track the sequence of events within
> vfio-ccw, based on the state of the device/FSM and our calling
> sequence within it.  So let's add a simple trace here so we can
> watch the states change as things go, and allow it to be folded
> into the rest of the other cio traces.
> 
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
>  drivers/s390/cio/vfio_ccw_private.h |  1 +
>  drivers/s390/cio/vfio_ccw_trace.c   |  1 +
>  drivers/s390/cio/vfio_ccw_trace.h   | 26 ++++++++++++++++++++++++++
>  3 files changed, 28 insertions(+)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

[PATCH v2 3/4] vfio-ccw: Add a trace for asynchronous requests

[Eric Farman]

Since the asynchronous requests are typically associated with
error recovery, let's add a simple trace when one of those is
issued to a device.

Signed-off-by: Eric Farman <farman@linux.ibm.com>
---
 drivers/s390/cio/vfio_ccw_fsm.c   |  4 ++++
 drivers/s390/cio/vfio_ccw_trace.c |  1 +
 drivers/s390/cio/vfio_ccw_trace.h | 30 ++++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+)

diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c
index d4119e4c4a8c..23648a9aa721 100644
--- a/drivers/s390/cio/vfio_ccw_fsm.c
+++ b/drivers/s390/cio/vfio_ccw_fsm.c
@@ -341,6 +341,10 @@ static void fsm_async_request(struct vfio_ccw_private *private,
 		/* should not happen? */
 		cmd_region->ret_code = -EINVAL;
 	}
+
+	trace_vfio_ccw_fsm_async_request(get_schid(private),
+					 cmd_region->command,
+					 cmd_region->ret_code);
 }
 
 /*
diff --git a/drivers/s390/cio/vfio_ccw_trace.c b/drivers/s390/cio/vfio_ccw_trace.c
index b37bc68e7f18..37ecbf8be805 100644
--- a/drivers/s390/cio/vfio_ccw_trace.c
+++ b/drivers/s390/cio/vfio_ccw_trace.c
@@ -9,5 +9,6 @@
 #define CREATE_TRACE_POINTS
 #include "vfio_ccw_trace.h"
 
+EXPORT_TRACEPOINT_SYMBOL(vfio_ccw_fsm_async_request);
 EXPORT_TRACEPOINT_SYMBOL(vfio_ccw_fsm_event);
 EXPORT_TRACEPOINT_SYMBOL(vfio_ccw_io_fctl);
diff --git a/drivers/s390/cio/vfio_ccw_trace.h b/drivers/s390/cio/vfio_ccw_trace.h
index 5005d57901b4..23b288eb53dc 100644
--- a/drivers/s390/cio/vfio_ccw_trace.h
+++ b/drivers/s390/cio/vfio_ccw_trace.h
@@ -17,6 +17,36 @@
 
 #include <linux/tracepoint.h>
 
+TRACE_EVENT(vfio_ccw_fsm_async_request,
+	TP_PROTO(struct subchannel_id schid,
+		 int command,
+		 int errno),
+	TP_ARGS(schid, command, errno),
+
+	TP_STRUCT__entry(
+		__field(u8, cssid)
+		__field(u8, ssid)
+		__field(u16, sch_no)
+		__field(int, command)
+		__field(int, errno)
+	),
+
+	TP_fast_assign(
+		__entry->cssid = schid.cssid;
+		__entry->ssid = schid.ssid;
+		__entry->sch_no = schid.sch_no;
+		__entry->command = command;
+		__entry->errno = errno;
+	),
+
+	TP_printk("schid=%x.%x.%04x command=%d errno=%d",
+		  __entry->cssid,
+		  __entry->ssid,
+		  __entry->sch_no,
+		  __entry->command,
+		  __entry->errno)
+);
+
 TRACE_EVENT(vfio_ccw_fsm_event,
 	TP_PROTO(struct subchannel_id schid, int state, int event),
 	TP_ARGS(schid, state, event),
-- 
2.17.1

Re: [PATCH v2 3/4] vfio-ccw: Add a trace for asynchronous requests

[Cornelia Huck]

On Wed, 16 Oct 2019 03:58:21 +0200
Eric Farman <farman@linux.ibm.com> wrote:

> Since the asynchronous requests are typically associated with
> error recovery, let's add a simple trace when one of those is
> issued to a device.
> 
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
>  drivers/s390/cio/vfio_ccw_fsm.c   |  4 ++++
>  drivers/s390/cio/vfio_ccw_trace.c |  1 +
>  drivers/s390/cio/vfio_ccw_trace.h | 30 ++++++++++++++++++++++++++++++
>  3 files changed, 35 insertions(+)

(...)

> diff --git a/drivers/s390/cio/vfio_ccw_trace.h b/drivers/s390/cio/vfio_ccw_trace.h
> index 5005d57901b4..23b288eb53dc 100644
> --- a/drivers/s390/cio/vfio_ccw_trace.h
> +++ b/drivers/s390/cio/vfio_ccw_trace.h
> @@ -17,6 +17,36 @@
>  
>  #include <linux/tracepoint.h>
>  
> +TRACE_EVENT(vfio_ccw_fsm_async_request,
> +	TP_PROTO(struct subchannel_id schid,
> +		 int command,
> +		 int errno),
> +	TP_ARGS(schid, command, errno),
> +
> +	TP_STRUCT__entry(
> +		__field(u8, cssid)
> +		__field(u8, ssid)
> +		__field(u16, sch_no)
> +		__field(int, command)
> +		__field(int, errno)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->cssid = schid.cssid;
> +		__entry->ssid = schid.ssid;
> +		__entry->sch_no = schid.sch_no;
> +		__entry->command = command;
> +		__entry->errno = errno;
> +	),
> +
> +	TP_printk("schid=%x.%x.%04x command=%d errno=%d",

I'd probably rather print the command as a hex value.

> +		  __entry->cssid,
> +		  __entry->ssid,
> +		  __entry->sch_no,
> +		  __entry->command,
> +		  __entry->errno)
> +);
> +
>  TRACE_EVENT(vfio_ccw_fsm_event,
>  	TP_PROTO(struct subchannel_id schid, int state, int event),
>  	TP_ARGS(schid, state, event),

Re: [PATCH v2 3/4] vfio-ccw: Add a trace for asynchronous requests

[Eric Farman]

On 10/16/19 6:15 AM, Cornelia Huck wrote:
> On Wed, 16 Oct 2019 03:58:21 +0200
> Eric Farman <farman@linux.ibm.com> wrote:
> 
>> Since the asynchronous requests are typically associated with
>> error recovery, let's add a simple trace when one of those is
>> issued to a device.
>>
>> Signed-off-by: Eric Farman <farman@linux.ibm.com>
>> ---
>>  drivers/s390/cio/vfio_ccw_fsm.c   |  4 ++++
>>  drivers/s390/cio/vfio_ccw_trace.c |  1 +
>>  drivers/s390/cio/vfio_ccw_trace.h | 30 ++++++++++++++++++++++++++++++
>>  3 files changed, 35 insertions(+)
> 
> (...)
> 
>> diff --git a/drivers/s390/cio/vfio_ccw_trace.h b/drivers/s390/cio/vfio_ccw_trace.h
>> index 5005d57901b4..23b288eb53dc 100644
>> --- a/drivers/s390/cio/vfio_ccw_trace.h
>> +++ b/drivers/s390/cio/vfio_ccw_trace.h
>> @@ -17,6 +17,36 @@
>>  
>>  #include <linux/tracepoint.h>
>>  
>> +TRACE_EVENT(vfio_ccw_fsm_async_request,
>> +	TP_PROTO(struct subchannel_id schid,
>> +		 int command,
>> +		 int errno),
>> +	TP_ARGS(schid, command, errno),
>> +
>> +	TP_STRUCT__entry(
>> +		__field(u8, cssid)
>> +		__field(u8, ssid)
>> +		__field(u16, sch_no)
>> +		__field(int, command)
>> +		__field(int, errno)
>> +	),
>> +
>> +	TP_fast_assign(
>> +		__entry->cssid = schid.cssid;
>> +		__entry->ssid = schid.ssid;
>> +		__entry->sch_no = schid.sch_no;
>> +		__entry->command = command;
>> +		__entry->errno = errno;
>> +	),
>> +
>> +	TP_printk("schid=%x.%x.%04x command=%d errno=%d",
> 
> I'd probably rather print the command as a hex value.

I'm fine with that too.  Want me to send an update?

> 
>> +		  __entry->cssid,
>> +		  __entry->ssid,
>> +		  __entry->sch_no,
>> +		  __entry->command,
>> +		  __entry->errno)
>> +);
>> +
>>  TRACE_EVENT(vfio_ccw_fsm_event,
>>  	TP_PROTO(struct subchannel_id schid, int state, int event),
>>  	TP_ARGS(schid, state, event),
> 

Re: [PATCH v2 3/4] vfio-ccw: Add a trace for asynchronous requests

[Cornelia Huck]

On Wed, 16 Oct 2019 07:36:09 -0400
Eric Farman <farman@linux.ibm.com> wrote:

> On 10/16/19 6:15 AM, Cornelia Huck wrote:
> > On Wed, 16 Oct 2019 03:58:21 +0200
> > Eric Farman <farman@linux.ibm.com> wrote:
> >   
> >> Since the asynchronous requests are typically associated with
> >> error recovery, let's add a simple trace when one of those is
> >> issued to a device.
> >>
> >> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> >> ---
> >>  drivers/s390/cio/vfio_ccw_fsm.c   |  4 ++++
> >>  drivers/s390/cio/vfio_ccw_trace.c |  1 +
> >>  drivers/s390/cio/vfio_ccw_trace.h | 30 ++++++++++++++++++++++++++++++
> >>  3 files changed, 35 insertions(+)  
> > 
> > (...)
> >   
> >> diff --git a/drivers/s390/cio/vfio_ccw_trace.h b/drivers/s390/cio/vfio_ccw_trace.h
> >> index 5005d57901b4..23b288eb53dc 100644
> >> --- a/drivers/s390/cio/vfio_ccw_trace.h
> >> +++ b/drivers/s390/cio/vfio_ccw_trace.h
> >> @@ -17,6 +17,36 @@
> >>  
> >>  #include <linux/tracepoint.h>
> >>  
> >> +TRACE_EVENT(vfio_ccw_fsm_async_request,
> >> +	TP_PROTO(struct subchannel_id schid,
> >> +		 int command,
> >> +		 int errno),
> >> +	TP_ARGS(schid, command, errno),
> >> +
> >> +	TP_STRUCT__entry(
> >> +		__field(u8, cssid)
> >> +		__field(u8, ssid)
> >> +		__field(u16, sch_no)
> >> +		__field(int, command)
> >> +		__field(int, errno)
> >> +	),
> >> +
> >> +	TP_fast_assign(
> >> +		__entry->cssid = schid.cssid;
> >> +		__entry->ssid = schid.ssid;
> >> +		__entry->sch_no = schid.sch_no;
> >> +		__entry->command = command;
> >> +		__entry->errno = errno;
> >> +	),
> >> +
> >> +	TP_printk("schid=%x.%x.%04x command=%d errno=%d",  
> > 
> > I'd probably rather print the command as a hex value.  
> 
> I'm fine with that too.  Want me to send an update?

I think that would be the easiest way.

> 
> >   
> >> +		  __entry->cssid,
> >> +		  __entry->ssid,
> >> +		  __entry->sch_no,
> >> +		  __entry->command,
> >> +		  __entry->errno)
> >> +);
> >> +
> >>  TRACE_EVENT(vfio_ccw_fsm_event,
> >>  	TP_PROTO(struct subchannel_id schid, int state, int event),
> >>  	TP_ARGS(schid, state, event),  
> >   

Re: [PATCH v2 3/4] vfio-ccw: Add a trace for asynchronous requests

[Eric Farman]

On 10/16/19 7:39 AM, Cornelia Huck wrote:
> On Wed, 16 Oct 2019 07:36:09 -0400
> Eric Farman <farman@linux.ibm.com> wrote:
> 
>> On 10/16/19 6:15 AM, Cornelia Huck wrote:
>>> On Wed, 16 Oct 2019 03:58:21 +0200
>>> Eric Farman <farman@linux.ibm.com> wrote:
>>>   
>>>> Since the asynchronous requests are typically associated with
>>>> error recovery, let's add a simple trace when one of those is
>>>> issued to a device.
>>>>
>>>> Signed-off-by: Eric Farman <farman@linux.ibm.com>
>>>> ---
>>>>  drivers/s390/cio/vfio_ccw_fsm.c   |  4 ++++
>>>>  drivers/s390/cio/vfio_ccw_trace.c |  1 +
>>>>  drivers/s390/cio/vfio_ccw_trace.h | 30 ++++++++++++++++++++++++++++++
>>>>  3 files changed, 35 insertions(+)  
>>>
>>> (...)
>>>   
>>>> diff --git a/drivers/s390/cio/vfio_ccw_trace.h b/drivers/s390/cio/vfio_ccw_trace.h
>>>> index 5005d57901b4..23b288eb53dc 100644
>>>> --- a/drivers/s390/cio/vfio_ccw_trace.h
>>>> +++ b/drivers/s390/cio/vfio_ccw_trace.h
>>>> @@ -17,6 +17,36 @@
>>>>  
>>>>  #include <linux/tracepoint.h>
>>>>  
>>>> +TRACE_EVENT(vfio_ccw_fsm_async_request,
>>>> +	TP_PROTO(struct subchannel_id schid,
>>>> +		 int command,
>>>> +		 int errno),
>>>> +	TP_ARGS(schid, command, errno),
>>>> +
>>>> +	TP_STRUCT__entry(
>>>> +		__field(u8, cssid)
>>>> +		__field(u8, ssid)
>>>> +		__field(u16, sch_no)
>>>> +		__field(int, command)
>>>> +		__field(int, errno)
>>>> +	),
>>>> +
>>>> +	TP_fast_assign(
>>>> +		__entry->cssid = schid.cssid;
>>>> +		__entry->ssid = schid.ssid;
>>>> +		__entry->sch_no = schid.sch_no;
>>>> +		__entry->command = command;
>>>> +		__entry->errno = errno;
>>>> +	),
>>>> +
>>>> +	TP_printk("schid=%x.%x.%04x command=%d errno=%d",  
>>>
>>> I'd probably rather print the command as a hex value.  
>>
>> I'm fine with that too.  Want me to send an update?
> 
> I think that would be the easiest way.

Will do.  Thanks for the reviews on the other ones!

 - Eric

> 
>>
>>>   
>>>> +		  __entry->cssid,
>>>> +		  __entry->ssid,
>>>> +		  __entry->sch_no,
>>>> +		  __entry->command,
>>>> +		  __entry->errno)
>>>> +);
>>>> +
>>>>  TRACE_EVENT(vfio_ccw_fsm_event,
>>>>  	TP_PROTO(struct subchannel_id schid, int state, int event),
>>>>  	TP_ARGS(schid, state, event),  
>>>   
> 

[PATCH v2 4/4] vfio-ccw: Rework the io_fctl trace

[Eric Farman]

Using __field_struct for the schib is convenient, but it doesn't
appear to let us filter based on any of the schib elements.
Specifying, the full schid or any element within it results
in various errors by the parser.  So, expand that out to its
component elements, so we can limit the trace to a single device.

While we are at it, rename this trace to the function name, so we
remember what is being traced instead of an abstract reference to the
function control bit of the SCSW.

Signed-off-by: Eric Farman <farman@linux.ibm.com>
---
 drivers/s390/cio/vfio_ccw_fsm.c   |  4 ++--
 drivers/s390/cio/vfio_ccw_trace.c |  2 +-
 drivers/s390/cio/vfio_ccw_trace.h | 18 +++++++++++-------
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c
index 23648a9aa721..23e61aa638e4 100644
--- a/drivers/s390/cio/vfio_ccw_fsm.c
+++ b/drivers/s390/cio/vfio_ccw_fsm.c
@@ -318,8 +318,8 @@ static void fsm_io_request(struct vfio_ccw_private *private,
 	}
 
 err_out:
-	trace_vfio_ccw_io_fctl(scsw->cmd.fctl, schid,
-			       io_region->ret_code, errstr);
+	trace_vfio_ccw_fsm_io_request(scsw->cmd.fctl, schid,
+				      io_region->ret_code, errstr);
 }
 
 /*
diff --git a/drivers/s390/cio/vfio_ccw_trace.c b/drivers/s390/cio/vfio_ccw_trace.c
index 37ecbf8be805..8c671d2519f6 100644
--- a/drivers/s390/cio/vfio_ccw_trace.c
+++ b/drivers/s390/cio/vfio_ccw_trace.c
@@ -11,4 +11,4 @@
 
 EXPORT_TRACEPOINT_SYMBOL(vfio_ccw_fsm_async_request);
 EXPORT_TRACEPOINT_SYMBOL(vfio_ccw_fsm_event);
-EXPORT_TRACEPOINT_SYMBOL(vfio_ccw_io_fctl);
+EXPORT_TRACEPOINT_SYMBOL(vfio_ccw_fsm_io_request);
diff --git a/drivers/s390/cio/vfio_ccw_trace.h b/drivers/s390/cio/vfio_ccw_trace.h
index 23b288eb53dc..f2de5d23c312 100644
--- a/drivers/s390/cio/vfio_ccw_trace.h
+++ b/drivers/s390/cio/vfio_ccw_trace.h
@@ -73,28 +73,32 @@ TRACE_EVENT(vfio_ccw_fsm_event,
 		__entry->event)
 );
 
-TRACE_EVENT(vfio_ccw_io_fctl,
+TRACE_EVENT(vfio_ccw_fsm_io_request,
 	TP_PROTO(int fctl, struct subchannel_id schid, int errno, char *errstr),
 	TP_ARGS(fctl, schid, errno, errstr),
 
 	TP_STRUCT__entry(
+		__field(u8, cssid)
+		__field(u8, ssid)
+		__field(u16, sch_no)
 		__field(int, fctl)
-		__field_struct(struct subchannel_id, schid)
 		__field(int, errno)
 		__field(char*, errstr)
 	),
 
 	TP_fast_assign(
+		__entry->cssid = schid.cssid;
+		__entry->ssid = schid.ssid;
+		__entry->sch_no = schid.sch_no;
 		__entry->fctl = fctl;
-		__entry->schid = schid;
 		__entry->errno = errno;
 		__entry->errstr = errstr;
 	),
 
-	TP_printk("schid=%x.%x.%04x fctl=%x errno=%d info=%s",
-		  __entry->schid.cssid,
-		  __entry->schid.ssid,
-		  __entry->schid.sch_no,
+	TP_printk("schid=%x.%x.%04x fctl=0x%x errno=%d info=%s",
+		  __entry->cssid,
+		  __entry->ssid,
+		  __entry->sch_no,
 		  __entry->fctl,
 		  __entry->errno,
 		  __entry->errstr)
-- 
2.17.1

Re: [PATCH v2 4/4] vfio-ccw: Rework the io_fctl trace

[Cornelia Huck]

On Wed, 16 Oct 2019 03:58:22 +0200
Eric Farman <farman@linux.ibm.com> wrote:

> Using __field_struct for the schib is convenient, but it doesn't
> appear to let us filter based on any of the schib elements.
> Specifying, the full schid or any element within it results

s/Specifying,/Specifying/

> in various errors by the parser.  So, expand that out to its
> component elements, so we can limit the trace to a single device.
> 
> While we are at it, rename this trace to the function name, so we
> remember what is being traced instead of an abstract reference to the
> function control bit of the SCSW.
> 
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
>  drivers/s390/cio/vfio_ccw_fsm.c   |  4 ++--
>  drivers/s390/cio/vfio_ccw_trace.c |  2 +-
>  drivers/s390/cio/vfio_ccw_trace.h | 18 +++++++++++-------
>  3 files changed, 14 insertions(+), 10 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>