| Message ID | 20191014180855.19400-2-farman@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio-ccw: A couple trace changes | expand |
On Mon, 14 Oct 2019 20:08:52 +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 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.
On 10/15/19 9:46 AM, Cornelia Huck wrote: > On Mon, 14 Oct 2019 20:08:52 +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 > > 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
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
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