Re: [PATCH v8 1/4] perf,kvm/{x86,s390}: Remove dependency on uapi/kvm_perf.h

[Hemant Kumar]

On 09/28/2015 08:51 PM, David Ahern wrote:
> On 9/28/15 9:16 AM, Scott Wood wrote:
> > On Mon, 2015-09-28 at 08:31 -0600, David Ahern wrote:
> > > On 9/28/15 7:00 AM, Alexander Yarygin wrote:
> > > > > diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> > > > > index fc1cffb..ef25fcf 100644
> > > > > --- a/tools/perf/builtin-kvm.c
> > > > > +++ b/tools/perf/builtin-kvm.c
> > > > > @@ -31,20 +31,18 @@
> > > > >    #include <math.h> 

[SNIP]

> > > > > @@ -60,7 +58,7 @@ bool exit_event_begin(struct perf_evsel *evsel,
> > > > >
> > > > >    bool kvm_entry_event(struct perf_evsel *evsel)
> > > > >    {
> > > > > - return !strcmp(evsel->name, KVM_ENTRY_TRACE);
> > > > > + return !strncmp(evsel->name, kvm_events_tp[0], 
> > > > > strlen(evsel->name));
> > > > >    }
> > > > >
> > > > >    bool exit_event_end(struct perf_evsel *evsel,
> > >
> > > I agree; don't rely on kvm_events_tp. Define KVM_ENTRY_TRACE and
> > > KVM_EXIT_TRACE like x86.
> >
> > If you mean defining them in uapi, that doesn't work for arches that 
> > have
> > multiple subarches that may have different trace events.  This patchset
> > doesn't actually implement dynamic support for the subarches, but it 
> > avoids
> > adding constants to uapi headers that only apply to one of the 
> > subarches.
>
> I don't agree on relying on kvm_events_tp[0] and [1]. If you need that 
> to be a runtime definition then change KVM_ENTRY_TRACE to const char 
> *kvm_entry_trace and s390 and other arches can have code to set 
> kvm_{entry,exit}_trace at runtime.

Yeah, will change them to kvm_{entry,exit}_trace, instead.

--
Thanks,
Hemant Kumar

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