Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230526095824.16336-1-Jonathan.Cameron@huawei.com>
 <20230526095824.16336-5-Jonathan.Cameron@huawei.com> <CAM9d7cgYZs4DqLmjPZCYDVrp-KVYoZYDyJHLwB1fOM7ZdzM2Pg@mail.gmail.com>
 <20230530145001.00002560@Huawei.com>
In-Reply-To: <20230530145001.00002560@Huawei.com>
From:   Namhyung Kim <namhyung@kernel.org>
Date:   Tue, 30 May 2023 23:07:09 -0700
Message-ID: <CAM9d7ciiVrHEKTMCUFLwDq7uvpLb4U2uvLn+uVoQbfqDeAuh9Q@mail.gmail.com>
Subject: Re: [PATCH v7 4/5] perf: CXL Performance Monitoring Unit driver
To:     Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc:     Liang Kan <kan.liang@linux.intel.com>, linux-cxl@vger.kernel.org,
        peterz@infradead.org, mark.rutland@arm.com, will@kernel.org,
        dan.j.williams@intel.com, mingo@redhat.com, acme@kernel.org,
        linuxarm@huawei.com, linux-perf-users@vger.kernel.org,
        linux-kernel@vger.kernel.org, Davidlohr Bueso <dave@stgolabs.net>,
        Dave Jiang <dave.jiang@intel.com>,
        Stephane Eranian <eranian@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

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