Re: [PATCH 5/6] hw-breakpoints: Arbitrate access to pmu following registers constraints

[Paul Mackerras <paulus@samba.org>]

Frederic Weisbecker writes:

> Allow or refuse to build a counter using the breakpoints pmu following
> given constraints.

As far as I can see, you assume each CPU has HBP_NUM breakpoint
registers which are all interchangeable and can all be used either for
data breakpoints or instruction breakpoints.  Is that accurate?

If so, we'll need to extend it a bit for Power since we have some CPUs
that have one data breakpoint register and one instruction breakpoint
register.  In general on powerpc the instruction and data breakpoint
facilities are separate, i.e. we have no registers that can be used
for either.

> +static void toggle_bp_slot(struct perf_event *bp, bool enable)
> +{
> +	int cpu = bp->cpu;
> +	unsigned int *nr;
> +	struct task_struct *tsk = bp->ctx->task;
> +
> +	/* Flexible */
> +	if (!bp->attr.pinned) {
> +		if (cpu >= 0) {
> +			nr = &per_cpu(nr_bp_flexible, cpu);
> +			goto toggle;
> +		}
> +
> +		for_each_online_cpu(cpu) {
> +			nr = &per_cpu(nr_bp_flexible, cpu);
> +			goto toggle;

...

> +toggle:
> +	*nr = enable ? *nr + 1 : *nr - 1;
> +}

This won't do what I think you want.  In the case where
!bp->attr.pinned and cpu == -1, it will only update the count for the
first online cpu, not all of them.

Paul.