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] [day] [month] [year] [list]

Message-ID: <CAM9d7ciiVrHEKTMCUFLwDq7uvpLb4U2uvLn+uVoQbfqDeAuh9Q@mail.gmail.com>
Date:   Tue, 30 May 2023 23:07:09 -0700
From:   Namhyung Kim <namhyung@...nel.org>
To:     Jonathan Cameron <Jonathan.Cameron@...wei.com>
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

On Tue, May 30, 2023 at 6:50 AM Jonathan Cameron
<Jonathan.Cameron@...wei.com> wrote:
>
>
> 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.

I was thinking if we could avoid IPIs for read from other
CPUs.  It has PERF_EV_CAP_READ_ACTIVE_PKG for some uncore events
which can be shared among CPUs in the same package
to skip the IPI and to read it from the local CPU.

But I think this can be a separate step.

Thanks,
Namhyung