Re: [PATCH] PERF(kernel): Cleanup power events V2

Ingo Molnar
2010-10-26T11:54:22Z

* Thomas Renninger <trenn@suse.de> wrote:

> On Tuesday 26 October 2010 13:21:29 Ingo Molnar wrote:
> >
> > * Jean Pihet <jean.pihet@newoldbits.com> wrote:
> ..
> > > >> +#ifndef _TRACE_POWER_ENUM_
> > > >> +#define _TRACE_POWER_ENUM_
> > > >> +enum {
> > > >> + POWER_NONE = 0,
> > > >> + POWER_CSTATE = 1,
> > > >> + POWER_PSTATE = 2,
> > > >> +};
> > > >> +#endif
> > > >
> > > > Since we are cleaning up all these events, those enum definitions dont really look
> > > > logical. For example, what is 'POWER_NONE'? Can a CPU have 'no power'?
> > >
> > > The enum belongs to the deprecated API so I would rather not touch it.
> > > Keeping the deprecated code isolated will make it easier to remove
> > > later.
> >
> > So what will replace it? We still have a state field.
>
> Nothing, this is part of the cleanup.

I mean, what will go into the state field of the power:cpu_idle event?

> As you state above: POWER_NONE does not make sense at all.
> The whole thing (type= attribute that vanishes now) is
> passed to userspace, but never gets used there because the
> same info is in the event name:
> cpu_frequency <-> frequency_switch <-> PSTATE
> cpu_idle <-> power_start/power_end <-> CSTATE

Ah, i see, so this 'state' enum went into the type field.

So my question is, and ignore this particular enum for now, what values go into the
state field, which field is still kept in the new events as well.

[ I'd like to avoid us having to define a third set of power events a few years down
the road ;-) ]

Thanks,

Ingo