Re: [PATCH 3/9] perf/x86: Add is_hardware_event

[Mark Rutland <mark.rutland@arm.com>]

On Fri, Jul 17, 2015 at 04:47:26PM +0100, Mark Rutland wrote:
> On Fri, Jul 17, 2015 at 04:03:36PM +0100, Liang, Kan wrote:
> > > 
> > > On Thu, Jul 16, 2015 at 09:33:45PM +0100, kan.liang@intel.com wrote:
> > > > From: Kan Liang <kan.liang@intel.com>
> > > >
> > > > Using is_hardware_event to replace !is_software_event to indicate a
> > > > hardware event.
> > > 
> > > Why...?
> > 
> > First, the comments of is_software_event is not correct. 
> > 0 or !is_software_event is not for a hardware event.
> > is_hardware_event is for a hardware event.
> 
> Circular logic is fantastic.

Sorry for the snark here. I completely misread this.

I agree that the comment is wrong. However, changing !is_software_event
to is_hardware_event is not always correct.

For example, perf_group_attach tests for the addition of a non-software
event to a software group, so we can mark the group as not consisting
solely of software events. For that to be done correctly, we need to
check !is_software_event.

I was wrong about the throttling, having confused active_oncpu and
nr_active. Sorry for the noise on that. However, as you mention that
does prevent the use of exclusive events for uncore PMUs, and I don't
see why that should change.

Thanks,
Mark.