Message ID | 20230324171313.18448-3-Jonathan.Cameron@huawei.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp792075vqo; Fri, 24 Mar 2023 10:25:04 -0700 (PDT) X-Google-Smtp-Source: AKy350ZzmcX4WJDVGVhDGGk4vD/Bj38zhqmysw8UxhvEwvCMwKesqaronJZ3LU0iDAeySLvCdbMX X-Received: by 2002:a17:90b:38cd:b0:234:67ef:304b with SMTP id nn13-20020a17090b38cd00b0023467ef304bmr3829416pjb.37.1679678704632; Fri, 24 Mar 2023 10:25:04 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1679678704; cv=none; d=google.com; s=arc-20160816; b=Hnb8q4arr0xI7+slmA83i8cmCRTA4rtxPOnIDjk/i+gl713d0DDBvnLsDHh8UHX7M+ FxQEc7lwIKGcpppkFKGUILv1y4//TGh+pOZoX+3uWiEIxosJMIC3QNCVx/Lj5f9tZdYA r2J1hJvFLaqACokucig43r6vFbGXSKZykPEbHORZ6GjBuJJ+Vwd3bzzxqgYF3f+m9Hqb vuoosxoCYfDqzEOrglwOsA4NtS38jx7/114sE7Mefa7tLsqtAy6e3FX4AgJEY4k/TjRt tB39fe9047Sl2oyWY5diwWhZOUb/SucV9bTZTOXNBHyTgAZ/SkxWSxYLwWdgCp4gDYiY NRXA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=Iy/RfxRzLkQ9aq41pZXqc351UxaPH+Nakq5sQQC+L4M=; b=yaQIV2wNTp4xUZZK7hKGMUi99jlwT9ckS8GiTVnf/IdG//VJ7haiP1o6xjQ9Cl9R5m 3lVHaFvr07S8GEUNiMeKvuHRrdGsdOxDvnw+XeMqhYWuApw/Hfgc/Ol/fTNpvF0io2LN JZehQvU3CX6m84KOjKBBfNs9zfoJdJSNGiNjKxjTEZ/5t3D5/5SG+tnjzv4YdPd9lQ9c 7INNU1F/hq0sB4T2q5WrRNVrX1wOMm2weUxvf2MK8XKlaYqYx5Pk1cmV5YnU3+jwR3dv dxsnkGox2L0In8KfwfUjGJFZSpVRVNZf8CmjO0PpmyclgbyaBAADpFPgQR+uuxIjrm5y PD0A== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id 101-20020a17090a0fee00b0023676bd55d1si238917pjz.94.2023.03.24.10.24.52; Fri, 24 Mar 2023 10:25:04 -0700 (PDT) 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; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232149AbjCXROj (ORCPT <rfc822;makky5685@gmail.com> + 99 others); Fri, 24 Mar 2023 13:14:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47350 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232102AbjCXROb (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 24 Mar 2023 13:14:31 -0400 Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A573830CA; Fri, 24 Mar 2023 10:14:10 -0700 (PDT) Received: from lhrpeml500005.china.huawei.com (unknown [172.18.147.226]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4Pjpcv6hgjz67PMg; Sat, 25 Mar 2023 01:10:39 +0800 (CST) Received: from SecurePC-101-06.china.huawei.com (10.122.247.231) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.21; Fri, 24 Mar 2023 17:14:08 +0000 From: Jonathan Cameron <Jonathan.Cameron@huawei.com> To: Liang Kan <kan.liang@linux.intel.com>, <linux-cxl@vger.kernel.org>, <peterz@infradead.org> CC: <mingo@redhat.com>, <acme@kernel.org>, <mark.rutland@arm.com>, <will@kernel.org>, <dan.j.williams@intel.com>, <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> Subject: [PATCH v2 2/5] perf: Allow a PMU to have a parent. Date: Fri, 24 Mar 2023 17:13:10 +0000 Message-ID: <20230324171313.18448-3-Jonathan.Cameron@huawei.com> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20230324171313.18448-1-Jonathan.Cameron@huawei.com> References: <20230324171313.18448-1-Jonathan.Cameron@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7BIT Content-Type: text/plain; charset=US-ASCII X-Originating-IP: [10.122.247.231] X-ClientProxiedBy: lhrpeml500002.china.huawei.com (7.191.160.78) To lhrpeml500005.china.huawei.com (7.191.163.240) X-CFilter-Loop: Reflected X-Spam-Status: No, score=-2.3 required=5.0 tests=RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1761270777287204324?= X-GMAIL-MSGID: =?utf-8?q?1761270777287204324?= |
Series |
CXL 3.0 Performance Monitoring Unit support
|
|
Commit Message
Jonathan Cameron
March 24, 2023, 5:13 p.m. UTC
Some PMUs have well defined parents such as PCI devices.
As the device_initialize() and device_add() are all within
pmu_dev_alloc() which is called from perf_pmu_register()
there is no opportunity to set the parent from within a driver.
Add a struct device *parent field to struct pmu and use that
to set the parent.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
include/linux/perf_event.h | 1 +
kernel/events/core.c | 1 +
2 files changed, 2 insertions(+)
Comments
On 2023-03-24 1:13 p.m., Jonathan Cameron wrote: > Some PMUs have well defined parents such as PCI devices. > As the device_initialize() and device_add() are all within > pmu_dev_alloc() which is called from perf_pmu_register() > there is no opportunity to set the parent from within a driver. > > Add a struct device *parent field to struct pmu and use that > to set the parent. Why we want a PMU parent? Maybe I missed it. I don't see that the parent is used anywhere. Thanks, Kan > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > include/linux/perf_event.h | 1 + > kernel/events/core.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index d5628a7b5eaa..b99db1eda72c 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -303,6 +303,7 @@ struct pmu { > > struct module *module; > struct device *dev; > + struct device *parent; > const struct attribute_group **attr_groups; > const struct attribute_group **attr_update; > const char *name; > diff --git a/kernel/events/core.c b/kernel/events/core.c > index fb3e436bcd4a..a84c282221f2 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -11367,6 +11367,7 @@ static int pmu_dev_alloc(struct pmu *pmu) > > dev_set_drvdata(pmu->dev, pmu); > pmu->dev->bus = &pmu_bus; > + pmu->dev->parent = pmu->parent; > pmu->dev->release = pmu_dev_release; > > ret = dev_set_name(pmu->dev, "%s", pmu->name);
On Mon, 27 Mar 2023 13:04:08 -0400 "Liang, Kan" <kan.liang@linux.intel.com> wrote: > On 2023-03-24 1:13 p.m., Jonathan Cameron wrote: > > Some PMUs have well defined parents such as PCI devices. > > As the device_initialize() and device_add() are all within > > pmu_dev_alloc() which is called from perf_pmu_register() > > there is no opportunity to set the parent from within a driver. > > > > Add a struct device *parent field to struct pmu and use that > > to set the parent. > > Why we want a PMU parent? Maybe I missed it. I don't see that the parent > is used anywhere. This allows you to identify the association between PMU and the hardware related device that is providing it by looking at the directory structure in sysfs rather than putting them directly under /sys/devices. ls -l /sys/bus/event_sources/devices/ ... armv8_pmuv3_0 -> ../../../devices/arm8_pmuv3_0 ... breakpoint -> ../../../devices/breakpoint ... cpmu0 -> ../../../devices/pci0000:0c/0000:0c:00.0/0000:0d:00.0/cpmu0/cpmu0 etc (the first cpmu0 is the parent registered as a child of the PCI EP and used for driver binding). So it's of use to userspace rather than in the kernel driver itself. Note that almost nothing is normally in the top level /sys/devices other than event_sources - because nearly all other struct device instances created by subsystems have parents assigned. On my system ls /sys/devices armv8_pmuv3_0 LNXSYSTEM:00 pci0000:0c pnp0 system uprobe breakpoint pci0000:00 platform software tracepoint virtual +CC Greg KH for input on whether / why this make sense. Thanks, Jonathan > > Thanks, > Kan > > > > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > --- > > include/linux/perf_event.h | 1 + > > kernel/events/core.c | 1 + > > 2 files changed, 2 insertions(+) > > > > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > > index d5628a7b5eaa..b99db1eda72c 100644 > > --- a/include/linux/perf_event.h > > +++ b/include/linux/perf_event.h > > @@ -303,6 +303,7 @@ struct pmu { > > > > struct module *module; > > struct device *dev; > > + struct device *parent; > > const struct attribute_group **attr_groups; > > const struct attribute_group **attr_update; > > const char *name; > > diff --git a/kernel/events/core.c b/kernel/events/core.c > > index fb3e436bcd4a..a84c282221f2 100644 > > --- a/kernel/events/core.c > > +++ b/kernel/events/core.c > > @@ -11367,6 +11367,7 @@ static int pmu_dev_alloc(struct pmu *pmu) > > > > dev_set_drvdata(pmu->dev, pmu); > > pmu->dev->bus = &pmu_bus; > > + pmu->dev->parent = pmu->parent; > > pmu->dev->release = pmu_dev_release; > > > > ret = dev_set_name(pmu->dev, "%s", pmu->name); >
On Tue, Mar 28, 2023 at 11:54:44AM +0100, Jonathan Cameron wrote: > On Mon, 27 Mar 2023 13:04:08 -0400 > "Liang, Kan" <kan.liang@linux.intel.com> wrote: > > > On 2023-03-24 1:13 p.m., Jonathan Cameron wrote: > > > Some PMUs have well defined parents such as PCI devices. > > > As the device_initialize() and device_add() are all within > > > pmu_dev_alloc() which is called from perf_pmu_register() > > > there is no opportunity to set the parent from within a driver. > > > > > > Add a struct device *parent field to struct pmu and use that > > > to set the parent. > > > > Why we want a PMU parent? Maybe I missed it. I don't see that the parent > > is used anywhere. > > This allows you to identify the association between PMU and the hardware related > device that is providing it by looking at the directory structure in sysfs rather > than putting them directly under /sys/devices. > > ls -l /sys/bus/event_sources/devices/ > > ... armv8_pmuv3_0 -> ../../../devices/arm8_pmuv3_0 > ... breakpoint -> ../../../devices/breakpoint > ... cpmu0 -> ../../../devices/pci0000:0c/0000:0c:00.0/0000:0d:00.0/cpmu0/cpmu0 > etc > > (the first cpmu0 is the parent registered as a child of the PCI EP and used for > driver binding). So it's of use to userspace rather than in the kernel driver > itself. > > Note that almost nothing is normally in the top level /sys/devices other than > event_sources - because nearly all other struct device instances created by > subsystems have parents assigned. > > On my system > > ls /sys/devices > > armv8_pmuv3_0 LNXSYSTEM:00 pci0000:0c pnp0 system uprobe > breakpoint pci0000:00 platform software tracepoint virtual > > +CC Greg KH for input on whether / why this make sense. That doesn't make sense, nothing should be in /sys/devices/ EXCEPT the root device of busses. Everything else is wrong and should have their code fixed up (i.e. "breakpoint", "software", etc.) thanks, greg k-h
On Tue, 28 Mar 2023 13:01:08 +0200 Greg KH <gregkh@linuxfoundation.org> wrote: > On Tue, Mar 28, 2023 at 11:54:44AM +0100, Jonathan Cameron wrote: > > On Mon, 27 Mar 2023 13:04:08 -0400 > > "Liang, Kan" <kan.liang@linux.intel.com> wrote: > > > > > On 2023-03-24 1:13 p.m., Jonathan Cameron wrote: > > > > Some PMUs have well defined parents such as PCI devices. > > > > As the device_initialize() and device_add() are all within > > > > pmu_dev_alloc() which is called from perf_pmu_register() > > > > there is no opportunity to set the parent from within a driver. > > > > > > > > Add a struct device *parent field to struct pmu and use that > > > > to set the parent. > > > > > > Why we want a PMU parent? Maybe I missed it. I don't see that the parent > > > is used anywhere. > > > > This allows you to identify the association between PMU and the hardware related > > device that is providing it by looking at the directory structure in sysfs rather > > than putting them directly under /sys/devices. > > > > ls -l /sys/bus/event_sources/devices/ > > > > ... armv8_pmuv3_0 -> ../../../devices/arm8_pmuv3_0 > > ... breakpoint -> ../../../devices/breakpoint > > ... cpmu0 -> ../../../devices/pci0000:0c/0000:0c:00.0/0000:0d:00.0/cpmu0/cpmu0 > > etc > > > > (the first cpmu0 is the parent registered as a child of the PCI EP and used for > > driver binding). So it's of use to userspace rather than in the kernel driver > > itself. > > > > Note that almost nothing is normally in the top level /sys/devices other than > > event_sources - because nearly all other struct device instances created by > > subsystems have parents assigned. > > > > On my system > > > > ls /sys/devices > > > > armv8_pmuv3_0 LNXSYSTEM:00 pci0000:0c pnp0 system uprobe > > breakpoint pci0000:00 platform software tracepoint virtual > > > > +CC Greg KH for input on whether / why this make sense. > > That doesn't make sense, nothing should be in /sys/devices/ EXCEPT the > root device of busses. Everything else is wrong and should have their > code fixed up (i.e. "breakpoint", "software", etc.) Thanks Greg. I was thinking to cycle back round to that once I'd got agreement on 'some' devices, but great to have clarity from the start that these should all have parents. For a few cases the parent is not immediately obvious but we'll figure it out. Jonathan > > thanks, > > greg k-h
On Wed, Mar 29, 2023 at 09:55:04AM +0100, Jonathan Cameron wrote: > On Tue, 28 Mar 2023 13:01:08 +0200 > Greg KH <gregkh@linuxfoundation.org> wrote: > > > On Tue, Mar 28, 2023 at 11:54:44AM +0100, Jonathan Cameron wrote: > > > On Mon, 27 Mar 2023 13:04:08 -0400 > > > "Liang, Kan" <kan.liang@linux.intel.com> wrote: > > > > > > > On 2023-03-24 1:13 p.m., Jonathan Cameron wrote: > > > > > Some PMUs have well defined parents such as PCI devices. > > > > > As the device_initialize() and device_add() are all within > > > > > pmu_dev_alloc() which is called from perf_pmu_register() > > > > > there is no opportunity to set the parent from within a driver. > > > > > > > > > > Add a struct device *parent field to struct pmu and use that > > > > > to set the parent. > > > > > > > > Why we want a PMU parent? Maybe I missed it. I don't see that the parent > > > > is used anywhere. > > > > > > This allows you to identify the association between PMU and the hardware related > > > device that is providing it by looking at the directory structure in sysfs rather > > > than putting them directly under /sys/devices. > > > > > > ls -l /sys/bus/event_sources/devices/ > > > > > > ... armv8_pmuv3_0 -> ../../../devices/arm8_pmuv3_0 > > > ... breakpoint -> ../../../devices/breakpoint > > > ... cpmu0 -> ../../../devices/pci0000:0c/0000:0c:00.0/0000:0d:00.0/cpmu0/cpmu0 > > > etc > > > > > > (the first cpmu0 is the parent registered as a child of the PCI EP and used for > > > driver binding). So it's of use to userspace rather than in the kernel driver > > > itself. > > > > > > Note that almost nothing is normally in the top level /sys/devices other than > > > event_sources - because nearly all other struct device instances created by > > > subsystems have parents assigned. > > > > > > On my system > > > > > > ls /sys/devices > > > > > > armv8_pmuv3_0 LNXSYSTEM:00 pci0000:0c pnp0 system uprobe > > > breakpoint pci0000:00 platform software tracepoint virtual > > > > > > +CC Greg KH for input on whether / why this make sense. > > > > That doesn't make sense, nothing should be in /sys/devices/ EXCEPT the > > root device of busses. Everything else is wrong and should have their > > code fixed up (i.e. "breakpoint", "software", etc.) > > Thanks Greg. > > I was thinking to cycle back round to that once I'd got agreement on 'some' devices, > but great to have clarity from the start that these should all have > parents. > > For a few cases the parent is not immediately obvious but we'll figure it out. If there is no "real" parent, then it is a virtual device and belongs under the "virtual" directory. Otherwise why are these "devices" at all with such generic names? thanks, greg k-h
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index d5628a7b5eaa..b99db1eda72c 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -303,6 +303,7 @@ struct pmu { struct module *module; struct device *dev; + struct device *parent; const struct attribute_group **attr_groups; const struct attribute_group **attr_update; const char *name; diff --git a/kernel/events/core.c b/kernel/events/core.c index fb3e436bcd4a..a84c282221f2 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -11367,6 +11367,7 @@ static int pmu_dev_alloc(struct pmu *pmu) dev_set_drvdata(pmu->dev, pmu); pmu->dev->bus = &pmu_bus; + pmu->dev->parent = pmu->parent; pmu->dev->release = pmu_dev_release; ret = dev_set_name(pmu->dev, "%s", pmu->name);