linux-kernel - Re: [PATCH v7 4/5] perf: CXL Performance Monitoring Unit driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 30 May 2023 14:50:01 +0100
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     Namhyung Kim <namhyung@...nel.org>
CC:     Liang Kan <kan.liang@...ux.intel.com>, <linux-cxl@...r.kernel.org>,
        <peterz@...radead.org>, <mark.rutland@....com>, <will@...nel.org>,
        <dan.j.williams@...el.com>, <mingo@...hat.com>, <acme@...nel.org>,
        <linuxarm@...wei.com>, <linux-perf-users@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        Davidlohr Bueso <dave@...olabs.net>,
        "Dave Jiang" <dave.jiang@...el.com>,
        Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH v7 4/5] perf: CXL Performance Monitoring Unit driver


Hi,

Tidied up the typos. Thanks,

> > +static int cxl_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> > +{
> > +       struct cxl_pmu_info *info = hlist_entry_safe(node, struct cxl_pmu_info, node);
> > +       unsigned int target;
> > +
> > +       if (info->on_cpu != cpu)
> > +               return 0;
> > +
> > +       info->on_cpu = -1;
> > +       target = cpumask_any_but(cpu_online_mask, cpu);
> > +       if (target >= nr_cpu_ids) {
> > +               dev_err(info->pmu.dev, "Unable to find a suitable CPU\n");
> > +               return 0;
> > +       }
> > +
> > +       perf_pmu_migrate_context(&info->pmu, cpu, target);
> > +       info->on_cpu = target;
> > +       /*
> > +        * CPU HP lock is held so we should be guaranteed that this CPU hasn't yet
> > +        * gone away.
> > +        */
> > +       WARN_ON(irq_set_affinity(info->irq, cpumask_of(target)));
> > +
> > +       return 0;
> > +}  
> 
> IIUC a CXL PMU hardware (say cxl_pmu_mem0.0) is shared across
> all CPUs and it would return the same value when read from any CPU,
> right?

Correct, it will return the same value when used from any CPU.
I'm not sure what issue you are indicating.

My understanding is that, even for such cases, perf uses percpu
variables that mean we still have to ensure that the interrupt
handling occurs on the CPU we have migrated the context to.

There are a lot of similar driver in perf already from a quick
git grep cpumask_any_but\(cpu_online_mask, 

It might be nice to enable perf to operate for these devices without
the percpu context though. I haven't looked into whether that
is worth doing.

Jonathan




> 
> Thanks,
> Namhyung
> 
> 
> > +
> > +static __init int cxl_pmu_init(void)
> > +{
> > +       int rc;
> > +
> > +       rc = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
> > +                                    "AP_PERF_CXL_PMU_ONLINE",
> > +                                    cxl_pmu_online_cpu, cxl_pmu_offline_cpu);
> > +       if (rc < 0)
> > +               return rc;
> > +       cxl_pmu_cpuhp_state_num = rc;
> > +
> > +       rc = cxl_driver_register(&cxl_pmu_driver);
> > +       if (rc)
> > +               cpuhp_remove_multi_state(cxl_pmu_cpuhp_state_num);
> > +
> > +       return rc;
> > +}
> > +
> > +static __exit void cxl_pmu_exit(void)
> > +{
> > +       cxl_driver_unregister(&cxl_pmu_driver);
> > +       cpuhp_remove_multi_state(cxl_pmu_cpuhp_state_num);
> > +}
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_IMPORT_NS(CXL);
> > +module_init(cxl_pmu_init);
> > +module_exit(cxl_pmu_exit);
> > +MODULE_ALIAS_CXL(CXL_DEVICE_PMU);
> > --
> > 2.39.2
> >  
>