lkml.org 
[lkml]   [2019]   [Apr]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 2/2] perf/arm-ccn: Clean up CPU hotplug handling
From
Date
On 04/16/2019 04:24 PM, Robin Murphy wrote:
> Like arm-cci, arm-ccn has the same issue of disabling preemption around
> operations which can take mutexes. Again, remove the definite bug by
> simply not trying to fight the theoretical races. And since we are
> touching the hotplug handling code, take the opportunity to streamline
> it, as there's really no need to store a full-sized cpumask to keep
> track of a single CPU ID.
>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
> drivers/perf/arm-ccn.c | 25 +++++++++++++------------
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/perf/arm-ccn.c b/drivers/perf/arm-ccn.c
> index 2ae76026e947..0bb52d9bdcf7 100644
> --- a/drivers/perf/arm-ccn.c
> +++ b/drivers/perf/arm-ccn.c
> @@ -167,7 +167,7 @@ struct arm_ccn_dt {
>
> struct hrtimer hrtimer;
>
> - cpumask_t cpu;
> + unsigned int cpu;
> struct hlist_node node;
>
> struct pmu pmu;
> @@ -559,7 +559,7 @@ static ssize_t arm_ccn_pmu_cpumask_show(struct device *dev,
> {
> struct arm_ccn *ccn = pmu_to_arm_ccn(dev_get_drvdata(dev));
>
> - return cpumap_print_to_pagebuf(true, buf, &ccn->dt.cpu);
> + return cpumap_print_to_pagebuf(true, buf, cpumask_of(ccn->dt.cpu));
> }
>
> static struct device_attribute arm_ccn_pmu_cpumask_attr =
> @@ -759,7 +759,7 @@ static int arm_ccn_pmu_event_init(struct perf_event *event)
> * mitigate this, we enforce CPU assignment to one, selected
> * processor (the one described in the "cpumask" attribute).
> */
> - event->cpu = cpumask_first(&ccn->dt.cpu);
> + event->cpu = ccn->dt.cpu;
>
> node_xp = CCN_CONFIG_NODE(event->attr.config);
> type = CCN_CONFIG_TYPE(event->attr.config);
> @@ -1215,15 +1215,15 @@ static int arm_ccn_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> struct arm_ccn *ccn = container_of(dt, struct arm_ccn, dt);
> unsigned int target;
>
> - if (!cpumask_test_and_clear_cpu(cpu, &dt->cpu))
> + if (cpu != dt->cpu)
> return 0;
> target = cpumask_any_but(cpu_online_mask, cpu);
> if (target >= nr_cpu_ids)
> return 0;
> perf_pmu_migrate_context(&dt->pmu, cpu, target);
> - cpumask_set_cpu(target, &dt->cpu);
> + dt->cpu = target;
> if (ccn->irq)
> - WARN_ON(irq_set_affinity_hint(ccn->irq, &dt->cpu) != 0);
> + WARN_ON(irq_set_affinity_hint(ccn->irq, cpumask_of(dt->cpu)));
> return 0;
> }
>
> @@ -1299,29 +1299,30 @@ static int arm_ccn_pmu_init(struct arm_ccn *ccn)
> }
>
> /* Pick one CPU which we will use to collect data from CCN... */
> - cpumask_set_cpu(get_cpu(), &ccn->dt.cpu);
> + ccn->dt.cpu = raw_smp_processor_id();
>
> /* Also make sure that the overflow interrupt is handled by this CPU */
> if (ccn->irq) {
> - err = irq_set_affinity_hint(ccn->irq, &ccn->dt.cpu);
> + err = irq_set_affinity_hint(ccn->irq, cpumask_of(ccn->dt.cpu));
> if (err) {
> dev_err(ccn->dev, "Failed to set interrupt affinity!\n");
> goto error_set_affinity;
> }
> }
>
> + cpuhp_state_add_instance_nocalls(CPUHP_AP_PERF_ARM_CCN_ONLINE,
> + &ccn->dt.node);
> +
> err = perf_pmu_register(&ccn->dt.pmu, name, -1);
> if (err)
> goto error_pmu_register;
>
> - cpuhp_state_add_instance_nocalls(CPUHP_AP_PERF_ARM_CCN_ONLINE,
> - &ccn->dt.node);
> - put_cpu();
> return 0;
>
> error_pmu_register:
> + cpuhp_state_remove_instance_nocalls(CPUHP_AP_PERF_ARM_CCN_ONLINE,
> + &ccn->dt.node);
> error_set_affinity:
> - put_cpu();

Super minor nit: We don't need the error_set_affinity label anymore, as
we don't do anything here. Otherwise:

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

\
 
 \ /
  Last update: 2019-04-16 18:27    [W:0.083 / U:0.388 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site