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

[Cornelia Huck <cohuck@redhat.com>]

On Tue, 15 Oct 2019 12:01:12 +0200
Steffen Maier <maier@linux.ibm.com> wrote:

> On 10/14/19 8:08 PM, Eric Farman 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(+)

(...)

> > diff --git a/drivers/s390/cio/vfio_ccw_trace.h b/drivers/s390/cio/vfio_ccw_trace.h
> > index 2a2937a40124..24a8152acfdf 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=%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.

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.)

> 
> > +		__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),
> >   
> 
>