Re: [PATCH] perf_events: fix and improve x86 event scheduling

[Stephane Eranian]

On Thu, Nov 10, 2011 at 3:37 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> Just throwing this out there (hasn't event been compiled etc..).
>
> The idea is to try the fixed counters first so that we don't
> 'accidentally' fill a GP counter with something that could have lived on
> the fixed purpose one and then end up under utilizing the PMU that way.
>
> It ought to solve the most common PMU programming fail on Intel
> thingies.
>
What are the configs for which you have failures on Intel?

I think I can improve my algorithm for fixed counters by treating
them separately and trying fixed counters first any supported
event.

> ---
> Index: linux-2.6/arch/x86/kernel/cpu/perf_event.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/cpu/perf_event.c
> +++ linux-2.6/arch/x86/kernel/cpu/perf_event.c
> @@ -558,14 +558,22 @@ int x86_schedule_events(struct cpu_hw_ev
> Â Â Â Â Â Â Â Â Â Â Â Âif (c->weight != w)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âcontinue;
>
> - Â Â Â Â Â Â Â Â Â Â Â for_each_set_bit(j, c->idxmsk, X86_PMC_IDX_MAX) {
> + Â Â Â Â Â Â Â Â Â Â Â if (x86_pmu.num_counters_fixed) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â j = X86_PMC_IDX_FIXED - 1;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â for_each_set_bit_cont(j, c->idxmsk, X86_PMC_IDX_MAX) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (!test_bit(k, used_mask))
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto assign;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â }
> + Â Â Â Â Â Â Â Â Â Â Â }
> +
> + Â Â Â Â Â Â Â Â Â Â Â for_each_set_bit(j, c->idxmsk, X86_PMC_IDX_FIXED) {
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âif (!test_bit(j, used_mask))
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto assign;
> Â Â Â Â Â Â Â Â Â Â Â Â}
>
> - Â Â Â Â Â Â Â Â Â Â Â if (j == X86_PMC_IDX_MAX)
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> + Â Â Â Â Â Â Â Â Â Â Â break;
>
> +assign:
> Â Â Â Â Â Â Â Â Â Â Â Â__set_bit(j, used_mask);
>
> Â Â Â Â Â Â Â Â Â Â Â Âif (assign)
> Index: linux-2.6/include/linux/bitops.h
> ===================================================================
> --- linux-2.6.orig/include/linux/bitops.h
> +++ linux-2.6/include/linux/bitops.h
> @@ -26,6 +26,12 @@ extern unsigned long __sw_hweight64(__u6
> Â Â Â Â Â Â (bit) < (size); \
> Â Â Â Â Â Â (bit) = find_next_bit((addr), (size), (bit) + 1))
>
> +#define for_each_set_bit_cont(bit, addr, size) \
> + Â Â Â for ((bit) = find_next_bit((addr), (size), (bit) + 1); \
> + Â Â Â Â Â Â(bit) < (size); \
> + Â Â Â Â Â Â(bit) = find_next_bit((addr), (size), (bit) + 1))
> +
> +
> Âstatic __inline__ int get_bitmask_order(unsigned int count)
> Â{
> Â Â Â Âint order;
>
>
N‹§²æìr¸›yúèšØb²X¬¶ÇvØ^–)Þ{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü}©ž²ÆzÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹®w¥¢¸?™¨è­Ú&¢)ßf”ù^jÇy§m…á@A«a¶Úÿ0¶ìh®å’i