[Intel-gfx] [PATCH] drm/i915: Add device name to display tracepoints

[Ville Syrjälä]

On Fri, Dec 13, 2019 at 07:06:01PM +0200, Ville Syrjälä wrote:
> On Fri, Dec 13, 2019 at 03:54:33PM +0000, Chris Wilson wrote:
> > Quoting Ville Syrjala (2019-12-13 15:28:23)
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > 
> > > Include dev_name() in the tracpoints so one can filter based on
> > > the device.
> > > 
> > > Example:
> > > echo 'dev=="0000:00:02.0"' > events/i915/intel_cpu_fifo_underrun/filter
> > > 
> > > TODO: maybe don't both specifying the field name always and just
> > >       make it 'dev' (or whatever) always?
> > > TODO: add for other tracpoints too if this is deemed good enough
> > > 
> > > Suggested-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_trace.h | 96 +++++++++++++++++++++----------
> > >  1 file changed, 65 insertions(+), 31 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> > > index 7ef7a1e1664c..8931b6756f44 100644
> > > --- a/drivers/gpu/drm/i915/i915_trace.h
> > > +++ b/drivers/gpu/drm/i915/i915_trace.h
> > > @@ -20,11 +20,18 @@
> > >  
> > >  /* watermark/fifo updates */
> > >  
> > > +#define __dev_name_i915(field, i915) __string(field, dev_name((i915)->drm.dev))
> > > +#define __dev_name_crtc(field, crtc) __string(field, dev_name((crtc)->base.dev->dev))
> > > +#define __assign_dev_name_i915(field, i915) __assign_str(field, dev_name((i915)->drm.dev))
> > > +#define __assign_dev_name_crtc(field, crtc) __assign_str(field, dev_name((crtc)->base.dev->dev))
> > > +#define __get_dev_name(field) __get_str(field)
> > 
> > Storing the string is quite expensive, I thought. Can we stash the i915
> > and stringify in the TP_printk? Or is stashing the string the secret for
> > the dev== filter?
> 
> Last time I stashed a pointer in there people complained that it can
> disappear before being consumed and cause a very theoretical oops.
> But I guess we could stash just the pci devfn and whatnot.

I believe 'domain + bus + devfn' would amount to 4 bytes. The downside
is that it'd be just an integer so we'd lose the dev=domain:bus:dev.fn
syntax for the filter. Suppose I could try to implement a new filter
type for it, but no guarantees that would get accepted. Seems a bit too
pci specific for kernel/trace/.

-- 
Ville Syrjälä
Intel