lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 29 Jun 2010 17:20:48 +0800 From: Lin Ming <ming.m.lin@...el.com> To: Ingo Molnar <mingo@...e.hu> Cc: Johannes Berg <johannes@...solutions.net>, Peter Zijlstra <peterz@...radead.org>, Greg KH <greg@...ah.com>, Corey Ashford <cjashfor@...ux.vnet.ibm.com>, Frederic Weisbecker <fweisbec@...il.com>, Paul Mundt <lethal@...ux-sh.org>, "eranian@...il.com" <eranian@...il.com>, "Gary.Mohr@...l.com" <Gary.Mohr@...l.com>, "arjan@...ux.intel.com" <arjan@...ux.intel.com>, "Zhang, Yanmin" <yanmin_zhang@...ux.intel.com>, Paul Mackerras <paulus@...ba.org>, "David S. Miller" <davem@...emloft.net>, Russell King <rmk+kernel@....linux.org.uk>, Arnaldo Carvalho de Melo <acme@...hat.com>, Will Deacon <will.deacon@....com>, Maynard Johnson <mpjohn@...ibm.com>, Carl Love <carll@...ibm.com>, Kay Sievers <kay.sievers@...y.org>, lkml <linux-kernel@...r.kernel.org>, Thomas Gleixner <tglx@...utronix.de>, Steven Rostedt <rostedt@...dmis.org> Subject: Re: [rfc] Describe events in a structured way via sysfs On Tue, 2010-06-29 at 16:55 +0800, Ingo Molnar wrote: > * Lin Ming <ming.m.lin@...el.com> wrote: > > > On Fri, 2010-06-25 at 01:33 +0800, Ingo Molnar wrote: > > > * Johannes Berg <johannes@...solutions.net> wrote: > > > > > > > On Thu, 2010-06-24 at 11:36 +0200, Ingo Molnar wrote: > > > > > > > > > That's probably best achieved via a TRACE_EVENT() variant, by passing in the > > > > > sysfs location. > > > > > > > > > > It might even make sense to make this a part of TRACE_EVENT() itself and make > > > > > 'NULL' the current default, non-sysfs-enumerated behavior. That way we can > > > > > gradually (and non-intrusively) find all the right sysfs places for events. > > > > > > > > No, this doesn't work. A lot of events are multi-instance. Say you have an > > > > event for each USB device. This event would have to show up in many places > > > > in sysfs, and each trace_foo() invocation needs to get the struct device > > > > pointer, not just the TRACE_EVENT() definition. Additionally, to > > > > create/destroy the sysfs pieces we need something like init_trace_foo(dev) > > > > and destroy_trace_foo(dev) be called when the sysfs points for the device > > > > should be created/destroyed. > > > > > > Yes - but even this could be expressed via TRACE_EVENT(): by giving it a > > > device-specific function pointer and then instantiating individual events from > > > a single, central place in sysfs. > > > > > > That is the place where we already know where it ends up in sysfs, and where > > > the event-specific function can match up whether that particular node belongs > > > to it and whether an additional event directory should be created for that > > > particular sysfs node. > > > > > > > The TRACE_EVENT() just defines the template, but such multi-instance events > > > > really should be standardised in terms of their struct device (or maybe > > > > kobject). > > > > > > > > I think that needs some TRACE_DEVICE_EVENT macro that creates the required > > > > inlines etc, and including the init/destroy that are called when the event > > > > should show up in sysfs. > > > > > > > > There's no way you can have the event show up in sysfs at the right spot > > > > with _just_ a TRACE_EVENT macro, since at define time in the header file you > > > > don't even have a valid struct device pointer. > > > > > > That would be another possible way to do it - to explicitly create the events > > > directory. It looks a bit simpler as we wouldnt have to touch TRACE_EVENT() > > > and because it directly expresses the 'this node has an events directory' > > > property at the place where we create the device node. > > > > Let me take i915 tracepoints as an example. > > Do you mean something like below? > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index 423dc90..9e7e4a0 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -28,6 +28,7 @@ > > */ > > > > #include <linux/device.h> > > +#include <linux/perf_event.h> > > #include "drmP.h" > > #include "drm.h" > > #include "i915_drm.h" > > @@ -413,7 +414,17 @@ int i965_reset(struct drm_device *dev, u8 flags) > > static int __devinit > > i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > { > > - return drm_get_dev(pdev, ent, &driver); > > + struct kobject *kobj; > > + int ret; > > + > > + ret = drm_get_dev(pdev, ent, &driver); > > + > > + if (!ret) { > > + kobj = &pdev->dev.kobj; > > + perf_sys_register_tp(kobj, "i915"); > > + } > > + > > + return ret; > > Yeah, something like that - assuming that this means that we'll add the events > directory to the device directory, to all the > /sys/bus/pci/drivers/i915/*/events/ driver directories right? (i havent > checked the DRM code) I haven't run the code, but I think yes. > > Small detail, it could be written a bit more compactly, like: Thanks for the tip. > > > + int ret; > > + > > + ret = drm_get_dev(pdev, ent, &driver); > > + if (!ret) > > + perf_sys_register_tp(&pdev->dev.kobj, "i915"); > > + > > + return ret; > > 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(). How does the driver know if an event is 'generic' if TRACE_EVENT is not touched? > > 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. Lin Ming > > Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists