lkml.org 
[lkml]   [2010]   [Jul]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [rfc] Describe events in a structured way via sysfs

* Lin Ming <ming.m.lin@intel.com> wrote:

> On Tue, 2010-06-29 at 18:26 +0800, Ingo Molnar wrote:
> > * Lin Ming <ming.m.lin@intel.com> wrote:
> >
> > > > Also, we can (optionally) consider 'generic', subsystem level events to
> > > > also show up under:
> > > >
> > > > /sys/bus/pci/drivers/i915/events/
> > > >
> > > > This would give a model to non-device-specific events to be listed one
> > > > level higher in the sysfs hierarchy.
> > > >
> > > > This too would be done in the driver, not by generic code. It's generally
> > > > the driver which knows how the events should be categorized.
> > >
> > > This is a bit difficult. I'd like not to touch TRACE_EVENT(). [...]
> >
> > We can certainly start with the simpler variant - it's also the more common
> > case.
> >
> > > [...] How does the driver know if an event is 'generic' if TRACE_EVENT is
> > > not touched?
> >
> > Well, it's per driver code which creates the 'events' directory anyway, so
> > that code decides where to link things. It can link it to the per driver kobj
> > - or to the per subsys kobj.
> >
> > > > I'd imagine something similar for wireless drivers as well - most
> > > > currently defined events would show up on a per device basis there.
> > > >
> > > > Can you see practical problems with this scheme?
> > >
> > > Not now. I may find some problems when write more detail code.
> >
> > Ok. Feel free to post RFC patches (even if they are not fully complete yet),
> > so that we can see how things are progressing.
> >
> > I suspect the best approach would be to try to figure out the right sysfs
> > placement for one or two existing driver tracepoints, so that we can see it
> > all in practice. (Obviously any changes to drivers will have to go via the
> > relevant driver maintainer tree(s).)
>
> Well, take i915 tracepoints as an example, the sys structures as below
>
> /sys/class/drm/card0/events/
> |-- i915_gem_object_bind
> | |-- enable
> | |-- filter
> | |-- format
> | `-- id
> |-- i915_gem_object_change_domain
> | |-- enable
> | |-- filter
> | |-- format
> | `-- id
> |-- i915_gem_object_clflush
> | |-- enable
> | |-- filter
> | |-- format
> | `-- id
> |-- i915_gem_object_create
> | |-- enable
> | |-- filter
> | |-- format
> | `-- id
> |-- i915_gem_object_destroy
> | |-- enable
> | |-- filter
> | |-- format
> | `-- id
> |-- i915_gem_object_get_fence
> | |-- enable
> | |-- filter
> | |-- format
> | `-- id
> |-- i915_gem_object_unbind
> | |-- enable
> | |-- filter
> | |-- format
> | `-- id
> |-- i915_gem_request_complete
> | |-- enable
> | |-- filter
> | |-- format
> | `-- id
> |-- i915_gem_request_flush
> | |-- enable
> | |-- filter
> | |-- format
> | `-- id
> |-- i915_gem_request_retire
> | |-- enable
> | |-- filter
> | |-- format
> | `-- id
> |-- i915_gem_request_submit
> | |-- enable
> | |-- filter
> | |-- format
> | `-- id
> |-- i915_gem_request_wait_begin
> | |-- enable
> | |-- filter
> | |-- format
> | `-- id
> |-- i915_gem_request_wait_end
> | |-- enable
> | |-- filter
> | |-- format
> | `-- id
> |-- i915_ring_wait_begin
> | |-- enable
> | |-- filter
> | |-- format
> | `-- id
> `-- i915_ring_wait_end
> |-- enable
> |-- filter
> |-- format
> `-- id
>
> And below is the very draft patch to export i915 tracepoints in sysfs.
> Is it the right direction?

Yeah, i think so.

The per driver impact is small and to the point:

> drivers/gpu/drm/i915/i915_drv.c | 15 +++-

> i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> {
> - return drm_get_dev(pdev, ent, &driver);
> + struct kobject *kobj;
> + struct drm_device *drm_dev;
> + int ret;
> +
> + ret = drm_get_dev(pdev, ent, &driver);
> +
> + if (!ret) {
> + drm_dev = pci_get_drvdata(pdev);
> + kobj = &drm_dev->primary->kdev.kobj;
> + perf_sys_register_tp(kobj, "i915");
> + }
> +
> + return ret;

(It could be even shorter - the same compactness comment as i made last time
still holds for this function.)

Thanks,

Ingo


\
 
 \ /
  Last update: 2010-07-03 14:57    [from the cache]
©2003-2011 Jasper Spaans