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    [W:0.040 / U:0.560 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site