Re: [PATCH v3 2/9] arm64: perf: Remove PMU locking

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

On Mon, Jul 08, 2019 at 03:32:50PM +0100, Julien Thierry wrote:
> Since the PMU driver uses direct registers for counter
> setup/manipulation, locking around these operations is no longer needed.
> 
> For operations that can be called with interrupts enabled, preemption
> still needs to be disabled to ensure the programming of the PMU is
> done on the expected CPU and not migrated mid-programming.

[...]

>  static void armv8pmu_start(struct arm_pmu *cpu_pmu)
>  {
> -	unsigned long flags;
> -	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> -
> -	raw_spin_lock_irqsave(&events->pmu_lock, flags);
> +	preempt_disable();
>  	/* Enable all counters */
>  	armv8pmu_pmcr_write(armv8pmu_pmcr_read() | ARMV8_PMU_PMCR_E);
> -	raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> +	preempt_enable();
>  }
> 
>  static void armv8pmu_stop(struct arm_pmu *cpu_pmu)
>  {
> -	unsigned long flags;
> -	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> -
> -	raw_spin_lock_irqsave(&events->pmu_lock, flags);
> +	preempt_disable();
>  	/* Disable all counters */
>  	armv8pmu_pmcr_write(armv8pmu_pmcr_read() & ~ARMV8_PMU_PMCR_E);
> -	raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> +	preempt_enable();
>  }

I think that we'd have bigger problems if these could be called in
preemptible context, since we couldn't guarantee which HW PMU instance
they'd operate on.

I also thought that the interrupt disable/enable was a hold-over from
the old days of perf core, and these days all of the synchronous
operations are held with the pmu ctx lock held (and interrupts
disabled).

Do you have an example of when these are called with interrupts enabled?
Or in a preemptible context?

Thanks,
Mark.

> 
>  static irqreturn_t armv8pmu_handle_irq(struct arm_pmu *cpu_pmu)
> --
> 1.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel