Re: [PATCH] perf/Power: PERF_EVENT_IOC_ENABLE does not reenable event

[Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>]

Paul Mackerras [paulus@samba.org] wrote:
| > +	/*
| > +	 * If this event was disabled in record_and_restart() because we
| > +	 * exceeded the ->event_limit, this is probably a good time to
| > +	 * re-enable the event ? If we don't reenable the event, we will
| > +	 * never notify the user again about this event.
| > +	 */
| 
| The comment seems a bit tentative. :)  If the PERF_EF_START bit is set
| then we are being told to restart the event.
| 
| >  	if (!(ef_flags & PERF_EF_START))
| >  		event->hw.state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
| > +	else
| > +		event->hw.state &= ~PERF_HES_STOPPED;
| 
| This looks fine, though I think you could equally well just set
| event->hw.state to 0 in the else clause.  That would clear the
| UPTODATE flag too, which is appropriate since we are about to put the
| event on a hardware counter.

Agree. I submitted a new patch with better comments and patch description
and cleared the state to 0.

Thanks for the review.

Sukadev