Re: [RFC][PATCH v2 10/11] perf: core, lookup pmu via sysfs

[Greg KH <greg@kroah.com>]

On Wed, May 19, 2010 at 01:49:38AM +0000, Lin Ming wrote:
> Add another pmu lookup method via sysfs.
> Now we have 2 methods: type->pmu and sys_fd->pmu
> 
> Changes log,
> Lookup pmu via sysfs (Ingo Molnar)
> 
> Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> ---
>  kernel/perf_event.c |   33 ++++++++++++++++++++++++++++++++-
>  1 files changed, 32 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/perf_event.c b/kernel/perf_event.c
> index 8640610..0ab4a8b 100644
> --- a/kernel/perf_event.c
> +++ b/kernel/perf_event.c
> @@ -4680,7 +4680,7 @@ static struct pmu *sw_perf_event_init(struct perf_event *event)
>  	return pmu;
>  }
>  
> -static struct pmu *perf_event_lookup_pmu(struct perf_event *event)
> +static struct pmu *perf_event_lookup_pmu_by_type(struct perf_event *event)
>  {
>  	struct pmu *pmu = NULL;
>  	int pmu_id = PERF_EVENT_ATTR_TYPE(&event->attr);
> @@ -4708,6 +4708,37 @@ static struct pmu *perf_event_lookup_pmu(struct perf_event *event)
>  	return pmu;
>  }
>  
> +static struct pmu *perf_event_lookup_pmu_by_fd(struct perf_event *event)
> +{
> +	struct pmu *pmu = NULL;
> +	struct kobject *kobj;
> +	int sys_fd = PERF_EVENT_ATTR_FD(&event->attr);
> +	struct file *file;
> +	int fput_needed;
> +
> +	file = fget_light(sys_fd, &fput_needed);
> +	if (!file)
> +		return NULL;
> +
> +	kobj = sysfs_get_kobject(file);
> +	if (kobj)
> +		pmu = container_of(kobj, struct pmu, kobj);

Ick, no, I don't think this is a "valid" path to get a kobject.

How would the kernel internally have a file handle, and yet, not have
the kobject?  What is happening that it is so backwards?  Normally the
kobject will get called when its attributes are read or written to, then
you have the kobject handle.  You shouldn't have things the other way
around.

Or am I really missing something here?

> +
> +	fput_light(file, fput_needed);
> +
> +	return pmu;
> +}
> +
> +static struct pmu *perf_event_lookup_pmu(struct perf_event *event)
> +{
> +	u32 type = event->attr.type;
> +
> +	if (type & PERF_EVENT_FD)
> +		return perf_event_lookup_pmu_by_fd(event);

How are we getting here?

totally confused,

greg k-h