vfio-ccw: A couple trace changes
Subsystems (4)
Eric Farman
maintainercover vfio-ccw: A couple trace changes
Hi Conny,
I mentioned these to you a while ago; just been caught up in other things. These aren't really pre-reqs for anything in our pipe, but they've been beneficial to me on their own, and simplify debugging of future work (such as the FSM needs brought up by Halil).
While the first patch refactors the trace so new ones can be added outside the FSM, none of the other patches in this series really do. I imagine that would be beneficial in the future, so I think it would be good to do that rework before we get too far along.
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: Rename 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 | 56 ++++++++++++++++++++++++++++- 6 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 drivers/s390/cio/vfio_ccw_trace.c
Signature · 1 line
2.17.1
Eric Farman
maintainerRFC PATCH 1/4 vfio-ccw: Refactor how the traces are built
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.
--- 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
drivers/s390/cio/Makefile +2 -2
@@ -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
drivers/s390/cio/vfio_ccw_cp.h +1 -0
@@ -15,6 +15,7 @@#include <asm/scsw.h> #include "orb.h" +#include "vfio_ccw_trace.h" /* * Max length for ccw chain.
drivers/s390/cio/vfio_ccw_fsm.c +0 -3
@@ -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;
drivers/s390/cio/vfio_ccw_trace.c new file +12 -0
@@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Tracepoint definitions for vfio_ccw +* +* Copyright IBM Corp. 2019 +* Author(s): Eric Farman <[email protected]> +*/ + +#define CREATE_TRACE_POINTS +#include "vfio_ccw_trace.h" + +EXPORT_TRACEPOINT_SYMBOL (vfio_ccw_io_fctl);
drivers/s390/cio/vfio_ccw_trace.h +2 -0
@@ -7,6 +7,8 @@* Halil Pasic <[email protected]> */ +#include "cio.h" +#undef TRACE_SYSTEM#define TRACE_SYSTEM vfio_ccw
Signature · 1 line
2.17.1
Re: [RFC PATCH 1/4] vfio-ccw: Refactor how the traces are built
Quoted 9 lines · On Mon, 14 Oct 2019 20:08:52 +0200 Eric Farman <[email protected]> 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 <[email protected]>
Quoted 8 lines
--- 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
Looks good.
I'm wondering whether we could consolidate tracepoints and s390dbf usage somehow. These two complement each other in a way (looking at a live system vs. looking at a crash dump, integration with other parts of the system), but they currently also cover at least partially different code paths. Not sure how much sense it makes to have double coverage at least for a subset of the functionality.
Eric Farman
maintainerRe: [RFC PATCH 1/4] vfio-ccw: Refactor how the traces are built
Quoted 21 lines · Cornelia Huck
Quoted 9 lines · On Mon, 14 Oct 2019 20:08:52 +0200 Eric Farman <[email protected]> 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 <[email protected]>Quoted 8 lines
--- 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
Quoted 9 lines
Looks good.
I'm wondering whether we could consolidate tracepoints and s390dbf usage somehow. These two complement each other in a way (looking at a live system vs. looking at a crash dump, integration with other parts of the system), but they currently also cover at least partially different code paths. Not sure how much sense it makes to have double coverage at least for a subset of the functionality.
Yeah, there's gaps that could/should be closed, and maybe this patch makes it easier to add traces to paths that s390dbf currently cover and could benefit from having both. Some more consideration of what is covered and where, and what is missing, is certainliy needed.
- Eric
Eric Farman
maintainerRFC PATCH 2/4 vfio-ccw: Trace the FSM jumptable
In reply to Eric FarmanIt 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.
--- 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(+)
drivers/s390/cio/vfio_ccw_private.h +1 -0
@@ -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); }
drivers/s390/cio/vfio_ccw_trace.c +1 -0
@@ -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);
drivers/s390/cio/vfio_ccw_trace.h +26 -0
@@ -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=%x event=%x" , + __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),
Signature · 1 line
2.17.1
Re: [RFC PATCH 2/4] vfio-ccw: Trace the FSM jumptable
Quoted 54 lines · 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 <[email protected]>--- 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(+)drivers/s390/cio/vfio_ccw_private.h +1
-0@@ -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); }drivers/s390/cio/vfio_ccw_trace.c +1
-0@@ -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);drivers/s390/cio/vfio_ccw_trace.h +11
-0@@ -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) + ),
drivers/s390/cio/vfio_ccw_trace.h line 40
Quoted 10 lines
drivers/s390/cio/vfio_ccw_trace.h +10
-0+ +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=%x event=%x",
/sys/kernel/debug/tracing/events](0)# grep -R '%[^%]*x'
Many existing TPs often seem to format hex output with a 0x prefix (either explicit with 0x%x or implicit with %#x). Since some of your other TPs also output decimal integer values, I wonder if a distinction would help unexperienced TP readers.
Quoted 9 lines
drivers/s390/cio/vfio_ccw_trace.h +5
-0+ __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),
Signature · 11 lines
Mit freundlichen Gruessen / Kind regards Steffen Maier Linux on IBM Z Development https://www.ibm.com/privacy/us/en/ IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Matthias Hartmann Geschaeftsfuehrung: Dirk Wittkopp Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Eric Farman
maintainerRe: [RFC PATCH 2/4] vfio-ccw: Trace the FSM jumptable
Quoted 73 lines · Steffen Maier
Quoted 59 lines · 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 <[email protected]>---
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(+)drivers/s390/cio/vfio_ccw_private.h +1
-0@@ -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);}
drivers/s390/cio/vfio_ccw_trace.c +1
-0@@ -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);drivers/s390/cio/vfio_ccw_trace.h +11
-0@@ -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) + ),drivers/s390/cio/vfio_ccw_trace.h line 40
Quoted 10 lines
drivers/s390/cio/vfio_ccw_trace.h +10
-0+ + 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=%x event=%x",/sys/kernel/debug/tracing/events](0)# grep -R '%[^%]*x'
Quoted 4 lines
Many existing TPs often seem to format hex output with a 0x prefix (either explicit with 0x%x or implicit with %#x). Since some of your other TPs also output decimal integer values, I wonder if a distinction would help unexperienced TP readers.
Fair enough. Since they're just enumerated values, they are probably better as %d; I don't have a good reason for picking %x (with or without a preceding 0x).
Quoted 13 lines
Quoted 11 lines
drivers/s390/cio/vfio_ccw_trace.h +5
-0+ __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),
Re: [RFC PATCH 2/4] vfio-ccw: Trace the FSM jumptable
In reply to Steffen MaierQuoted 13 lines · On Tue, 15 Oct 2019 12:01:12 +0200 Steffen Maier <[email protected]> wrote:
Quoted 12 lines · 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 <[email protected]>--- 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(+)
(...)
Quoted 32 lines
Quoted 19 lines
drivers/s390/cio/vfio_ccw_trace.h +11
-0@@ -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) + ),drivers/s390/cio/vfio_ccw_trace.h line 40
Quoted 10 lines
drivers/s390/cio/vfio_ccw_trace.h +10
-0+ +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=%x event=%x",/sys/kernel/debug/tracing/events](0)# grep -R '%[^%]*x'
Quoted 4 lines
Many existing TPs often seem to format hex output with a 0x prefix (either explicit with 0x%x or implicit with %#x). Since some of your other TPs also output decimal integer values, I wonder if a distinction would help unexperienced TP readers.
I generally agree. However, we explicitly don't want to do that for schid formatting (as it should match the bus id). For event, it might become relevant should we want to introduce a high number of new events in the future (currently, there's a grand total of four events.)
Quoted 12 lines
Quoted 10 lines
drivers/s390/cio/vfio_ccw_trace.h +5
-0+ __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),
Re: [RFC PATCH 2/4] vfio-ccw: Trace the FSM jumptable
Quoted 56 lines · Cornelia Huck
Quoted 13 lines · On Tue, 15 Oct 2019 12:01:12 +0200 Steffen Maier <[email protected]> wrote:
Quoted 12 lines · 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 <[email protected]>--- 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(+)(...)
Quoted 32 lines
Quoted 19 lines
drivers/s390/cio/vfio_ccw_trace.h +11
-0@@ -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) + ),drivers/s390/cio/vfio_ccw_trace.h line 40
Quoted 10 lines
drivers/s390/cio/vfio_ccw_trace.h +10
-0+ +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=%x event=%x",/sys/kernel/debug/tracing/events](0)# grep -R '%[^%]*x'
Quoted 4 lines
Many existing TPs often seem to format hex output with a 0x prefix (either explicit with 0x%x or implicit with %#x). Since some of your other TPs also output decimal integer values, I wonder if a distinction would help unexperienced TP readers.
Quoted 4 lines
I generally agree. However, we explicitly don't want to do that for schid formatting (as it should match the bus id). For event, it might become relevant should we want to introduce a high number of new events in the future (currently, there's a grand total of four events.)
Yeah, thanks for clarifying. I meant just state and event, not schid.
Quoted 14 lines
Quoted 13 lines
Quoted 11 lines
drivers/s390/cio/vfio_ccw_trace.h +5
-0+ __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),
Signature · 11 lines
Mit freundlichen Gruessen / Kind regards Steffen Maier Linux on IBM Z Development https://www.ibm.com/privacy/us/en/ IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Matthias Hartmann Geschaeftsfuehrung: Dirk Wittkopp Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Eric Farman
maintainerRFC PATCH 3/4 vfio-ccw: Add a trace for asynchronous requests
In reply to Eric FarmanSince the asynchronous requests are typically associated with error recovery, let's add a simple trace when one of those is issued to a device.
--- drivers/s390/cio/vfio_ccw_fsm.c | 4 ++++ drivers/s390/cio/vfio_ccw_trace.c | 1 + drivers/s390/cio/vfio_ccw_trace.h | 26 ++++++++++++++++++++++++++ 3 files changed, 31 insertions(+)
drivers/s390/cio/vfio_ccw_fsm.c +4 -0
@@ -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); }/*
drivers/s390/cio/vfio_ccw_trace.c +1 -0
@@ -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);
drivers/s390/cio/vfio_ccw_trace.h +26 -0
@@ -17,6 +17,32 @@#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_struct(struct subchannel_id, schid) + __field(int, command) +__field (int, errno) + ), + +TP_fast_assign ( + __entry->schid = schid; + __entry->command = command; + __entry->errno = errno; + ), + +TP_printk ("schid=%x.%x.%04x command=%d errno=%d" , + __entry->schid.cssid, + __entry->schid.ssid, + __entry->schid.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),
Signature · 1 line
2.17.1
Re: [RFC PATCH 3/4] vfio-ccw: Add a trace for asynchronous requests
Quoted 43 lines · 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 <[email protected]>--- drivers/s390/cio/vfio_ccw_fsm.c | 4 ++++ drivers/s390/cio/vfio_ccw_trace.c | 1 + drivers/s390/cio/vfio_ccw_trace.h | 26 ++++++++++++++++++++++++++ 3 files changed, 31 insertions(+)drivers/s390/cio/vfio_ccw_fsm.c +4
-0@@ -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); }/* drivers/s390/cio/vfio_ccw_trace.c +1
-0@@ -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);drivers/s390/cio/vfio_ccw_trace.h +0
-0@@ -17,6 +17,32 @@
drivers/s390/cio/vfio_ccw_trace.h line 27
Quoted 10 lines
drivers/s390/cio/vfio_ccw_trace.h +8
-0#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_struct(struct subchannel_id, schid)
Not sure: Does this allow the user to filter for fields of struct subchannel_id or can the user express a filter on the entire combined struct subchannel_id? In the preceding patch you have the 3 parts of schid as explicit separate trace fields in the tracepoint.
Quoted 22 lines
drivers/s390/cio/vfio_ccw_trace.h +18
-0+ __field(int, command) +__field (int, errno) + ), + +TP_fast_assign ( + __entry->schid = schid; + __entry->command = command; + __entry->errno = errno; + ), + +TP_printk ("schid=%x.%x.%04x command=%d errno=%d" , + __entry->schid.cssid, + __entry->schid.ssid, + __entry->schid.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),
Signature · 11 lines
Mit freundlichen Gruessen / Kind regards Steffen Maier Linux on IBM Z Development https://www.ibm.com/privacy/us/en/ IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Matthias Hartmann Geschaeftsfuehrung: Dirk Wittkopp Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Eric Farman
maintainerRe: [RFC PATCH 3/4] vfio-ccw: Add a trace for asynchronous requests
Quoted 50 lines · Steffen Maier
Quoted 49 lines · 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 <[email protected]>---
drivers/s390/cio/vfio_ccw_fsm.c | 4 ++++drivers/s390/cio/vfio_ccw_trace.c| 1 +drivers/s390/cio/vfio_ccw_trace.h| 26 ++++++++++++++++++++++++++3 files changed, 31 insertions(+)drivers/s390/cio/vfio_ccw_fsm.c +4
-0@@ -341,6 +341,10 @@ static void fsm_async_request(structvfio_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); }/*
drivers/s390/cio/vfio_ccw_trace.c +1
-0@@ -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);drivers/s390/cio/vfio_ccw_trace.h +0
-0@@ -17,6 +17,32 @@ #include <linux/tracepoint.h>
Quoted 12 lines
drivers/s390/cio/vfio_ccw_trace.h line 27
Quoted 8 lines
drivers/s390/cio/vfio_ccw_trace.h +8
-0+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_struct(struct subchannel_id, schid)Not sure: Does this allow the user to filter for fields of struct
subchannel_idor can the user express a filter on the entire combined structsubchannel_id?
Good question... I tried playing around with this a little bit, and while format says it's using "REC->schid.foo", trying any combination to get an element within schid just fails, citing some form of invalid argument (the exact reason depends on what I try). Harrumph.
Quoted 2 lines
In the preceding patch you have the 3 parts of schid as explicit separate trace fields in the tracepoint.
Yeah, I did. Why didn't I do that here? :)
Well it seems that the one trace that exists (vfio_ccw_fsm_io_request (nee vfio_ccw_fsm_io_fctl)), uses this same field_struct, instead of the three-piece schid. So, I need to change this in both the I/O and asynchronous traces.
Thanks for the suggestions!
- Eric
Quoted 25 lines
Quoted 23 lines
drivers/s390/cio/vfio_ccw_trace.h +18
-0+ __field(int, command) + __field(int, errno) + ), + + TP_fast_assign( + __entry->schid = schid; + __entry->command = command; + __entry->errno = errno; + ), + + TP_printk("schid=%x.%x.%04x command=%d errno=%d", + __entry->schid.cssid, + __entry->schid.ssid, + __entry->schid.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),
Eric Farman
maintainerRFC PATCH 4/4 vfio-ccw: Rename the io_fctl trace
In reply to Eric FarmanRename 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 (since that exists in the IRB, but not the ORB).
--- drivers/s390/cio/vfio_ccw_fsm.c | 4 ++-- drivers/s390/cio/vfio_ccw_trace.c | 2 +- drivers/s390/cio/vfio_ccw_trace.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)
drivers/s390/cio/vfio_ccw_fsm.c +2 -2
@@ -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); }/*
drivers/s390/cio/vfio_ccw_trace.c +1 -1
@@ -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);
drivers/s390/cio/vfio_ccw_trace.h +1 -1
@@ -69,7 +69,7 @@ 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),
Signature · 1 line
2.17.1
Re: [RFC PATCH 4/4] vfio-ccw: Rename the io_fctl trace
Quoted 10 lines · On Mon, 14 Oct 2019 20:08:55 +0200 Eric Farman <[email protected]> wrote:
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 (since that exists in the IRB, but not the ORB).
Signed-off-by Eric Farman <[email protected]>--- drivers/s390/cio/vfio_ccw_fsm.c | 4 ++-- drivers/s390/cio/vfio_ccw_trace.c | 2 +- drivers/s390/cio/vfio_ccw_trace.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)
That makes sense (I don't supposed this is used in any tooling, as it is more of a low-level debug trace.)