Re: [arc-linux-dev] Re: [PATCH v3 3/6] ARCv2: perf: Support sampling events using overflow interrupts

[Alexey Brodkin]

Hi Peter,

On Wed, 2015-08-26 at 16:35 +-0200, Peter Zijlstra wrote:
+AD4- On Wed, Aug 26, 2015 at 01:17:20PM +-0000, Alexey Brodkin wrote:
+AD4- +AD4- Hi Peter,
+AD4- +AD4- 
+AD4- +AD4- On Wed, 2015-08-26 at 15:07 +-0200, Peter Zijlstra wrote:
+AD4- +AD4- +AD4- On Mon, Aug 24, 2015 at 05:20:20PM +-0300, Alexey Brodkin wrote:
+AD4- +AD4- +AD4- +AD4- +AEAAQA- -139,9 +-141,11 +AEAAQA- static int arc+AF8-pmu+AF8-event+AF8-init(struct perf+AF8-event +ACo-event)
+AD4- +AD4- +AD4- +AD4-  	struct hw+AF8-perf+AF8-event +ACo-hwc +AD0- +ACY-event-+AD4-hw+ADs-
+AD4- +AD4- +AD4- +AD4-  	int ret+ADs-
+AD4- +AD4- +AD4- +AD4-  
+AD4- +AD4- +AD4- +AD4- -	hwc-+AD4-sample+AF8-period  +AD0- arc+AF8-pmu-+AD4-max+AF8-period+ADs-
+AD4- +AD4- +AD4- +AD4- -	hwc-+AD4-last+AF8-period +AD0- hwc-+AD4-sample+AF8-period+ADs-
+AD4- +AD4- +AD4- +AD4- -	local64+AF8-set(+ACY-hwc-+AD4-period+AF8-left, hwc-+AD4-sample+AF8-period)+ADs-
+AD4- +AD4- +AD4- +AD4- +-	if (+ACE-is+AF8-sampling+AF8-event(event)) +AHs-
+AD4- +AD4- +AD4- +AD4- +-		hwc-+AD4-sample+AF8-period  +AD0- arc+AF8-pmu-+AD4-max+AF8-period+ADs-
+AD4- +AD4- +AD4- +AD4- +-		hwc-+AD4-last+AF8-period +AD0- hwc-+AD4-sample+AF8-period+ADs-
+AD4- +AD4- +AD4- +AD4- +-		local64+AF8-set(+ACY-hwc-+AD4-period+AF8-left, hwc-+AD4-sample+AF8-period)+ADs-
+AD4- +AD4- +AD4- +AD4- +-	+AH0-
+AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- So here we set a max+AF8-period sample period for +ACE-sampling events such that
+AD4- +AD4- +AD4- we can properly deal with (short) counter overflow and accumulate into a
+AD4- +AD4- +AD4- 64bit value.
+AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- +AD4-  	switch (event-+AD4-attr.type) +AHs-
+AD4- +AD4- +AD4- +AD4-  	case PERF+AF8-TYPE+AF8-HARDWARE:
+AD4- +AD4- +AD4- +AD4- +AEAAQA- -243,6 +-247,11 +AEAAQA- static void arc+AF8-pmu+AF8-start(struct perf+AF8-event +ACo-event, int flags)
+AD4- +AD4- +AD4- +AD4-  
+AD4- +AD4- +AD4- +AD4-  	arc+AF8-pmu+AF8-event+AF8-set+AF8-period(event)+ADs-
+AD4- +AD4- +AD4- +AD4-  
+AD4- +AD4- +AD4- +AD4- +-	/+ACo- Enable interrupt for this counter +ACo-/
+AD4- +AD4- +AD4- +AD4- +-	if (is+AF8-sampling+AF8-event(event))
+AD4- +AD4- +AD4- +AD4- +-		write+AF8-aux+AF8-reg(ARC+AF8-REG+AF8-PCT+AF8-INT+AF8-CTRL,
+AD4- +AD4- +AD4- +AD4- +-			      read+AF8-aux+AF8-reg(ARC+AF8-REG+AF8-PCT+AF8-INT+AF8-CTRL) +AHw- (1 +ADwAPA- idx))+ADs-
+AD4- +AD4- +AD4- +AD4- +-
+AD4- +AD4- +AD4- 
+AD4- +AD4- +AD4- Yet here you fail to actually enable the interrupt for the non sampling
+AD4- +AD4- +AD4- events, which makes the above not work.
+AD4- +AD4- 
+AD4- +AD4- Indeed we intentionally leave interrupts disabled for non-sampling events.
+AD4- +AD4-  +AFs-1+AF0- We have quite large counters so we don't expect to overflow normally
+AD4- +AD4-  +AFs-2+AF0- We may re-use the same code for hardware that lacks support of IRQs in PCT.
+AD4- +AD4-      See we check if IRQs are available and if not set PERF+AF8-PMU+AF8-CAP+AF8-NO+AF8-INTERRUPT
+AD4- +AD4-      that will guarantee we won't get sampling event and for non-sampling events
+AD4- +AD4-      we won't use IRQs.
+AD4- 
+AD4- Tricky, I was seeing is+AF8-isa+AF8-arcv2() calls elsewhere, so I figured you'd
+AD4- make it conditional on that.
+AD4- 
+AD4- But sure, if you think you can live with 1 this'll work.

Well indeed there's a room for improvement always.
But from our current experience existing implementation works pretty fine.

Moreover having now PCT IRQs we mostly use sampling events that allow doing
real profiling.

-Alexey--
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/