From: Greg KH on
On Wed, May 19, 2010 at 01:46:42AM +0000, Lin Ming wrote:
> +struct kobj_type event_ktype = {
> + .sysfs_ops = &event_sysfs_ops,
> +};

No release function? Hm, are you setting yourself up to be publically
made fun of? You have read the kobject documentation about this, right?
Have you looked at the kernel logs for when you try to free this object?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on
On Wed, May 19, 2010 at 01:46:42AM +0000, Lin Ming wrote:
> Now only exports cpu hardware events.
>
> For each PMU, there are 2 sysfs dirs: event_source and events.
>
> For example,
> /sys/devices/system/cpu/event_source/
> /sys/devices/system/cpu/events/
>
> $ tree /sys/devices/system/cpu/event_source/
> /sys/devices/system/cpu/event_source/
> `-- id
>
> $ tree /sys/devices/system/cpu/events/
> /sys/devices/system/cpu/events/
> |-- L1-dcache-load-misses
> | |-- event_source -> ../../event_source

What is this symlink for? Is it needed as they all seem to point to the
same thing.

Also, as you are trying to add new sysfs interfaces, please document
them in Documentation/ABI.

And, do you have to use "raw" kobjects here? Any chance you can use a
struct device instead?

I'm still not really understanding what exactly you are trying to show
in the sysfs tree here anyway, perhaps the documentation update will
clear that up for me.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Lin Ming on
On Wed, 2010-05-19 at 04:07 +0800, Greg KH wrote:
> On Wed, May 19, 2010 at 01:46:42AM +0000, Lin Ming wrote:
> > +struct kobj_type event_ktype = {
> > + .sysfs_ops = &event_sysfs_ops,
> > +};
>
> No release function? Hm, are you setting yourself up to be publically
> made fun of? You have read the kobject documentation about this, right?
> Have you looked at the kernel logs for when you try to free this object?

Ah, will fix this "fun".

>
> thanks,
>
> greg k-h

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on
On Wed, May 19, 2010 at 10:34:55AM +0800, Lin Ming wrote:
> On Wed, 2010-05-19 at 04:05 +0800, Greg KH wrote:
> > On Wed, May 19, 2010 at 01:46:42AM +0000, Lin Ming wrote:
> > > Now only exports cpu hardware events.
> > >
> > > For each PMU, there are 2 sysfs dirs: event_source and events.
> > >
> > > For example,
> > > /sys/devices/system/cpu/event_source/
> > > /sys/devices/system/cpu/events/
> > >
> > > $ tree /sys/devices/system/cpu/event_source/
> > > /sys/devices/system/cpu/event_source/
> > > `-- id
> > >
> > > $ tree /sys/devices/system/cpu/events/
> > > /sys/devices/system/cpu/events/
> > > |-- L1-dcache-load-misses
> > > | |-- event_source -> ../../event_source
> >
> > What is this symlink for? Is it needed as they all seem to point to the
> > same thing.
>
> The symlink is used to find the "event_source" of the "event".
>
> /sys/devices/system/cpu/event_source/
>
> /sys/devices/system/cpu/events/
> |-- L1-dcache-load-misses
> | |-- event_source -> ../../event_source
>
> For above example, "event_source" is the cpu pmu and the "event" is
> L1-dcache-load-misses.
>
> Yes, they point to the same thing, because all the events
> under /sys/devices/system/cpu/events/* is monitored by the same event
> source, ie, the cpu pmu(/sys/devices/system/cpu/event_source/).

Then, why have a symlink if they are all going to be the same?

> > I'm still not really understanding what exactly you are trying to show
> > in the sysfs tree here anyway, perhaps the documentation update will
> > clear that up for me.
>
> To support multiple pmus

What is a "pmu"?

> (or call them event source),

What is an "event source"?

> I want to show all
> the pmus and events in the sysfs tree.

Why do these things need to be in sysfs? What benifit is it going to
have?

> For cpu pmu,
> /sys/devices/system/cpu/event_source/*
> /sys/devices/system/cpu/events/*

What is a "cpu pmu"?

> For node pmu,
> /sys/devices/system/node/event_source/*
> /sys/devices/system/node/events/*

What is a "node pmu"?

> For mce,
> /sys/kernel/events/mce/*

What? No, don't create /sys/kernel/events/. Is that in this patchset?
Ick, that's such a "general" name that it is going to get confusing very
quickly. Heck, I still don't know what this "event" thing is yet :)

> and so on...the exact placement is not yet final.

Again, why do you need/want anything in sysfs in the first place?
What problem is it going to solve? Who is going to benifit? Why do
they care? What is this whole thing about?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Lin Ming on
On Wed, 2010-05-19 at 10:48 +0800, Greg KH wrote:
> On Wed, May 19, 2010 at 10:34:55AM +0800, Lin Ming wrote:
> > On Wed, 2010-05-19 at 04:05 +0800, Greg KH wrote:
> > > On Wed, May 19, 2010 at 01:46:42AM +0000, Lin Ming wrote:
> > > > Now only exports cpu hardware events.
> > > >
> > > > For each PMU, there are 2 sysfs dirs: event_source and events.
> > > >
> > > > For example,
> > > > /sys/devices/system/cpu/event_source/
> > > > /sys/devices/system/cpu/events/
> > > >
> > > > $ tree /sys/devices/system/cpu/event_source/
> > > > /sys/devices/system/cpu/event_source/
> > > > `-- id
> > > >
> > > > $ tree /sys/devices/system/cpu/events/
> > > > /sys/devices/system/cpu/events/
> > > > |-- L1-dcache-load-misses
> > > > | |-- event_source -> ../../event_source
> > >
> > > What is this symlink for? Is it needed as they all seem to point to the
> > > same thing.
> >
> > The symlink is used to find the "event_source" of the "event".
> >
> > /sys/devices/system/cpu/event_source/
> >
> > /sys/devices/system/cpu/events/
> > |-- L1-dcache-load-misses
> > | |-- event_source -> ../../event_source
> >
> > For above example, "event_source" is the cpu pmu and the "event" is
> > L1-dcache-load-misses.
> >
> > Yes, they point to the same thing, because all the events
> > under /sys/devices/system/cpu/events/* is monitored by the same event
> > source, ie, the cpu pmu(/sys/devices/system/cpu/event_source/).
>
> Then, why have a symlink if they are all going to be the same?

So how about move the symlink to parent directory, like

/sys/devices/system/cpu/event_source/

/sys/devices/system/cpu/events/
|-- event_source -> ../event_source

>
> > > I'm still not really understanding what exactly you are trying to show
> > > in the sysfs tree here anyway, perhaps the documentation update will
> > > clear that up for me.
> >
> > To support multiple pmus
>
> What is a "pmu"?

Performance Monitoring Unit

>
> > (or call them event source),
>
> What is an "event source"?

The source who generates the event.
The event includes hardware events, cache-misses, bus-cycles,
interrupts, error conditions, hotplug......
and software events, context-switches, page-faults, sched events......

>
> > I want to show all
> > the pmus and events in the sysfs tree.
>
> Why do these things need to be in sysfs? What benifit is it going to
> have?

Kernel to provide the information of the event source and
hardware/software events via sysfs and user-space can get
this info from sysfs.

>
> > For cpu pmu,
> > /sys/devices/system/cpu/event_source/*
> > /sys/devices/system/cpu/events/*
>
> What is a "cpu pmu"?

The performance monitoring unit in cpu.

>
> > For node pmu,
> > /sys/devices/system/node/event_source/*
> > /sys/devices/system/node/events/*
>
> What is a "node pmu"?

The pmu shared by logical cpus in a package, for example, Nehalem uncore
pmu, PowerPc nest units.

>
> > For mce,
> > /sys/kernel/events/mce/*
>
> What? No, don't create /sys/kernel/events/. Is that in this patchset?

No, not in this patchset.

> Ick, that's such a "general" name that it is going to get confusing very
> quickly. Heck, I still don't know what this "event" thing is yet :)
>
> > and so on...the exact placement is not yet final.
>
> Again, why do you need/want anything in sysfs in the first place?
> What problem is it going to solve? Who is going to benifit? Why do
> they care? What is this whole thing about?
>
> thanks,
>
> greg k-h

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/