* [GIT PULL] arm64 fix for 5.7-rc8/final
From: Catalin Marinas @ 2020-05-29 17:20 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Will Deacon, linux-kernel, linux-arm-kernel
Hi Linus,
Please pull the arm64 fix below. Thanks.
The following changes since commit 8cfb347ad0cffdbfc69c82506fb3be9429563211:
arm64: Add get_user() type annotation on the !access_ok() path (2020-05-22 16:59:49 +0100)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux tags/arm64-fixes
for you to fetch changes up to ba051f097fc30b00f6b66044386901141351a557:
arm64/kernel: Fix return value when cpu_online() fails in __cpu_up() (2020-05-28 12:04:55 +0100)
----------------------------------------------------------------
Ensure __cpu_up() returns an error if cpu_online() is false after
waiting for completion on cpu_running.
----------------------------------------------------------------
Nobuhiro Iwamatsu (1):
arm64/kernel: Fix return value when cpu_online() fails in __cpu_up()
arch/arm64/kernel/smp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] arm: dts: am335x-boneblack: add gpio-line-names
From: Tony Lindgren @ 2020-05-29 17:21 UTC (permalink / raw)
To: Drew Fustini
Cc: Linus Walleij, Grygorii Strashko, Benoît Cousson,
Rob Herring, Linux-OMAP, devicetree, linux-kernel, Jason Kridner,
Robert Nelson
In-Reply-To: <20200528131620.GA3126290@x1>
* Drew Fustini <drew@beagleboard.org> [200528 13:17]:
> FYI - Linus W. provided an Acked-by in related thread [0].
>
> Anyone else have any review comments?
Looks good to me thanks. But as the merge window is about
to open, let's do fixes only at this point and leave this
for v5.9.
Regards,
Tony
>
> [0] https://lore.kernel.org/linux-devicetree/CACRpkdZLRjcE0FGwVR-Q7a50aEmpB=xO4q6H8_EaV199fGr0OA@mail.gmail.com/
^ permalink raw reply
* Re: [PATCH v4 04/11] thermal: Store device mode in struct thermal_zone_device
From: Andrzej Pietrasiewicz @ 2020-05-29 17:21 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-pm, linux-acpi, netdev, linux-wireless, platform-driver-x86,
linux-arm-kernel, linux-renesas-soc, linux-rockchip,
Emmanuel Grumbach, Heiko Stuebner, Rafael J . Wysocki,
Vishal Kulkarni, Luca Coelho, Miquel Raynal, kernel,
Fabio Estevam, Amit Kucheria, Chunyan Zhang, Daniel Lezcano,
Allison Randal, NXP Linux Team
In-Reply-To: <5010f7df-59d6-92ef-c99a-0dbd715f0ad2@collabora.com>
Hi again,
W dniu 29.05.2020 o 18:08, Andrzej Pietrasiewicz pisze:
> Hi Guenter,
>
> W dniu 29.05.2020 o 17:42, Guenter Roeck pisze:
>> On Thu, May 28, 2020 at 09:20:44PM +0200, Andrzej Pietrasiewicz wrote:
>>> Prepare for eliminating get_mode().
>>>
>> Might be worthwhile to explain (not only in the subject) what you are
>> doing here.
>>
>>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>>> ---
>>> drivers/acpi/thermal.c | 18 ++++++----------
>>> .../ethernet/mellanox/mlxsw/core_thermal.c | 21 +++++++------------
>>> drivers/platform/x86/acerhdf.c | 15 ++++++-------
>>> drivers/thermal/da9062-thermal.c | 6 ++----
>>> drivers/thermal/imx_thermal.c | 17 +++++++--------
>>> .../intel/int340x_thermal/int3400_thermal.c | 12 +++--------
>>> .../thermal/intel/intel_quark_dts_thermal.c | 16 +++++++-------
>>> drivers/thermal/thermal_of.c | 10 +++------
>>
>> After this patch is applied on top of the thermal 'testing' branch,
>> there are still local instances of thermal_device_mode in
>> drivers/thermal/st/stm_thermal.c
>> drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>>
>> If there is a reason not to replace those, it might make sense to explain
>> it here.
>>
>
> My understanding is that these two are sensor devices which are "plugged"
> into their "parent" thermal zone device. The latter is the "proper" tzd.
> They both use thermal_zone_of_device_ops instead of thermal_zone_device_ops.
> The former doesn't even have get_mode(). The thermal core, when it calls
> get_mode(), operates on the "parent" thermal zone devices.
>
> Consequently, the drivers you mention use their "mode" members for
> their private purpose, not for the purpose of storing the "parent"
> thermal zone device mode.
>
Let me also say it differently.
Both drivers which you mention use devm_thermal_zone_of_sensor_register().
It calls thermal_zone_of_sensor_register(), which "will search the list of
thermal zones described in device tree and look for the zone that refer to
the sensor device pointed by @dev->of_node as temperature providers. For
the zone pointing to the sensor node, the sensor will be added to the DT
thermal zone device." When a match is found thermal_zone_of_add_sensor()
is invoked, which (using thermal_zone_get_zone_by_name()) iterates over
all registered thermal_zone_devices. The one eventually found will be
returned and propagated to the original caller of
devm_thermal_zone_of_sensor_register(). The state of this returned
device is managed elsewhere (in that device's struct tzd). The "mode"
member you are referring to is thus unrelated.
Regards,
Andrzej
^ permalink raw reply
* Re: [PATCH v4 04/11] thermal: Store device mode in struct thermal_zone_device
From: Andrzej Pietrasiewicz @ 2020-05-29 17:21 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-pm, linux-acpi, netdev, linux-wireless, platform-driver-x86,
linux-arm-kernel, linux-renesas-soc, linux-rockchip,
Emmanuel Grumbach, Heiko Stuebner, Rafael J . Wysocki,
Vishal Kulkarni, Luca Coelho, Miquel Raynal, kernel,
Fabio Estevam, Amit Kucheria, Chunyan Zhang, Daniel Lezcano,
Allison Randal, NXP Linux Team, Darren Hart, Zhang Rui,
Gayatri Kammela, Len Brown, Johannes Berg, Intel Linux Wireless,
Sascha Hauer, Ido Schimmel, Baolin Wang, Jiri Pirko, Orson Zhai,
Thomas Gleixner, Kalle Valo, Support Opensource, Enrico Weigelt,
Peter Kaestle, Sebastian Reichel, Bartlomiej Zolnierkiewicz,
Pengutronix Kernel Team, Niklas Söderlund, Shawn Guo,
David S . Miller, Andy Shevchenko
In-Reply-To: <5010f7df-59d6-92ef-c99a-0dbd715f0ad2@collabora.com>
Hi again,
W dniu 29.05.2020 o 18:08, Andrzej Pietrasiewicz pisze:
> Hi Guenter,
>
> W dniu 29.05.2020 o 17:42, Guenter Roeck pisze:
>> On Thu, May 28, 2020 at 09:20:44PM +0200, Andrzej Pietrasiewicz wrote:
>>> Prepare for eliminating get_mode().
>>>
>> Might be worthwhile to explain (not only in the subject) what you are
>> doing here.
>>
>>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>>> ---
>>> drivers/acpi/thermal.c | 18 ++++++----------
>>> .../ethernet/mellanox/mlxsw/core_thermal.c | 21 +++++++------------
>>> drivers/platform/x86/acerhdf.c | 15 ++++++-------
>>> drivers/thermal/da9062-thermal.c | 6 ++----
>>> drivers/thermal/imx_thermal.c | 17 +++++++--------
>>> .../intel/int340x_thermal/int3400_thermal.c | 12 +++--------
>>> .../thermal/intel/intel_quark_dts_thermal.c | 16 +++++++-------
>>> drivers/thermal/thermal_of.c | 10 +++------
>>
>> After this patch is applied on top of the thermal 'testing' branch,
>> there are still local instances of thermal_device_mode in
>> drivers/thermal/st/stm_thermal.c
>> drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>>
>> If there is a reason not to replace those, it might make sense to explain
>> it here.
>>
>
> My understanding is that these two are sensor devices which are "plugged"
> into their "parent" thermal zone device. The latter is the "proper" tzd.
> They both use thermal_zone_of_device_ops instead of thermal_zone_device_ops.
> The former doesn't even have get_mode(). The thermal core, when it calls
> get_mode(), operates on the "parent" thermal zone devices.
>
> Consequently, the drivers you mention use their "mode" members for
> their private purpose, not for the purpose of storing the "parent"
> thermal zone device mode.
>
Let me also say it differently.
Both drivers which you mention use devm_thermal_zone_of_sensor_register().
It calls thermal_zone_of_sensor_register(), which "will search the list of
thermal zones described in device tree and look for the zone that refer to
the sensor device pointed by @dev->of_node as temperature providers. For
the zone pointing to the sensor node, the sensor will be added to the DT
thermal zone device." When a match is found thermal_zone_of_add_sensor()
is invoked, which (using thermal_zone_get_zone_by_name()) iterates over
all registered thermal_zone_devices. The one eventually found will be
returned and propagated to the original caller of
devm_thermal_zone_of_sensor_register(). The state of this returned
device is managed elsewhere (in that device's struct tzd). The "mode"
member you are referring to is thus unrelated.
Regards,
Andrzej
^ permalink raw reply
* Re: [PATCH v4 04/11] thermal: Store device mode in struct thermal_zone_device
From: Andrzej Pietrasiewicz @ 2020-05-29 17:21 UTC (permalink / raw)
To: Guenter Roeck
Cc: Emmanuel Grumbach, Heiko Stuebner, Kalle Valo, linux-wireless,
Peter Kaestle, platform-driver-x86, Vishal Kulkarni, Luca Coelho,
Miquel Raynal, Shawn Guo, kernel, Fabio Estevam, Amit Kucheria,
linux-rockchip, Chunyan Zhang, Daniel Lezcano, linux-acpi,
linux-arm-kernel, Darren Hart, Zhang Rui, Gayatri Kammela,
NXP Linux Team, Johannes Berg, linux-pm, Sascha Hauer,
Intel Linux Wireless, Ido Schimmel, Niklas Söderlund,
Jiri Pirko, Orson Zhai, Thomas Gleixner, Allison Randal,
Support Opensource, netdev, Rafael J . Wysocki, Sebastian Reichel,
linux-renesas-soc, Bartlomiej Zolnierkiewicz,
Pengutronix Kernel Team, Baolin Wang, Len Brown, Enrico Weigelt,
David S . Miller, Andy Shevchenko
In-Reply-To: <5010f7df-59d6-92ef-c99a-0dbd715f0ad2@collabora.com>
Hi again,
W dniu 29.05.2020 o 18:08, Andrzej Pietrasiewicz pisze:
> Hi Guenter,
>
> W dniu 29.05.2020 o 17:42, Guenter Roeck pisze:
>> On Thu, May 28, 2020 at 09:20:44PM +0200, Andrzej Pietrasiewicz wrote:
>>> Prepare for eliminating get_mode().
>>>
>> Might be worthwhile to explain (not only in the subject) what you are
>> doing here.
>>
>>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>>> ---
>>> drivers/acpi/thermal.c | 18 ++++++----------
>>> .../ethernet/mellanox/mlxsw/core_thermal.c | 21 +++++++------------
>>> drivers/platform/x86/acerhdf.c | 15 ++++++-------
>>> drivers/thermal/da9062-thermal.c | 6 ++----
>>> drivers/thermal/imx_thermal.c | 17 +++++++--------
>>> .../intel/int340x_thermal/int3400_thermal.c | 12 +++--------
>>> .../thermal/intel/intel_quark_dts_thermal.c | 16 +++++++-------
>>> drivers/thermal/thermal_of.c | 10 +++------
>>
>> After this patch is applied on top of the thermal 'testing' branch,
>> there are still local instances of thermal_device_mode in
>> drivers/thermal/st/stm_thermal.c
>> drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>>
>> If there is a reason not to replace those, it might make sense to explain
>> it here.
>>
>
> My understanding is that these two are sensor devices which are "plugged"
> into their "parent" thermal zone device. The latter is the "proper" tzd.
> They both use thermal_zone_of_device_ops instead of thermal_zone_device_ops.
> The former doesn't even have get_mode(). The thermal core, when it calls
> get_mode(), operates on the "parent" thermal zone devices.
>
> Consequently, the drivers you mention use their "mode" members for
> their private purpose, not for the purpose of storing the "parent"
> thermal zone device mode.
>
Let me also say it differently.
Both drivers which you mention use devm_thermal_zone_of_sensor_register().
It calls thermal_zone_of_sensor_register(), which "will search the list of
thermal zones described in device tree and look for the zone that refer to
the sensor device pointed by @dev->of_node as temperature providers. For
the zone pointing to the sensor node, the sensor will be added to the DT
thermal zone device." When a match is found thermal_zone_of_add_sensor()
is invoked, which (using thermal_zone_get_zone_by_name()) iterates over
all registered thermal_zone_devices. The one eventually found will be
returned and propagated to the original caller of
devm_thermal_zone_of_sensor_register(). The state of this returned
device is managed elsewhere (in that device's struct tzd). The "mode"
member you are referring to is thus unrelated.
Regards,
Andrzej
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] ARM: dts: aspeed: rainier: Add line-name checkstop
From: bentyner @ 2020-05-29 17:22 UTC (permalink / raw)
To: openbmc; +Cc: joel, Ben Tyner
From: Ben Tyner <ben.tyner@ibm.com>
Rainier uses GPIO B6 as the checkstop GPIO. Define the line-name
so that this GPIO can be found by name.
Signed-off-by: Ben Tyner <ben.tyner@ibm.com>
---
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
index 01db238ce741..61723d3131d2 100644
--- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
@@ -73,7 +73,7 @@
&gpio0 {
gpio-line-names =
/*A0-A7*/ "","","","","","","","",
- /*B0-B7*/ "","","","","","","","",
+ /*B0-B7*/ "","","","","","","checkstop","",
/*C0-C7*/ "","","","","","","","",
/*D0-D7*/ "","","","","","","","",
/*E0-E7*/ "","","","","","","","",
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 1/2] migration/rdma: fix potential nullptr access in rdma_start_incoming_migration
From: Dr. David Alan Gilbert @ 2020-05-29 17:21 UTC (permalink / raw)
To: Juan Quintela; +Cc: zhang.zhanghailiang, Pan Nengyuan, qemu-devel, euler.robot
In-Reply-To: <878si2hnht.fsf@secure.mitica>
* Juan Quintela (quintela@redhat.com) wrote:
> Pan Nengyuan <pannengyuan@huawei.com> wrote:
> > 'rdma' is NULL when taking the first error branch in rdma_start_incoming_migration.
> > And it will cause a null pointer access in label 'err'. Fix that.
> >
> > Fixes: 59c59c67ee6b0327ae932deb303caa47919aeb1e
> > Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
>
> good catch.
Thanks, Queued
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply
* Re: [PATCH v1 1/2] spi: dw: Make DMA request line assignments explicit for Intel Medfield
From: Mark Brown @ 2020-05-29 17:22 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-spi
In-Reply-To: <20200528102311.79948-1-andriy.shevchenko@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 420 bytes --]
On Thu, May 28, 2020 at 01:23:10PM +0300, Andy Shevchenko wrote:
> The 2afccbd283ae ("spi: dw: Discard static DW DMA slave structures")
> did a clean up of global variables, which is fine, but messed up with
> the carefully provided information in the custom DMA slave structures.
> There reader can find an assignment of the DMA request lines in use.
This doesn't apply against current code, please rebase and resend.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH] power: pmic: Add SPL Kconfig entry for PFUZE100
From: Marek Vasut @ 2020-05-29 17:22 UTC (permalink / raw)
To: u-boot
Add Kconfig entry for the PFUZE PMIC, SPL variant.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Stefano Babic <sbabic@denx.de>
---
drivers/power/pmic/Kconfig | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
index ef8bf49d49..a62aa38054 100644
--- a/drivers/power/pmic/Kconfig
+++ b/drivers/power/pmic/Kconfig
@@ -105,6 +105,13 @@ config DM_PMIC_PFUZE100
This config enables implementation of driver-model pmic uclass features
for PMIC PFUZE100. The driver implements read/write operations.
+config SPL_DM_PMIC_PFUZE100
+ bool "Enable Driver Model for PMIC PFUZE100 in SPL"
+ depends on DM_PMIC
+ ---help---
+ This config enables implementation of driver-model pmic uclass features
+ for PMIC PFUZE100 in SPL. The driver implements read/write operations.
+
config DM_PMIC_MAX77686
bool "Enable Driver Model for PMIC MAX77686"
depends on DM_PMIC
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v4 05/11] thermal: remove get_mode() operation of drivers
From: Andrzej Pietrasiewicz @ 2020-05-29 17:22 UTC (permalink / raw)
To: Guenter Roeck
Cc: Emmanuel Grumbach, Heiko Stuebner, Kalle Valo, linux-wireless,
Peter Kaestle, platform-driver-x86, Vishal Kulkarni, Luca Coelho,
Miquel Raynal, Shawn Guo, kernel, Fabio Estevam, Amit Kucheria,
linux-rockchip, Chunyan Zhang, Daniel Lezcano, linux-acpi,
linux-arm-kernel, Darren Hart, Zhang Rui, Gayatri Kammela,
NXP Linux Team, Johannes Berg, linux-pm, Sascha
In-Reply-To: <20200529154910.GA158174@roeck-us.net>
Hi Guenter,
W dniu 29.05.2020 o 17:49, Guenter Roeck pisze:
> On Thu, May 28, 2020 at 09:20:45PM +0200, Andrzej Pietrasiewicz wrote:
>> get_mode() is now redundant, as the state is stored in struct
>> thermal_zone_device.
>>
>> Consequently the "mode" attribute in sysfs can always be visible, because
>> it is always possible to get the mode from struct tzd.
>>
>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>
> There is a slight semantic change for the two drivers which still have
> a local copy of enum thermal_device_mode: Previously trying to read the
> mode for those would return -EPERM since they don't have a get_mode
> function. Now the global value for mode is returned, but I am not sure
> if it matches the local value.
Please see my replies to your comment about patch 4/11.
Regards,
Andrzej
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 05/11] thermal: remove get_mode() operation of drivers
From: Andrzej Pietrasiewicz @ 2020-05-29 17:22 UTC (permalink / raw)
To: Guenter Roeck
Cc: Emmanuel Grumbach, Heiko Stuebner, Kalle Valo, linux-wireless,
Peter Kaestle, platform-driver-x86, Vishal Kulkarni, Luca Coelho,
Miquel Raynal, Shawn Guo, kernel, Fabio Estevam, Amit Kucheria,
linux-rockchip, Chunyan Zhang, Daniel Lezcano, linux-acpi,
linux-arm-kernel, Darren Hart, Zhang Rui, Gayatri Kammela,
NXP Linux Team, Johannes Berg, linux-pm, Sascha Hauer,
Intel Linux Wireless, Ido Schimmel, Niklas Söderlund,
Jiri Pirko, Orson Zhai, Thomas Gleixner, Allison Randal,
Support Opensource, netdev, Rafael J . Wysocki, Sebastian Reichel,
linux-renesas-soc, Bartlomiej Zolnierkiewicz,
Pengutronix Kernel Team, Baolin Wang, Len Brown, Enrico Weigelt,
David S . Miller, Andy Shevchenko
In-Reply-To: <20200529154910.GA158174@roeck-us.net>
Hi Guenter,
W dniu 29.05.2020 o 17:49, Guenter Roeck pisze:
> On Thu, May 28, 2020 at 09:20:45PM +0200, Andrzej Pietrasiewicz wrote:
>> get_mode() is now redundant, as the state is stored in struct
>> thermal_zone_device.
>>
>> Consequently the "mode" attribute in sysfs can always be visible, because
>> it is always possible to get the mode from struct tzd.
>>
>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>
> There is a slight semantic change for the two drivers which still have
> a local copy of enum thermal_device_mode: Previously trying to read the
> mode for those would return -EPERM since they don't have a get_mode
> function. Now the global value for mode is returned, but I am not sure
> if it matches the local value.
Please see my replies to your comment about patch 4/11.
Regards,
Andrzej
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: Some -serious- BPF-related litmus tests
From: Joel Fernandes @ 2020-05-29 17:23 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Boqun Feng, Andrii Nakryiko, Paul E . McKenney, Alan Stern,
Peter Zijlstra, parri.andrea, will, npiggin, dhowells, j.alglave,
luc.maranget, Akira Yokosawa, dlustig, open list, linux-arch
In-Reply-To: <CAEf4BzbzyA0mn7O-+x2peGa9WUuaGSi0+Gpyy-6t5iJwVLYf5A@mail.gmail.com>
On Thu, May 28, 2020 at 09:38:35PM -0700, Andrii Nakryiko wrote:
> On Thu, May 28, 2020 at 2:48 PM Joel Fernandes <joel@joelfernandes.org> wrote:
> >
> > On Mon, May 25, 2020 at 11:38:23AM -0700, Andrii Nakryiko wrote:
> > > On Mon, May 25, 2020 at 7:53 AM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > >
> > > > Hi Andrii,
> > > >
> > > > On Fri, May 22, 2020 at 12:38:21PM -0700, Andrii Nakryiko wrote:
> > > > > On 5/22/20 10:43 AM, Paul E. McKenney wrote:
> > > > > > On Fri, May 22, 2020 at 10:32:01AM -0400, Alan Stern wrote:
> > > > > > > On Fri, May 22, 2020 at 11:44:07AM +0200, Peter Zijlstra wrote:
> > > > > > > > On Thu, May 21, 2020 at 05:38:50PM -0700, Paul E. McKenney wrote:
> > > > > > > > > Hello!
> > > > > > > > >
> > > > > > > > > Just wanted to call your attention to some pretty cool and pretty serious
> > > > > > > > > litmus tests that Andrii did as part of his BPF ring-buffer work:
> > > > > > > > >
> > > > > > > > > https://lore.kernel.org/bpf/20200517195727.279322-3-andriin@fb.com/
> > > > > > > > >
> > > > > > > > > Thoughts?
> > > > > > > >
> > > > > > > > I find:
> > > > > > > >
> > > > > > > > smp_wmb()
> > > > > > > > smp_store_release()
> > > > > > > >
> > > > > > > > a _very_ weird construct. What is that supposed to even do?
> > > > > > >
> > > > > > > Indeed, it looks like one or the other of those is redundant (depending
> > > > > > > on the context).
> > > > > >
> > > > > > Probably. Peter instead asked what it was supposed to even do. ;-)
> > > > >
> > > > > I agree, I think smp_wmb() is redundant here. Can't remember why I thought
> > > > > that it's necessary, this algorithm went through a bunch of iterations,
> > > > > starting as completely lockless, also using READ_ONCE/WRITE_ONCE at some
> > > > > point, and settling on smp_read_acquire/smp_store_release, eventually. Maybe
> > > > > there was some reason, but might be that I was just over-cautious. See reply
> > > > > on patch thread as well ([0]).
> > > > >
> > > > > [0] https://lore.kernel.org/bpf/CAEf4Bza26AbRMtWcoD5+TFhnmnU6p5YJ8zO+SoAJCDtp1jVhcQ@mail.gmail.com/
> > > > >
> > > >
> > > > While we are at it, could you explain a bit on why you use
> > > > smp_store_release() on consumer_pos? I ask because IIUC, consumer_pos is
> > > > only updated at consumer side, and there is no other write at consumer
> > > > side that we want to order with the write to consumer_pos. So I fail
> > > > to find why smp_store_release() is necessary.
> > > >
> > > > I did the following modification on litmus tests, and I didn't see
> > > > different results (on States) between two versions of litmus tests.
> > > >
> > >
> > > This is needed to ensure that producer can reliably detect whether it
> > > needs to trigger poll notification.
> >
> > Boqun's question is on the consumer side though. Are you saying that on the
> > consumer side, the loads prior to the smp_store_release() on the consumer
> > side should have been seen by the consumer? You are already using
> > smp_load_acquire() so that should be satisified already because the
> > smp_load_acquire() makes sure that the smp_load_acquire()'s happens before
> > any future loads and stores.
>
> Consumer is reading two things: producer_pos and each record's length
> header, and writes consumer_pos. I re-read this paragraph many times,
> but I'm still a bit confused on what exactly you are trying to say.
This is what I was saying in the other thread. I think you missed that
comment. If you are adding litmus documentation, at least it should be clear
what memory ordering is being verified. Both me and Boqun tried to remove a
memory barrier each and the test still passes. So what exactly are you
verifying from a memory consistency standpoint? I know you have those various
rFail things and conditions - but I am assuming the goal here is to verify
memory consistency as well. Or are we just throwing enough memory barriers at
the problem to make sure the test passes, without understanding exactly what
ordering is needed?
> Can you please specify in each case release()/acquire() of which
> variable you are talking about?
I don't want to speculate and confuse the thread more. I am afraid the burden
of specifying what the various release/acquire orders is on the author of the
code introducing the memory barriers ;-). That is, IMHO you should probably add
code comments in the test about why a certain memory barrier is needed.
That said, I need to do more diligence and read the actual BPF ring buffer
code to understand what you're modeling. I will try to make time to do that.
thanks!
- Joel
^ permalink raw reply
* Re: [PATCH v14 1/1] perf tools: add support for libpfm4
From: Arnaldo Carvalho de Melo @ 2020-05-29 17:23 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Yonghong Song, Andrii Nakryiko,
Greg Kroah-Hartman, Thomas Gleixner, Igor Lubashev,
Alexey Budankov, Florian Fainelli, Adrian Hunter, Andi Kleen,
Jiwei Sun, yuzhoujian <yuzhoujian>
In-Reply-To: <CAP-5=fWn1=DtZyfGtYEFd=-zDY1O+9A1fcG_3bDKsuoQDZ4i=Q@mail.gmail.com>
Em Fri, May 29, 2020 at 10:03:51AM -0700, Ian Rogers escreveu:
> On Tue, May 5, 2020 at 11:29 AM Ian Rogers <irogers@google.com> wrote:
> >
> > From: Stephane Eranian <eranian@google.com>
> >
> > This patch links perf with the libpfm4 library if it is available
> > and LIBPFM4 is passed to the build. The libpfm4 library
> > contains hardware event tables for all processors supported by
> > perf_events. It is a helper library that helps convert from a
> > symbolic event name to the event encoding required by the
> > underlying kernel interface. This library is open-source and
> > available from: http://perfmon2.sf.net.
> >
> > With this patch, it is possible to specify full hardware events
> > by name. Hardware filters are also supported. Events must be
> > specified via the --pfm-events and not -e option. Both options
> > are active at the same time and it is possible to mix and match:
> >
> > $ perf stat --pfm-events inst_retired:any_p:c=1:i -e cycles ....
> >
> > Signed-off-by: Stephane Eranian <eranian@google.com>
> > Reviewed-by: Ian Rogers <irogers@google.com>
>
> Ping.
Check my tmp.perf/core branch, I had to make some adjustments, mostly in
the 'perf test' entries as I merged a java demangle test that touched
the same files,
I'm now doing the build tests.
- Arnaldo
> Thanks,
> Ian
>
> > ---
> > tools/perf/Documentation/perf-record.txt | 11 +
> > tools/perf/Documentation/perf-stat.txt | 10 +
> > tools/perf/Documentation/perf-top.txt | 11 +
> > tools/perf/Makefile.config | 13 ++
> > tools/perf/Makefile.perf | 2 +
> > tools/perf/builtin-record.c | 6 +
> > tools/perf/builtin-stat.c | 6 +
> > tools/perf/builtin-top.c | 6 +
> > tools/perf/tests/Build | 1 +
> > tools/perf/tests/builtin-test.c | 9 +
> > tools/perf/tests/pfm.c | 203 ++++++++++++++++
> > tools/perf/tests/tests.h | 3 +
> > tools/perf/util/Build | 2 +
> > tools/perf/util/evsel.c | 2 +-
> > tools/perf/util/evsel.h | 1 +
> > tools/perf/util/parse-events.c | 30 ++-
> > tools/perf/util/parse-events.h | 4 +
> > tools/perf/util/pfm.c | 281 +++++++++++++++++++++++
> > tools/perf/util/pfm.h | 37 +++
> > 19 files changed, 630 insertions(+), 8 deletions(-)
> > create mode 100644 tools/perf/tests/pfm.c
> > create mode 100644 tools/perf/util/pfm.c
> > create mode 100644 tools/perf/util/pfm.h
> >
> > diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> > index 561ef55743e2..492b6b6f2b77 100644
> > --- a/tools/perf/Documentation/perf-record.txt
> > +++ b/tools/perf/Documentation/perf-record.txt
> > @@ -613,6 +613,17 @@ appended unit character - B/K/M/G
> > The number of threads to run when synthesizing events for existing processes.
> > By default, the number of threads equals 1.
> >
> > +ifdef::HAVE_LIBPFM[]
> > +--pfm-events events::
> > +Select a PMU event using libpfm4 syntax (see http://perfmon2.sf.net)
> > +including support for event filters. For example '--pfm-events
> > +inst_retired:any_p:u:c=1:i'. More than one event can be passed to the
> > +option using the comma separator. Hardware events and generic hardware
> > +events cannot be mixed together. The latter must be used with the -e
> > +option. The -e option and this one can be mixed and matched. Events
> > +can be grouped using the {} notation.
> > +endif::HAVE_LIBPFM[]
> > +
> > SEE ALSO
> > --------
> > linkperf:perf-stat[1], linkperf:perf-list[1], linkperf:perf-intel-pt[1]
> > diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
> > index 3fb5028aef08..b69af18dccd0 100644
> > --- a/tools/perf/Documentation/perf-stat.txt
> > +++ b/tools/perf/Documentation/perf-stat.txt
> > @@ -71,6 +71,16 @@ report::
> > --tid=<tid>::
> > stat events on existing thread id (comma separated list)
> >
> > +ifdef::HAVE_LIBPFM[]
> > +--pfm-events events::
> > +Select a PMU event using libpfm4 syntax (see http://perfmon2.sf.net)
> > +including support for event filters. For example '--pfm-events
> > +inst_retired:any_p:u:c=1:i'. More than one event can be passed to the
> > +option using the comma separator. Hardware events and generic hardware
> > +events cannot be mixed together. The latter must be used with the -e
> > +option. The -e option and this one can be mixed and matched. Events
> > +can be grouped using the {} notation.
> > +endif::HAVE_LIBPFM[]
> >
> > -a::
> > --all-cpus::
> > diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
> > index 20227dabc208..ee2024691d46 100644
> > --- a/tools/perf/Documentation/perf-top.txt
> > +++ b/tools/perf/Documentation/perf-top.txt
> > @@ -329,6 +329,17 @@ Default is to monitor all CPUS.
> > The known limitations include exception handing such as
> > setjmp/longjmp will have calls/returns not match.
> >
> > +ifdef::HAVE_LIBPFM[]
> > +--pfm-events events::
> > +Select a PMU event using libpfm4 syntax (see http://perfmon2.sf.net)
> > +including support for event filters. For example '--pfm-events
> > +inst_retired:any_p:u:c=1:i'. More than one event can be passed to the
> > +option using the comma separator. Hardware events and generic hardware
> > +events cannot be mixed together. The latter must be used with the -e
> > +option. The -e option and this one can be mixed and matched. Events
> > +can be grouped using the {} notation.
> > +endif::HAVE_LIBPFM[]
> > +
> > INTERACTIVE PROMPTING KEYS
> > --------------------------
> >
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index 12a8204d63c6..b67804fee1e3 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -1012,6 +1012,19 @@ ifdef LIBCLANGLLVM
> > endif
> > endif
> >
> > +ifdef LIBPFM4
> > + $(call feature_check,libpfm4)
> > + ifeq ($(feature-libpfm4), 1)
> > + CFLAGS += -DHAVE_LIBPFM
> > + EXTLIBS += -lpfm
> > + ASCIIDOC_EXTRA = -aHAVE_LIBPFM=1
> > + $(call detected,CONFIG_LIBPFM4)
> > + else
> > + msg := $(warning libpfm4 not found, disables libpfm4 support. Please install libpfm4-dev);
> > + NO_LIBPFM4 := 1
> > + endif
> > +endif
> > +
> > # Among the variables below, these:
> > # perfexecdir
> > # perf_include_dir
> > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > index 94a495594e99..dc82578c8773 100644
> > --- a/tools/perf/Makefile.perf
> > +++ b/tools/perf/Makefile.perf
> > @@ -118,6 +118,8 @@ include ../scripts/utilities.mak
> > #
> > # Define LIBBPF_DYNAMIC to enable libbpf dynamic linking.
> > #
> > +# Define LIBPFM4 to enable libpfm4 events extension.
> > +#
> >
> > # As per kernel Makefile, avoid funny character set dependencies
> > unexport LC_ALL
> > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > index e4efdbf1a81e..98387cce3207 100644
> > --- a/tools/perf/builtin-record.c
> > +++ b/tools/perf/builtin-record.c
> > @@ -45,6 +45,7 @@
> > #include "util/units.h"
> > #include "util/bpf-event.h"
> > #include "util/util.h"
> > +#include "util/pfm.h"
> > #include "asm/bug.h"
> > #include "perf.h"
> >
> > @@ -2506,6 +2507,11 @@ static struct option __record_options[] = {
> > OPT_UINTEGER(0, "num-thread-synthesize",
> > &record.opts.nr_threads_synthesize,
> > "number of threads to run for event synthesis"),
> > +#ifdef HAVE_LIBPFM
> > + OPT_CALLBACK(0, "pfm-events", &record.evlist, "event",
> > + "libpfm4 event selector. use 'perf list' to list available events",
> > + parse_libpfm_events_option),
> > +#endif
> > OPT_END()
> > };
> >
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index e0c1ad23c768..f6e2dd57f48e 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -66,6 +66,7 @@
> > #include "util/time-utils.h"
> > #include "util/top.h"
> > #include "util/affinity.h"
> > +#include "util/pfm.h"
> > #include "asm/bug.h"
> >
> > #include <linux/time64.h>
> > @@ -935,6 +936,11 @@ static struct option stat_options[] = {
> > "Use with 'percore' event qualifier to show the event "
> > "counts of one hardware thread by sum up total hardware "
> > "threads of same physical core"),
> > +#ifdef HAVE_LIBPFM
> > + OPT_CALLBACK(0, "pfm-events", &evsel_list, "event",
> > + "libpfm4 event selector. use 'perf list' to list available events",
> > + parse_libpfm_events_option),
> > +#endif
> > OPT_END()
> > };
> >
> > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> > index 372c38254654..20c41d9040ee 100644
> > --- a/tools/perf/builtin-top.c
> > +++ b/tools/perf/builtin-top.c
> > @@ -53,6 +53,7 @@
> >
> > #include "util/debug.h"
> > #include "util/ordered-events.h"
> > +#include "util/pfm.h"
> >
> > #include <assert.h>
> > #include <elf.h>
> > @@ -1575,6 +1576,11 @@ int cmd_top(int argc, const char **argv)
> > "WARNING: should be used on grouped events."),
> > OPT_BOOLEAN(0, "stitch-lbr", &top.stitch_lbr,
> > "Enable LBR callgraph stitching approach"),
> > +#ifdef HAVE_LIBPFM
> > + OPT_CALLBACK(0, "pfm-events", &top.evlist, "event",
> > + "libpfm4 event selector. use 'perf list' to list available events",
> > + parse_libpfm_events_option),
> > +#endif
> > OPTS_EVSWITCH(&top.evswitch),
> > OPT_END()
> > };
> > diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> > index c75557aeef0e..4e74a363b0b0 100644
> > --- a/tools/perf/tests/Build
> > +++ b/tools/perf/tests/Build
> > @@ -57,6 +57,7 @@ perf-y += maps.o
> > perf-y += time-utils-test.o
> > perf-y += genelf.o
> > perf-y += api-io.o
> > +perf-y += pfm.o
> >
> > $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build
> > $(call rule_mkdir)
> > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> > index 3471ec52ea11..57c6f8b31624 100644
> > --- a/tools/perf/tests/builtin-test.c
> > +++ b/tools/perf/tests/builtin-test.c
> > @@ -317,6 +317,15 @@ static struct test generic_tests[] = {
> > .desc = "maps__merge_in",
> > .func = test__maps__merge_in,
> > },
> > + {
> > + .desc = "Test libpfm4 support",
> > + .func = test__pfm,
> > + .subtest = {
> > + .skip_if_fail = true,
> > + .get_nr = test__pfm_subtest_get_nr,
> > + .get_desc = test__pfm_subtest_get_desc,
> > + }
> > + },
> > {
> > .func = NULL,
> > },
> > diff --git a/tools/perf/tests/pfm.c b/tools/perf/tests/pfm.c
> > new file mode 100644
> > index 000000000000..76a53126efdf
> > --- /dev/null
> > +++ b/tools/perf/tests/pfm.c
> > @@ -0,0 +1,203 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Test support for libpfm4 event encodings.
> > + *
> > + * Copyright 2020 Google LLC.
> > + */
> > +#include "tests.h"
> > +#include "util/debug.h"
> > +#include "util/evlist.h"
> > +#include "util/pfm.h"
> > +
> > +#include <linux/kernel.h>
> > +
> > +#ifdef HAVE_LIBPFM
> > +static int test__pfm_events(void);
> > +static int test__pfm_group(void);
> > +#endif
> > +
> > +static const struct {
> > + int (*func)(void);
> > + const char *desc;
> > +} pfm_testcase_table[] = {
> > +#ifdef HAVE_LIBPFM
> > + {
> > + .func = test__pfm_events,
> > + .desc = "test of individual --pfm-events",
> > + },
> > + {
> > + .func = test__pfm_group,
> > + .desc = "test groups of --pfm-events",
> > + },
> > +#endif
> > +};
> > +
> > +#ifdef HAVE_LIBPFM
> > +static int count_pfm_events(struct perf_evlist *evlist)
> > +{
> > + struct perf_evsel *evsel;
> > + int count = 0;
> > +
> > + perf_evlist__for_each_entry(evlist, evsel) {
> > + count++;
> > + }
> > + return count;
> > +}
> > +
> > +static int test__pfm_events(void)
> > +{
> > + struct evlist *evlist;
> > + struct option opt;
> > + size_t i;
> > + const struct {
> > + const char *events;
> > + int nr_events;
> > + } table[] = {
> > + {
> > + .events = "",
> > + .nr_events = 0,
> > + },
> > + {
> > + .events = "instructions",
> > + .nr_events = 1,
> > + },
> > + {
> > + .events = "instructions,cycles",
> > + .nr_events = 2,
> > + },
> > + {
> > + .events = "stereolab",
> > + .nr_events = 0,
> > + },
> > + {
> > + .events = "instructions,instructions",
> > + .nr_events = 2,
> > + },
> > + {
> > + .events = "stereolab,instructions",
> > + .nr_events = 0,
> > + },
> > + {
> > + .events = "instructions,stereolab",
> > + .nr_events = 1,
> > + },
> > + };
> > +
> > + for (i = 0; i < ARRAY_SIZE(table); i++) {
> > + evlist = evlist__new();
> > + if (evlist == NULL)
> > + return -ENOMEM;
> > +
> > + opt.value = evlist;
> > + parse_libpfm_events_option(&opt,
> > + table[i].events,
> > + 0);
> > + TEST_ASSERT_EQUAL(table[i].events,
> > + count_pfm_events(&evlist->core),
> > + table[i].nr_events);
> > + TEST_ASSERT_EQUAL(table[i].events,
> > + evlist->nr_groups,
> > + 0);
> > +
> > + evlist__delete(evlist);
> > + }
> > + return 0;
> > +}
> > +
> > +static int test__pfm_group(void)
> > +{
> > + struct evlist *evlist;
> > + struct option opt;
> > + size_t i;
> > + const struct {
> > + const char *events;
> > + int nr_events;
> > + int nr_groups;
> > + } table[] = {
> > + {
> > + .events = "{},",
> > + .nr_events = 0,
> > + .nr_groups = 0,
> > + },
> > + {
> > + .events = "{instructions}",
> > + .nr_events = 1,
> > + .nr_groups = 1,
> > + },
> > + {
> > + .events = "{instructions},{}",
> > + .nr_events = 1,
> > + .nr_groups = 1,
> > + },
> > + {
> > + .events = "{},{instructions}",
> > + .nr_events = 0,
> > + .nr_groups = 0,
> > + },
> > + {
> > + .events = "{instructions},{instructions}",
> > + .nr_events = 2,
> > + .nr_groups = 2,
> > + },
> > + {
> > + .events = "{instructions,cycles},{instructions,cycles}",
> > + .nr_events = 4,
> > + .nr_groups = 2,
> > + },
> > + {
> > + .events = "{stereolab}",
> > + .nr_events = 0,
> > + .nr_groups = 0,
> > + },
> > + {
> > + .events =
> > + "{instructions,cycles},{instructions,stereolab}",
> > + .nr_events = 3,
> > + .nr_groups = 1,
> > + },
> > + };
> > +
> > + for (i = 0; i < ARRAY_SIZE(table); i++) {
> > + evlist = evlist__new();
> > + if (evlist == NULL)
> > + return -ENOMEM;
> > +
> > + opt.value = evlist;
> > + parse_libpfm_events_option(&opt,
> > + table[i].events,
> > + 0);
> > + TEST_ASSERT_EQUAL(table[i].events,
> > + count_pfm_events(&evlist->core),
> > + table[i].nr_events);
> > + TEST_ASSERT_EQUAL(table[i].events,
> > + evlist->nr_groups,
> > + table[i].nr_groups);
> > +
> > + evlist__delete(evlist);
> > + }
> > + return 0;
> > +}
> > +#endif
> > +
> > +const char *test__pfm_subtest_get_desc(int i)
> > +{
> > + if (i < 0 || i >= (int)ARRAY_SIZE(pfm_testcase_table))
> > + return NULL;
> > + return pfm_testcase_table[i].desc;
> > +}
> > +
> > +int test__pfm_subtest_get_nr(void)
> > +{
> > + return (int)ARRAY_SIZE(pfm_testcase_table);
> > +}
> > +
> > +int test__pfm(struct test *test __maybe_unused, int i __maybe_unused)
> > +{
> > +#ifdef HAVE_LIBPFM
> > + if (i < 0 || i >= (int)ARRAY_SIZE(pfm_testcase_table))
> > + return TEST_FAIL;
> > + return pfm_testcase_table[i].func();
> > +#else
> > + return TEST_SKIP;
> > +#endif
> > +}
> > diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> > index d6d4ac34eeb7..a2ae0d2e6087 100644
> > --- a/tools/perf/tests/tests.h
> > +++ b/tools/perf/tests/tests.h
> > @@ -113,6 +113,9 @@ int test__maps__merge_in(struct test *t, int subtest);
> > int test__time_utils(struct test *t, int subtest);
> > int test__jit_write_elf(struct test *test, int subtest);
> > int test__api_io(struct test *test, int subtest);
> > +int test__pfm(struct test *test, int subtest);
> > +const char *test__pfm_subtest_get_desc(int subtest);
> > +int test__pfm_subtest_get_nr(void);
> >
> > bool test__bp_signal_is_supported(void);
> > bool test__bp_account_is_supported(void);
> > diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> > index ca07a162d602..dfda916f0b4c 100644
> > --- a/tools/perf/util/Build
> > +++ b/tools/perf/util/Build
> > @@ -179,6 +179,8 @@ perf-$(CONFIG_LIBBPF) += bpf-event.o
> >
> > perf-$(CONFIG_CXX) += c++/
> >
> > +perf-$(CONFIG_LIBPFM4) += pfm.o
> > +
> > CFLAGS_config.o += -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
> > CFLAGS_llvm-utils.o += -DPERF_INCLUDE_DIR="BUILD_STR($(perf_include_dir_SQ))"
> >
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index f3e60c45d59a..d7ea1a7e74cd 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -2416,7 +2416,7 @@ bool evsel__fallback(struct evsel *evsel, int err, char *msg, size_t msgsize)
> >
> > /* Is there already the separator in the name. */
> > if (strchr(name, '/') ||
> > - strchr(name, ':'))
> > + (strchr(name, ':') && !evsel->is_libpfm_event))
> > sep = "";
> >
> > if (asprintf(&new_name, "%s%su", name, sep) < 0)
> > diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> > index 351c0aaf2a11..5a4f20ddeb49 100644
> > --- a/tools/perf/util/evsel.h
> > +++ b/tools/perf/util/evsel.h
> > @@ -76,6 +76,7 @@ struct evsel {
> > bool ignore_missing_thread;
> > bool forced_leader;
> > bool use_uncore_alias;
> > + bool is_libpfm_event;
> > /* parse modifier helper */
> > int exclude_GH;
> > int sample_read;
> > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > index b7a0518d607d..0ee3338a0449 100644
> > --- a/tools/perf/util/parse-events.c
> > +++ b/tools/perf/util/parse-events.c
> > @@ -36,6 +36,7 @@
> > #include "metricgroup.h"
> > #include "util/evsel_config.h"
> > #include "util/event.h"
> > +#include "util/pfm.h"
> >
> > #define MAX_NAME_LEN 100
> >
> > @@ -344,6 +345,7 @@ static char *get_config_name(struct list_head *head_terms)
> > static struct evsel *
> > __add_event(struct list_head *list, int *idx,
> > struct perf_event_attr *attr,
> > + bool init_attr,
> > char *name, struct perf_pmu *pmu,
> > struct list_head *config_terms, bool auto_merge_stats,
> > const char *cpu_list)
> > @@ -352,7 +354,8 @@ __add_event(struct list_head *list, int *idx,
> > struct perf_cpu_map *cpus = pmu ? pmu->cpus :
> > cpu_list ? perf_cpu_map__new(cpu_list) : NULL;
> >
> > - event_attr_init(attr);
> > + if (init_attr)
> > + event_attr_init(attr);
> >
> > evsel = perf_evsel__new_idx(attr, *idx);
> > if (!evsel)
> > @@ -370,15 +373,25 @@ __add_event(struct list_head *list, int *idx,
> > if (config_terms)
> > list_splice(config_terms, &evsel->config_terms);
> >
> > - list_add_tail(&evsel->core.node, list);
> > + if (list)
> > + list_add_tail(&evsel->core.node, list);
> > +
> > return evsel;
> > }
> >
> > +struct evsel *parse_events__add_event(int idx, struct perf_event_attr *attr,
> > + char *name, struct perf_pmu *pmu)
> > +{
> > + return __add_event(NULL, &idx, attr, false, name, pmu, NULL, false,
> > + NULL);
> > +}
> > +
> > static int add_event(struct list_head *list, int *idx,
> > struct perf_event_attr *attr, char *name,
> > struct list_head *config_terms)
> > {
> > - return __add_event(list, idx, attr, name, NULL, config_terms, false, NULL) ? 0 : -ENOMEM;
> > + return __add_event(list, idx, attr, true, name, NULL, config_terms,
> > + false, NULL) ? 0 : -ENOMEM;
> > }
> >
> > static int add_event_tool(struct list_head *list, int *idx,
> > @@ -390,7 +403,8 @@ static int add_event_tool(struct list_head *list, int *idx,
> > .config = PERF_COUNT_SW_DUMMY,
> > };
> >
> > - evsel = __add_event(list, idx, &attr, NULL, NULL, NULL, false, "0");
> > + evsel = __add_event(list, idx, &attr, true, NULL, NULL, NULL, false,
> > + "0");
> > if (!evsel)
> > return -ENOMEM;
> > evsel->tool_event = tool_event;
> > @@ -1446,8 +1460,8 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
> >
> > if (!head_config) {
> > attr.type = pmu->type;
> > - evsel = __add_event(list, &parse_state->idx, &attr, NULL, pmu, NULL,
> > - auto_merge_stats, NULL);
> > + evsel = __add_event(list, &parse_state->idx, &attr, true, NULL,
> > + pmu, NULL, auto_merge_stats, NULL);
> > if (evsel) {
> > evsel->pmu_name = name ? strdup(name) : NULL;
> > evsel->use_uncore_alias = use_uncore_alias;
> > @@ -1488,7 +1502,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
> > return -EINVAL;
> > }
> >
> > - evsel = __add_event(list, &parse_state->idx, &attr,
> > + evsel = __add_event(list, &parse_state->idx, &attr, true,
> > get_config_name(head_config), pmu,
> > &config_terms, auto_merge_stats, NULL);
> > if (evsel) {
> > @@ -2817,6 +2831,8 @@ void print_events(const char *event_glob, bool name_only, bool quiet_flag,
> > print_sdt_events(NULL, NULL, name_only);
> >
> > metricgroup__print(true, true, NULL, name_only, details_flag);
> > +
> > + print_libpfm_events(name_only, long_desc);
> > }
> >
> > int parse_events__is_hardcoded_term(struct parse_events_term *term)
> > diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> > index 6ead9661238c..04e3f627c081 100644
> > --- a/tools/perf/util/parse-events.h
> > +++ b/tools/perf/util/parse-events.h
> > @@ -17,6 +17,7 @@ struct evlist;
> > struct parse_events_error;
> >
> > struct option;
> > +struct perf_pmu;
> >
> > struct tracepoint_path {
> > char *system;
> > @@ -187,6 +188,9 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
> > bool auto_merge_stats,
> > bool use_alias);
> >
> > +struct evsel *parse_events__add_event(int idx, struct perf_event_attr *attr,
> > + char *name, struct perf_pmu *pmu);
> > +
> > int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
> > char *str,
> > struct list_head **listp);
> > diff --git a/tools/perf/util/pfm.c b/tools/perf/util/pfm.c
> > new file mode 100644
> > index 000000000000..d735acb6c29c
> > --- /dev/null
> > +++ b/tools/perf/util/pfm.c
> > @@ -0,0 +1,281 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Support for libpfm4 event encoding.
> > + *
> > + * Copyright 2020 Google LLC.
> > + */
> > +#include "util/cpumap.h"
> > +#include "util/debug.h"
> > +#include "util/event.h"
> > +#include "util/evlist.h"
> > +#include "util/evsel.h"
> > +#include "util/parse-events.h"
> > +#include "util/pmu.h"
> > +#include "util/pfm.h"
> > +
> > +#include <string.h>
> > +#include <linux/kernel.h>
> > +#include <perfmon/pfmlib_perf_event.h>
> > +
> > +static void libpfm_initialize(void)
> > +{
> > + int ret;
> > +
> > + ret = pfm_initialize();
> > + if (ret != PFM_SUCCESS) {
> > + ui__warning("libpfm failed to initialize: %s\n",
> > + pfm_strerror(ret));
> > + }
> > +}
> > +
> > +int parse_libpfm_events_option(const struct option *opt, const char *str,
> > + int unset __maybe_unused)
> > +{
> > + struct evlist *evlist = *(struct evlist **)opt->value;
> > + struct perf_event_attr attr;
> > + struct perf_pmu *pmu;
> > + struct evsel *evsel, *grp_leader = NULL;
> > + char *p, *q, *p_orig;
> > + const char *sep;
> > + int grp_evt = -1;
> > + int ret;
> > +
> > + libpfm_initialize();
> > +
> > + p_orig = p = strdup(str);
> > + if (!p)
> > + return -1;
> > + /*
> > + * force loading of the PMU list
> > + */
> > + perf_pmu__scan(NULL);
> > +
> > + for (q = p; strsep(&p, ",{}"); q = p) {
> > + sep = p ? str + (p - p_orig - 1) : "";
> > + if (*sep == '{') {
> > + if (grp_evt > -1) {
> > + ui__error(
> > + "nested event groups not supported\n");
> > + goto error;
> > + }
> > + grp_evt++;
> > + }
> > +
> > + /* no event */
> > + if (*q == '\0')
> > + continue;
> > +
> > + memset(&attr, 0, sizeof(attr));
> > + event_attr_init(&attr);
> > +
> > + ret = pfm_get_perf_event_encoding(q, PFM_PLM0|PFM_PLM3,
> > + &attr, NULL, NULL);
> > +
> > + if (ret != PFM_SUCCESS) {
> > + ui__error("failed to parse event %s : %s\n", str,
> > + pfm_strerror(ret));
> > + goto error;
> > + }
> > +
> > + pmu = perf_pmu__find_by_type((unsigned int)attr.type);
> > + evsel = parse_events__add_event(evlist->core.nr_entries,
> > + &attr, q, pmu);
> > + if (evsel == NULL)
> > + goto error;
> > +
> > + evsel->is_libpfm_event = true;
> > +
> > + evlist__add(evlist, evsel);
> > +
> > + if (grp_evt == 0)
> > + grp_leader = evsel;
> > +
> > + if (grp_evt > -1) {
> > + evsel->leader = grp_leader;
> > + grp_leader->core.nr_members++;
> > + grp_evt++;
> > + }
> > +
> > + if (*sep == '}') {
> > + if (grp_evt < 0) {
> > + ui__error(
> > + "cannot close a non-existing event group\n");
> > + goto error;
> > + }
> > + evlist->nr_groups++;
> > + grp_leader = NULL;
> > + grp_evt = -1;
> > + }
> > + }
> > + return 0;
> > +error:
> > + free(p_orig);
> > + return -1;
> > +}
> > +
> > +static const char *srcs[PFM_ATTR_CTRL_MAX] = {
> > + [PFM_ATTR_CTRL_UNKNOWN] = "???",
> > + [PFM_ATTR_CTRL_PMU] = "PMU",
> > + [PFM_ATTR_CTRL_PERF_EVENT] = "perf_event",
> > +};
> > +
> > +static void
> > +print_attr_flags(pfm_event_attr_info_t *info)
> > +{
> > + int n = 0;
> > +
> > + if (info->is_dfl) {
> > + printf("[default] ");
> > + n++;
> > + }
> > +
> > + if (info->is_precise) {
> > + printf("[precise] ");
> > + n++;
> > + }
> > +
> > + if (!n)
> > + printf("- ");
> > +}
> > +
> > +static void
> > +print_libpfm_events_detailed(pfm_event_info_t *info, bool long_desc)
> > +{
> > + pfm_event_attr_info_t ainfo;
> > + const char *src;
> > + int j, ret;
> > +
> > + ainfo.size = sizeof(ainfo);
> > +
> > + printf(" %s\n", info->name);
> > + printf(" [%s]\n", info->desc);
> > + if (long_desc) {
> > + if (info->equiv)
> > + printf(" Equiv: %s\n", info->equiv);
> > +
> > + printf(" Code : 0x%"PRIx64"\n", info->code);
> > + }
> > + pfm_for_each_event_attr(j, info) {
> > + ret = pfm_get_event_attr_info(info->idx, j,
> > + PFM_OS_PERF_EVENT_EXT, &ainfo);
> > + if (ret != PFM_SUCCESS)
> > + continue;
> > +
> > + if (ainfo.type == PFM_ATTR_UMASK) {
> > + printf(" %s:%s\n", info->name, ainfo.name);
> > + printf(" [%s]\n", ainfo.desc);
> > + }
> > +
> > + if (!long_desc)
> > + continue;
> > +
> > + if (ainfo.ctrl >= PFM_ATTR_CTRL_MAX)
> > + ainfo.ctrl = PFM_ATTR_CTRL_UNKNOWN;
> > +
> > + src = srcs[ainfo.ctrl];
> > + switch (ainfo.type) {
> > + case PFM_ATTR_UMASK:
> > + printf(" Umask : 0x%02"PRIx64" : %s: ",
> > + ainfo.code, src);
> > + print_attr_flags(&ainfo);
> > + putchar('\n');
> > + break;
> > + case PFM_ATTR_MOD_BOOL:
> > + printf(" Modif : %s: [%s] : %s (boolean)\n", src,
> > + ainfo.name, ainfo.desc);
> > + break;
> > + case PFM_ATTR_MOD_INTEGER:
> > + printf(" Modif : %s: [%s] : %s (integer)\n", src,
> > + ainfo.name, ainfo.desc);
> > + break;
> > + case PFM_ATTR_NONE:
> > + case PFM_ATTR_RAW_UMASK:
> > + case PFM_ATTR_MAX:
> > + default:
> > + printf(" Attr : %s: [%s] : %s\n", src,
> > + ainfo.name, ainfo.desc);
> > + }
> > + }
> > +}
> > +
> > +/*
> > + * list all pmu::event:umask, pmu::event
> > + * printed events may not be all valid combinations of umask for an event
> > + */
> > +static void
> > +print_libpfm_events_raw(pfm_pmu_info_t *pinfo, pfm_event_info_t *info)
> > +{
> > + pfm_event_attr_info_t ainfo;
> > + int j, ret;
> > + bool has_umask = false;
> > +
> > + ainfo.size = sizeof(ainfo);
> > +
> > + pfm_for_each_event_attr(j, info) {
> > + ret = pfm_get_event_attr_info(info->idx, j,
> > + PFM_OS_PERF_EVENT_EXT, &ainfo);
> > + if (ret != PFM_SUCCESS)
> > + continue;
> > +
> > + if (ainfo.type != PFM_ATTR_UMASK)
> > + continue;
> > +
> > + printf("%s::%s:%s\n", pinfo->name, info->name, ainfo.name);
> > + has_umask = true;
> > + }
> > + if (!has_umask)
> > + printf("%s::%s\n", pinfo->name, info->name);
> > +}
> > +
> > +void print_libpfm_events(bool name_only, bool long_desc)
> > +{
> > + pfm_event_info_t info;
> > + pfm_pmu_info_t pinfo;
> > + int i, p, ret;
> > +
> > + libpfm_initialize();
> > +
> > + /* initialize to zero to indicate ABI version */
> > + info.size = sizeof(info);
> > + pinfo.size = sizeof(pinfo);
> > +
> > + if (!name_only)
> > + puts("\nList of pre-defined events (to be used in --pfm-events):\n");
> > +
> > + pfm_for_all_pmus(p) {
> > + bool printed_pmu = false;
> > +
> > + ret = pfm_get_pmu_info(p, &pinfo);
> > + if (ret != PFM_SUCCESS)
> > + continue;
> > +
> > + /* only print events that are supported by host HW */
> > + if (!pinfo.is_present)
> > + continue;
> > +
> > + /* handled by perf directly */
> > + if (pinfo.pmu == PFM_PMU_PERF_EVENT)
> > + continue;
> > +
> > + for (i = pinfo.first_event; i != -1;
> > + i = pfm_get_event_next(i)) {
> > +
> > + ret = pfm_get_event_info(i, PFM_OS_PERF_EVENT_EXT,
> > + &info);
> > + if (ret != PFM_SUCCESS)
> > + continue;
> > +
> > + if (!name_only && !printed_pmu) {
> > + printf("%s:\n", pinfo.name);
> > + printed_pmu = true;
> > + }
> > +
> > + if (!name_only)
> > + print_libpfm_events_detailed(&info, long_desc);
> > + else
> > + print_libpfm_events_raw(&pinfo, &info);
> > + }
> > + if (!name_only && printed_pmu)
> > + putchar('\n');
> > + }
> > +}
> > diff --git a/tools/perf/util/pfm.h b/tools/perf/util/pfm.h
> > new file mode 100644
> > index 000000000000..7d70dda87012
> > --- /dev/null
> > +++ b/tools/perf/util/pfm.h
> > @@ -0,0 +1,37 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Support for libpfm4 event encoding.
> > + *
> > + * Copyright 2020 Google LLC.
> > + */
> > +#ifndef __PERF_PFM_H
> > +#define __PERF_PFM_H
> > +
> > +#include <subcmd/parse-options.h>
> > +
> > +#ifdef HAVE_LIBPFM
> > +int parse_libpfm_events_option(const struct option *opt, const char *str,
> > + int unset);
> > +
> > +void print_libpfm_events(bool name_only, bool long_desc);
> > +
> > +#else
> > +#include <linux/compiler.h>
> > +
> > +static inline int parse_libpfm_events_option(
> > + const struct option *opt __maybe_unused,
> > + const char *str __maybe_unused,
> > + int unset __maybe_unused)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline void print_libpfm_events(bool name_only __maybe_unused,
> > + bool long_desc __maybe_unused)
> > +{
> > +}
> > +
> > +#endif
> > +
> > +
> > +#endif /* __PERF_PFM_H */
> > --
> > 2.26.2.526.g744177e7f7-goog
> >
--
- Arnaldo
^ permalink raw reply
* Re: [PATCH BlueZ 2/5] mesh: Debug output clean up
From: Gix, Brian @ 2020-05-29 17:22 UTC (permalink / raw)
To: linux-bluetooth@vger.kernel.org, Stotland, Inga
In-Reply-To: <20200529063750.186278-3-inga.stotland@intel.com>
Hi Inga,
On Thu, 2020-05-28 at 23:37 -0700, Inga Stotland wrote:
> This changes l_info() to l_debug() for recurring cases and
> removes some excessive debug output.
> ---
> mesh/manager.c | 6 +-
> mesh/mesh-io-generic.c | 3 +-
> mesh/model.c | 28 +-------
> mesh/net.c | 150 ++++++++++++++---------------------------
> mesh/pb-adv.c | 7 +-
> mesh/prov-initiator.c | 2 +-
> 6 files changed, 58 insertions(+), 138 deletions(-)
>
> diff --git a/mesh/manager.c b/mesh/manager.c
> index a7383e4d5..2be471088 100644
> --- a/mesh/manager.c
> +++ b/mesh/manager.c
> @@ -219,7 +219,7 @@ static void add_start(void *user_data, int err)
> {
> struct l_dbus_message *reply;
>
> - l_info("Start callback");
> + l_debug("Start callback");
>
> if (err == MESH_ERROR_NONE)
> reply = l_dbus_message_new_method_return(add_pending->msg);
> @@ -270,8 +270,8 @@ static struct l_dbus_message *add_node_call(struct l_dbus *dbus,
> add_pending->agent = node_get_agent(node);
>
> if (!node_is_provisioner(node) || (add_pending->agent == NULL)) {
> - l_info("Provisioner: %d", node_is_provisioner(node));
> - l_info("Agent: %p", add_pending->agent);
> + l_debug("Provisioner: %d", node_is_provisioner(node));
> + l_debug("Agent: %p", add_pending->agent);
> reply = dbus_error(msg, MESH_ERROR_NOT_AUTHORIZED,
> "Missing Interfaces");
> goto fail;
> diff --git a/mesh/mesh-io-generic.c b/mesh/mesh-io-generic.c
> index 3ad130567..7a4008bd9 100644
> --- a/mesh/mesh-io-generic.c
> +++ b/mesh/mesh-io-generic.c
> @@ -173,7 +173,7 @@ static void event_callback(const void *buf, uint8_t size, void *user_data)
> break;
>
> default:
> - l_info("Other Meta Evt - %d", event);
> + l_debug("Other Meta Evt - %d", event);
> }
> }
>
> @@ -804,7 +804,6 @@ static bool recv_register(struct mesh_io *io, const uint8_t *filter,
> if (!cb || !filter || !len)
> return false;
>
> - l_info("%s %2.2x", __func__, filter[0]);
> rx_reg = l_queue_remove_if(pvt->rx_regs, find_by_filter, filter);
>
> l_free(rx_reg);
> diff --git a/mesh/model.c b/mesh/model.c
> index 945583324..dc6f28eb8 100644
> --- a/mesh/model.c
> +++ b/mesh/model.c
> @@ -514,15 +514,8 @@ static void cmplt(uint16_t remote, uint8_t status,
> {
> struct timeval tx_end;
>
> - if (status)
> - l_debug("Tx-->%4.4x (%d octets) Failed (%d)",
> - remote, size, status);
> - else
> - l_debug("Tx-->%4.4x (%d octets) Succeeded", remote, size);
> -
> - /* print_packet("Sent Data", data, size); */
> -
> gettimeofday(&tx_end, NULL);
> +
> if (tx_end.tv_sec == tx_start.tv_sec) {
> l_debug("Duration 0.%6.6lu seconds",
> tx_end.tv_usec - tx_start.tv_usec);
> @@ -580,22 +573,18 @@ static bool msg_send(struct mesh_node *node, bool credential, uint16_t src,
> net_idx = appkey_net_idx(node_get_net(node), app_idx);
> }
>
> - l_debug("(%x) %p", app_idx, key);
> - l_debug("net_idx %x", net_idx);
> -
> out = l_malloc(out_len);
>
> iv_index = mesh_net_get_iv_index(net);
>
> seq_num = mesh_net_next_seq_num(net);
> +
> if (!mesh_crypto_payload_encrypt(label, msg, out, msg_len, src, dst,
> key_aid, seq_num, iv_index, szmic, key)) {
> l_error("Failed to Encrypt Payload");
> goto done;
> }
>
> - /* print_packet("Encrypted with", key, 16); */
> -
> ret = mesh_net_app_send(net, credential, src, dst, key_aid, net_idx,
> ttl, seq_num, iv_index, segmented,
> szmic, out, out_len, cmplt, NULL);
> @@ -647,8 +636,6 @@ static void model_bind_idx(struct mesh_node *node, struct mesh_model *mod,
>
> l_queue_push_tail(mod->bindings, L_UINT_TO_PTR(idx));
>
> - l_debug("Add %4.4x to model %8.8x", idx, mod->id);
> -
I would like to preserve debug output (or perhaps "rework" is a better word) most state changing Config Server
commands such as this, and including:
Model Bind
Add/Update App/Net Keys
Set TTL/Beaconing/(othwer features)
Add Pubs/Subs
etc
These should be infrequent enough that they shouldn't be noisy, but they allow developers to make sure internal
daemon functionality states are being set up correctly.
In the case of this one, I might change it to
l_debug("Bind key %3.3x to model %8.8x on element %4.4x")
> if (!mod->cbs)
> /* External model */
> config_update_model_bindings(node, mod);
> @@ -781,7 +768,6 @@ static int add_virt_sub(struct mesh_net *net, struct mesh_model *mod,
>
> l_queue_push_head(mod->virtuals, virt);
> mesh_net_dst_reg(net, virt->addr);
> - l_debug("Added virtual sub addr %4.4x", virt->addr);
> }
>
> if (dst)
> @@ -809,7 +795,6 @@ static int add_sub(struct mesh_net *net, struct mesh_model *mod,
>
> l_queue_push_tail(mod->subs, L_UINT_TO_PTR(grp));
> mesh_net_dst_reg(net, grp);
> - l_debug("Added group subscription %4.4x", grp);
> }
>
> return MESH_STATUS_SUCCESS;
> @@ -929,6 +914,7 @@ bool mesh_model_rx(struct mesh_node *node, bool szmict, uint32_t seq0,
>
> /* Don't process if already in RPL */
> crpl = node_get_crpl(node);
> +
> if (net_msg_check_replay_cache(net, src, crpl, seq, iv_index))
> return false;
>
> @@ -1064,8 +1050,6 @@ int mesh_model_publish(struct mesh_node *node, uint32_t mod_id,
> bool result;
> int status;
>
> - /* print_packet("Mod Tx", msg, msg_len); */
> -
> if (!net || msg_len > 380)
> return MESH_ERROR_INVALID_ARGS;
>
> @@ -1092,8 +1076,6 @@ int mesh_model_publish(struct mesh_node *node, uint32_t mod_id,
> if (mod->pub->virt)
> label = mod->pub->virt->label;
>
> - l_debug("publish dst=%x", mod->pub->addr);
> -
> net_idx = appkey_net_idx(net, mod->pub->idx);
>
> result = msg_send(node, mod->pub->credential != 0, src,
> @@ -1108,8 +1090,6 @@ bool mesh_model_send(struct mesh_node *node, uint16_t src, uint16_t dst,
> uint8_t ttl, bool segmented,
> const void *msg, uint16_t msg_len)
> {
> - /* print_packet("Mod Tx", msg, msg_len); */
> -
> /* If SRC is 0, use the Primary Element */
> if (src == 0)
> src = node_get_primary(node);
> @@ -1279,14 +1259,12 @@ void mesh_model_app_key_delete(struct mesh_node *node, struct l_queue *models,
> int mesh_model_binding_del(struct mesh_node *node, uint16_t addr, uint32_t id,
> uint16_t app_idx)
> {
> - l_debug("0x%x, 0x%x, %d", addr, id, app_idx);
> return update_binding(node, addr, id, app_idx, true);
> }
>
> int mesh_model_binding_add(struct mesh_node *node, uint16_t addr, uint32_t id,
> uint16_t app_idx)
> {
> - l_debug("0x%x, 0x%x, %d", addr, id, app_idx);
> return update_binding(node, addr, id, app_idx, false);
> }
>
> diff --git a/mesh/net.c b/mesh/net.c
> index f104be0f9..f9e7bce4c 100644
> --- a/mesh/net.c
> +++ b/mesh/net.c
> @@ -307,7 +307,7 @@ static void trigger_heartbeat(struct mesh_net *net, uint16_t feature,
> {
> struct mesh_net_heartbeat *hb = &net->heartbeat;
>
> - l_info("%s: %4.4x --> %d", __func__, feature, in_use);
> + l_debug("%s: %4.4x --> %d", __func__, feature, in_use);
> if (in_use) {
> if (net->heartbeat.features & feature)
> return; /* no change */
> @@ -371,7 +371,7 @@ static void frnd_kr_phase2(void *a, void *b)
> * receives it's first Poll using the new keys (?)
> */
>
> - l_info("Use Both KeySet %d && %d for %4.4x",
> + l_debug("Use Both KeySet %d && %d for %4.4x",
> frnd->net_key_cur, frnd->net_key_upd, frnd->lp_addr);
> }
>
> @@ -379,7 +379,7 @@ static void frnd_kr_phase3(void *a, void *b)
> {
> struct mesh_friend *frnd = a;
>
> - l_info("Replace KeySet %d with %d for %4.4x",
> + l_debug("Replace KeySet %d with %d for %4.4x",
> frnd->net_key_cur, frnd->net_key_upd, frnd->lp_addr);
> net_key_unref(frnd->net_key_cur);
> frnd->net_key_cur = frnd->net_key_upd;
> @@ -627,10 +627,6 @@ static void queue_friend_update(struct mesh_net *net)
> update.u.one[0].data[1] = flags;
> l_put_be32(net->iv_index, update.u.one[0].data + 2);
> update.u.one[0].data[6] = 0x01; /* More Data */
> - /* print_packet("Frnd-Beacon-SRC",
> - * beacon_data, sizeof(beacon_data));
> - */
> - /* print_packet("Frnd-Update", update.u.one[0].data, 6); */
>
> l_queue_foreach(net->friends, enqueue_update, &update);
> }
> @@ -1570,7 +1566,7 @@ static void send_frnd_ack(struct mesh_net *net, uint16_t src, uint16_t dst,
> l_put_be32(hdr, msg);
> l_put_be32(flags, msg + 3);
>
> - l_info("Send Friend ACK to Segs: %8.8x", flags);
> + l_debug("Send Friend ACK to Segs: %8.8x", flags);
>
> if (is_lpn_friend(net, dst)) {
> /* If we are acking our LPN Friend, queue, don't send */
> @@ -1603,7 +1599,7 @@ static void send_net_ack(struct mesh_net *net, struct mesh_sar *sar,
>
> l_put_be32(hdr, msg);
> l_put_be32(flags, msg + 3);
> - l_info("Send%s ACK to Segs: %8.8x", sar->frnd ? " Friend" : "", flags);
> + l_debug("Send%s ACK to Segs: %8.8x", sar->frnd ? " Friend" : "", flags);
>
> if (is_lpn_friend(net, dst)) {
> /* If we are acking our LPN Friend, queue, don't send */
> @@ -1629,7 +1625,7 @@ static void inseg_to(struct l_timeout *seg_timeout, void *user_data)
> return;
>
> /* Send NAK */
> - l_info("Timeout %p %3.3x", sar, sar->app_idx);
> + l_debug("Timeout %p %3.3x", sar, sar->app_idx);
> send_net_ack(net, sar, sar->flags);
>
> sar->seg_timeout = l_timeout_create(SEG_TO, inseg_to, net, NULL);
> @@ -1647,7 +1643,6 @@ static void inmsg_to(struct l_timeout *msg_timeout, void *user_data)
>
> sar->msg_timeout = NULL;
>
> - /* print_packet("Incoming SAR Timeout", sar->buf, sar->len); */
> mesh_sar_free(sar);
> }
>
> @@ -1668,7 +1663,6 @@ static void outmsg_to(struct l_timeout *msg_timeout, void *user_data)
> sar->buf, sar->len - 4,
> sar->user_data);
>
> - /* print_packet("Outgoing SAR Timeout", sar->buf, sar->len); */
> mesh_sar_free(sar);
> }
>
> @@ -1698,13 +1692,13 @@ static void ack_received(struct mesh_net *net, bool timeout,
> uint32_t ack_copy = ack_flag;
> uint16_t i;
>
> - l_info("ACK Rxed (%x) (to:%d): %8.8x", seq0, timeout, ack_flag);
> + l_debug("ACK Rxed (%x) (to:%d): %8.8x", seq0, timeout, ack_flag);
>
> outgoing = l_queue_find(net->sar_out, match_sar_seq0,
> L_UINT_TO_PTR(seq0));
>
> if (!outgoing) {
> - l_info("Not Found: %4.4x", seq0);
> + l_debug("Not Found: %4.4x", seq0);
> return;
> }
>
> @@ -1743,7 +1737,7 @@ static void ack_received(struct mesh_net *net, bool timeout,
>
> ack_copy |= seg_flag;
>
> - l_info("Resend Seg %d net:%p dst:%x app_idx:%3.3x",
> + l_debug("Resend Seg %d net:%p dst:%x app_idx:%3.3x",
> i, net, outgoing->remote, outgoing->app_idx);
>
> send_seg(net, outgoing, i);
> @@ -1918,12 +1912,12 @@ static void friend_seg_rxed(struct mesh_net *net,
> if (segN == segO)
> frnd_msg->last_len = size;
>
> - l_info("RXed Seg %d, Flags %8.8x (cnt: %d)",
> + l_debug("RXed Seg %d, Flags %8.8x (cnt: %d)",
> segO, frnd_msg->flags, cnt);
>
> /* In reality, if one of these is true, then *both* must be true */
> if ((cnt == segN) || (frnd_msg->flags == expected)) {
> - l_info("Full ACK");
> + l_debug("Full ACK");
> send_frnd_ack(net, dst, src, hdr, frnd_msg->flags);
>
> if (frnd_msg->ttl > 1) {
> @@ -1943,7 +1937,7 @@ static void friend_seg_rxed(struct mesh_net *net,
>
> /* Always ACK if this is the largest outstanding segment */
> if ((largest & frnd_msg->flags) == largest) {
> - l_info("Partial ACK");
> + l_debug("Partial ACK");
> send_frnd_ack(net, dst, src, hdr, frnd_msg->flags);
> }
>
> @@ -1997,7 +1991,7 @@ static bool seg_rxed(struct mesh_net *net, bool frnd, uint32_t iv_index,
> expected = 0xffffffff >> (31 - segN);
>
> if (sar_in) {
> - l_info("RXed (old: %04x %06x size:%d) %d of %d",
> + l_debug("RXed (old: %04x %06x size:%d) %d of %d",
> seqZero, seq, size, segO, segN);
> /* Sanity Check--> certain things must match */
> if (SEG_MAX(true, sar_in->len) != segN ||
> @@ -2012,7 +2006,7 @@ static bool seg_rxed(struct mesh_net *net, bool frnd, uint32_t iv_index,
> } else {
> uint16_t len = MAX_SEG_TO_LEN(segN);
>
> - l_info("RXed (new: %04x %06x size: %d len: %d) %d of %d",
> + l_debug("RXed (new: %04x %06x size: %d len: %d) %d of %d",
> seqZero, seq, size, len, segO, segN);
> l_debug("Queue Size: %d", l_queue_length(net->sar_in));
> sar_in = mesh_sar_new(len);
> @@ -2031,7 +2025,6 @@ static bool seg_rxed(struct mesh_net *net, bool frnd, uint32_t iv_index,
> l_debug("First Seg %4.4x", sar_in->flags);
> l_queue_push_head(net->sar_in, sar_in);
> }
> - /* print_packet("Seg", data, size); */
>
> seg_off = segO * MAX_SEG_LEN;
> memcpy(sar_in->buf + seg_off, data, size);
> @@ -2044,8 +2037,6 @@ static bool seg_rxed(struct mesh_net *net, bool frnd, uint32_t iv_index,
> sar_in->flags |= this_seg_flag;
> sar_in->ttl = ttl;
>
> - l_debug("Have Frags %4.4x", sar_in->flags);
> -
> /* Msg length only definitive on last segment */
> if (segO == segN)
> sar_in->len = segN * MAX_SEG_LEN + size;
> @@ -2166,7 +2157,7 @@ static bool ctl_received(struct mesh_net *net, uint16_t key_id,
> l_queue_find(net->friends,
> match_by_friend,
> L_UINT_TO_PTR(l_get_be16(pkt))));
> - l_info("Remaining Friends: %d", l_queue_length(net->friends));
> + l_debug("Remaining Friends: %d", l_queue_length(net->friends));
> break;
>
> case NET_OP_PROXY_SUB_ADD:
> @@ -2212,7 +2203,7 @@ static bool ctl_received(struct mesh_net *net, uint16_t key_id,
> if (net->heartbeat.sub_max_hops < hops)
> net->heartbeat.sub_max_hops = hops;
>
> - l_info("HB: cnt:%4.4x min:%2.2x max:%2.2x",
> + l_debug("HB: cnt:%4.4x min:%2.2x max:%2.2x",
> net->heartbeat.sub_count,
> net->heartbeat.sub_min_hops,
> net->heartbeat.sub_max_hops);
> @@ -2381,8 +2372,6 @@ static enum _relay_advice packet_received(void *user_data,
> if (is_us(net, net_src, true))
> return RELAY_NONE;
>
> - l_debug("check %08x", cache_cookie);
> -
> /* As a Relay, suppress repeats of last N packets that pass through */
> /* The "cache_cookie" should be unique part of App message */
> if (msg_in_cache(net, net_src, net_seq, cache_cookie))
> @@ -2394,7 +2383,7 @@ static enum _relay_advice packet_received(void *user_data,
> if (is_us(net, net_dst, false) ||
> (net_ctl && net_opcode == NET_OP_HEARTBEAT)) {
>
> - l_info("RX: App 0x%04x -> 0x%04x : TTL 0x%02x : SEQ 0x%06x",
> + l_debug("RX: App 0x%04x -> 0x%04x : TTL 0x%02x : SEQ 0x%06x",
> net_src, net_dst, net_ttl, net_seq);
>
> if (net_ctl) {
> @@ -2404,7 +2393,6 @@ static enum _relay_advice packet_received(void *user_data,
> if (net_dst & 0x8000)
> return RELAY_NONE;
>
> - /* print_packet("Got ACK", msg, app_msg_len); */
> /* Pedantic check for correct size */
> if (app_msg_len != 7)
> return RELAY_NONE;
> @@ -2576,7 +2564,7 @@ static void iv_upd_to(struct l_timeout *upd_timeout, void *user_data)
> case IV_UPD_UPDATING:
> if (l_queue_length(net->sar_out) ||
> l_queue_length(net->sar_queue)) {
> - l_info("don't leave IV Update until sar_out empty");
> + l_debug("don't leave IV Update until sar_out empty");
> l_timeout_modify(net->iv_update_timeout, 10);
> break;
> }
> @@ -2626,7 +2614,7 @@ static int key_refresh_phase_two(struct mesh_net *net, uint16_t idx)
> if (!subnet || !subnet->net_key_upd)
> return MESH_STATUS_INVALID_NETKEY;
>
> - l_info("Key refresh procedure phase 2: start using new net TX keys");
> + l_debug("Key refresh procedure phase 2: start using new net TX keys");
> subnet->key_refresh = 1;
> subnet->net_key_tx = subnet->net_key_upd;
> /* TODO: Provisioner may need to stay in phase three until
> @@ -2659,7 +2647,7 @@ static int key_refresh_finish(struct mesh_net *net, uint16_t idx)
> if (subnet->kr_phase == KEY_REFRESH_PHASE_NONE)
> return MESH_STATUS_SUCCESS;
>
> - l_info("Key refresh phase 3: use new keys only, discard old ones");
> + l_debug("Key refresh phase 3: use new keys only, discard old ones");
>
> /* Switch to using new keys, discard old ones */
> net_key_unref(subnet->net_key_cur);
> @@ -2715,12 +2703,12 @@ static void update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,
> * Starting permissive state will allow us maximum
> * (96 hours) to resync
> */
> - l_info("iv_upd_state = IV_UPD_UPDATING");
> + l_debug("iv_upd_state = IV_UPD_UPDATING");
> net->iv_upd_state = IV_UPD_UPDATING;
> net->iv_update_timeout = l_timeout_create(
> IV_IDX_UPD_MIN, iv_upd_to, net, NULL);
> } else {
> - l_info("iv_upd_state = IV_UPD_NORMAL");
> + l_debug("iv_upd_state = IV_UPD_NORMAL");
> net->iv_upd_state = IV_UPD_NORMAL;
> }
> } else if (ivu) {
> @@ -2736,7 +2724,7 @@ static void update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,
> return;
>
> if (!net->iv_update) {
> - l_info("iv_upd_state = IV_UPD_UPDATING");
> + l_debug("iv_upd_state = IV_UPD_UPDATING");
> net->iv_upd_state = IV_UPD_UPDATING;
> net->iv_update_timeout = l_timeout_create(
> IV_IDX_UPD_MIN, iv_upd_to, net, NULL);
> @@ -2937,7 +2925,6 @@ bool mesh_net_attach(struct mesh_net *net, struct mesh_io *io)
> if (!fast_cache)
> fast_cache = l_queue_new();
>
> - l_info("Register io cb");
> mesh_io_register_recv_cb(io, snb, sizeof(snb),
> beacon_recv, NULL);
> mesh_io_register_recv_cb(io, pkt, sizeof(pkt),
> @@ -2981,7 +2968,7 @@ bool mesh_net_iv_index_update(struct mesh_net *net)
> if (net->iv_upd_state != IV_UPD_NORMAL)
> return false;
>
> - l_info("iv_upd_state = IV_UPD_UPDATING");
> + l_debug("iv_upd_state = IV_UPD_UPDATING");
> mesh_net_flush_msg_queues(net);
> if (!mesh_config_write_iv_index(node_config_get(net->node),
> net->iv_index + 1, true))
> @@ -3082,23 +3069,18 @@ static bool send_seg(struct mesh_net *net, struct mesh_sar *msg, uint8_t segO)
> mesh_net_iv_index_update(net);
>
> l_debug("segN %d segment %d seg_off %d", segN, segO, seg_off);
> - /* print_packet("Sending", msg->buf + seg_off, seg_len); */
> - {
> - /* TODO: Are we RXing on an LPN's behalf? Then set RLY bit */
> -
> - if (!mesh_crypto_packet_build(false, msg->ttl,
> - seq_num,
> - msg->src, msg->remote,
> - 0,
> - msg->segmented, msg->key_aid,
> - msg->szmic, false, msg->seqZero,
> - segO, segN,
> +
> + /* TODO: Are we RXing on an LPN's behalf? Then set RLY bit */
> + if (!mesh_crypto_packet_build(false, msg->ttl, seq_num, msg->src,
> + msg->remote, 0, msg->segmented,
> + msg->key_aid, msg->szmic, false,
> + msg->seqZero, segO, segN,
> msg->buf + seg_off, seg_len,
> packet + 1, &packet_len)) {
> - l_error("Failed to build packet");
> - return false;
> - }
> + l_error("Failed to build packet");
> + return false;
> }
> +
> print_packet("Clr-Net Tx", packet + 1, packet_len);
>
> subnet = l_queue_find(net->subnets, match_key_index,
> @@ -3112,20 +3094,7 @@ static bool send_seg(struct mesh_net *net, struct mesh_sar *msg, uint8_t segO)
> return false;
> }
>
> - /* print_packet("Step 4", packet + 1, packet_len); */
> -
> - {
> - char *str;
> -
> - send_msg_pkt(net, packet, packet_len + 1);
> -
> - str = l_util_hexstring(packet + 1, packet_len);
> - l_info("TX: Network %04x -> %04x : %s (%u) : TTL %d : SEQ %06x",
> - msg->src, msg->remote, str,
> - packet_len, msg->ttl,
> - msg->frnd ? msg->seqAuth + segO : seq_num);
> - l_free(str);
> - }
> + send_msg_pkt(net, packet, packet_len + 1);
>
> msg->last_seg = segO;
>
> @@ -3140,7 +3109,6 @@ void mesh_net_send_seg(struct mesh_net *net, uint32_t net_key_id,
> uint32_t hdr,
> const void *seg, uint16_t seg_len)
> {
> - char *str;
> uint8_t packet[30];
> uint8_t packet_len;
> bool segmented = !!((hdr >> SEG_HDR_SHIFT) & true);
> @@ -3174,14 +3142,12 @@ void mesh_net_send_seg(struct mesh_net *net, uint32_t net_key_id,
> return;
> }
>
> - /* print_packet("Step 4", packet + 1, packet_len); */
> -
> send_msg_pkt(net, packet, packet_len + 1);
>
> - str = l_util_hexstring(packet + 1, packet_len);
> - l_info("TX: Friend Seg-%d %04x -> %04x : %s (%u) : TTL %d : SEQ %06x",
> - segO, src, dst, str, packet_len, ttl, seq);
> - l_free(str);
> + l_debug("TX: Friend Seg-%d %04x -> %04x : len %u) : TTL %d : SEQ %06x",
> + segO, src, dst, packet_len, ttl, seq);
> +
> + print_packet("TX: Friend", packet + 1, packet_len);
> }
>
> bool mesh_net_app_send(struct mesh_net *net, bool frnd_cred, uint16_t src,
> @@ -3222,7 +3188,8 @@ bool mesh_net_app_send(struct mesh_net *net, bool frnd_cred, uint16_t src,
> szmic, seq & SEQ_ZERO_MASK,
> msg, msg_len);
>
> - /* If addressed to a unicast address and successfully enqueued,
> + /*
> + * If addressed to a unicast address and successfully enqueued,
> * or delivered to one of our Unicast addresses we are done
> */
> if ((result && IS_UNICAST(dst)) || src == dst ||
> @@ -3243,6 +3210,7 @@ bool mesh_net_app_send(struct mesh_net *net, bool frnd_cred, uint16_t src,
> payload->iv_index = mesh_net_get_iv_index(net);
> payload->seqAuth = seq;
> payload->segmented = segmented;
> +
> if (segmented) {
> payload->flags = 0xffffffff >> (31 - seg_max);
> payload->seqZero = seq & SEQ_ZERO_MASK;
> @@ -3256,13 +3224,14 @@ bool mesh_net_app_send(struct mesh_net *net, bool frnd_cred, uint16_t src,
> /* Delay sending Outbound SAR unless prior
> * SAR to same DST has completed */
>
> - l_info("OB-Queued SeqZero: %4.4x", payload->seqZero);
> + l_debug("OB-Queued SeqZero: %4.4x", payload->seqZero);
> l_queue_push_tail(net->sar_queue, payload);
> return true;
> }
> }
>
> result = true;
> +
> if (!IS_UNICAST(dst) && segmented) {
> int i;
>
> @@ -3303,7 +3272,6 @@ void mesh_net_ack_send(struct mesh_net *net, uint32_t key_id,
> uint8_t data[7];
> uint8_t pkt_len;
> uint8_t pkt[30];
> - char *str;
>
> hdr = NET_OP_SEG_ACKNOWLEDGE << OPCODE_HDR_SHIFT;
> hdr |= rly << RELAY_HDR_SHIFT;
> @@ -3334,14 +3302,11 @@ void mesh_net_ack_send(struct mesh_net *net, uint32_t key_id,
> return;
> }
>
> - /* print_packet("Step 4", pkt, pkt_len); */
> send_msg_pkt(net, pkt, pkt_len + 1);
>
> - str = l_util_hexstring(pkt + 1, pkt_len);
> - l_info("TX: Friend ACK %04x -> %04x : %s (%u) : TTL %d : SEQ %06x",
> - src, dst, str, pkt_len,
> - ttl, seq);
> - l_free(str);
> + l_debug("TX: Friend ACK %04x -> %04x : len %u : TTL %d : SEQ %06x",
> + src, dst, pkt_len, ttl, seq);
> + print_packet("TX: Friend ACK", pkt + 1, pkt_len);
> }
>
> void mesh_net_transport_send(struct mesh_net *net, uint32_t key_id,
> @@ -3419,26 +3384,13 @@ void mesh_net_transport_send(struct mesh_net *net, uint32_t key_id,
> pkt + 1, &pkt_len))
> return;
>
> - /* print_packet("Step 2", pkt + 1, pkt_len); */
> -
> if (!net_key_encrypt(key_id, iv_index, pkt + 1, pkt_len)) {
> l_error("Failed to encode packet");
> return;
> }
>
> - /* print_packet("Step 4", pkt + 1, pkt_len); */
> -
> - if (dst != 0) {
> - char *str;
> -
> + if (dst != 0)
> send_msg_pkt(net, pkt, pkt_len + 1);
> -
> - str = l_util_hexstring(pkt + 1, pkt_len);
> - l_info("TX: Network %04x -> %04x : %s (%u) : TTL %d : SEQ %06x",
> - src, dst, str, pkt_len,
> - ttl, use_seq);
> - l_free(str);
> - }
> }
>
> uint8_t mesh_net_key_refresh_phase_set(struct mesh_net *net, uint16_t idx,
> @@ -3524,7 +3476,7 @@ int mesh_net_update_key(struct mesh_net *net, uint16_t idx,
>
> if (subnet->net_key_upd) {
> net_key_unref(subnet->net_key_upd);
> - l_info("Warning: overwriting new keys");
> + l_debug("Warning: overwriting new keys");
> }
>
> /* Preserve starting data */
> @@ -3539,7 +3491,7 @@ int mesh_net_update_key(struct mesh_net *net, uint16_t idx,
> l_queue_foreach(net->friends, frnd_kr_phase1,
> L_UINT_TO_PTR(subnet->net_key_upd));
>
> - l_info("key refresh phase 1: Key ID %d", subnet->net_key_upd);
> + l_debug("key refresh phase 1: Key ID %d", subnet->net_key_upd);
>
> if (!mesh_config_net_key_update(node_config_get(net->node), idx, value))
> return MESH_STATUS_STORAGE_FAIL;
> @@ -3812,9 +3764,6 @@ bool net_msg_check_replay_cache(struct mesh_net *net, uint16_t src,
> rpl_get_list(net->node, net->replay_cache);
> }
>
> - l_debug("Test Replay src: %4.4x seq: %6.6x iv: %8.8x",
> - src, seq, iv_index);
> -
> rpe = l_queue_find(net->replay_cache, match_replay_cache,
> L_UINT_TO_PTR(src));
>
> @@ -3854,7 +3803,6 @@ void net_msg_add_replay_cache(struct mesh_net *net, uint16_t src, uint32_t seq,
> L_UINT_TO_PTR(src));
>
> if (!rpe) {
> - l_debug("New Entry for %4.4x", src);
> rpe = l_new(struct mesh_rpl, 1);
> rpe->src = src;
> }
> diff --git a/mesh/pb-adv.c b/mesh/pb-adv.c
> index c1316ed1b..ae5b81391 100644
> --- a/mesh/pb-adv.c
> +++ b/mesh/pb-adv.c
> @@ -157,8 +157,6 @@ static void send_adv_segs(struct pb_adv_session *session, const uint8_t *data,
> init_size = size;
> }
>
> - /* print_packet("FULL-TX", data, size); */
> -
> l_debug("Sending %u fragments for %u octets", max_seg + 1, size);
>
> buf[6] = max_seg << 2;
> @@ -168,7 +166,6 @@ static void send_adv_segs(struct pb_adv_session *session, const uint8_t *data,
>
> l_debug("max_seg: %2.2x", max_seg);
> l_debug("size: %2.2x, CRC: %2.2x", size, buf[9]);
> - /* print_packet("PB-TX", buf + 1, init_size + 9); */
>
> pb_adv_send(session, MESH_IO_TX_COUNT_UNLIMITED, 200,
> buf, init_size + 10);
> @@ -186,8 +183,6 @@ static void send_adv_segs(struct pb_adv_session *session, const uint8_t *data,
> buf[6] = (i << 2) | 0x02;
> memcpy(buf + 7, data + consumed, seg_size);
>
> - /* print_packet("PB-TX", buf + 1, seg_size + 6); */
> -
> pb_adv_send(session, MESH_IO_TX_COUNT_UNLIMITED, 200,
> buf, seg_size + 7);
>
> @@ -225,7 +220,7 @@ static void tx_timeout(struct l_timeout *timeout, void *user_data)
>
> mesh_send_cancel(filter, sizeof(filter));
>
> - l_info("TX timeout");
> + l_debug("TX timeout");
> cb = session->close_cb;
> user_data = session->user_data;
> cb(user_data, 1);
> diff --git a/mesh/prov-initiator.c b/mesh/prov-initiator.c
> index 4ef31e95d..4de4df62d 100644
> --- a/mesh/prov-initiator.c
> +++ b/mesh/prov-initiator.c
> @@ -770,7 +770,7 @@ static void int_prov_rx(void *user_data, const uint8_t *data, uint16_t len)
> break;
>
> case PROV_COMPLETE: /* Complete */
> - l_info("Provisioning Complete");
> + l_debug("Provisioning Complete");
> prov->state = INT_PROV_IDLE;
> int_prov_close(prov, PROV_ERR_SUCCESS);
> break;
^ permalink raw reply
* Re: Xen XSM/FLASK policy, grub defaults, etc.
From: Julien Grall @ 2020-05-29 17:24 UTC (permalink / raw)
To: Jan Beulich
Cc: Jürgen Groß, Stefano Stabellini, cjwatson@debian.org,
Wei Liu, Andrew Cooper, George Dunlap,
xen-devel@lists.xenproject.org, Ian Jackson, Daniel De Graaf
In-Reply-To: <63838ce9-8dbf-aacf-521d-97540758a945@suse.com>
Hi Jan,
On 29/05/2020 16:11, Jan Beulich wrote:
> On 29.05.2020 17:05, Julien Grall wrote:
>> On 29/05/2020 15:47, Ian Jackson wrote:
>>> George Dunlap writes ("Re: Xen XSM/FLASK policy, grub defaults, etc."):
>>>> Which isn’t to say we shouldn’t do it; but it might be nice to also have an intermediate solution that works right now, even if it’s not optimal.
>>>
>>> I propose the following behaviour by updste-grub:
>>>
>>> 1. Look for an ELF note, TBD. If it's found, make XSM boot entries.
>>> (For now, skip this step, since the ELF note is not defined.)
>>
>> I am afraid the ELF note is a very x86 thing. On Arm, we don't have such
>> thing for the kernel/xen (actually the final binary is not even an ELF).
>
> Ouch - of course. Is there anything similar one could employ there?
In the past, we discussed about adding support for note in the zImage
(arm32 kernel format) but I never got the chance to pursue the discussion.
With Juergen's hypfs series, the hypervisor now embbed the .config. Is
it possible to extract it?
Cheers,
--
Julien Grall
^ permalink raw reply
* Re: [sched/fair] 0b0695f2b3: phoronix-test-suite.compress-gzip.0.seconds 19.8% regression
From: Vincent Guittot @ 2020-05-29 17:26 UTC (permalink / raw)
To: lkp
In-Reply-To: <CAKfTPtD+JW-mBt20vHAwOBxo7wbYG3seAc2+t2dWkqSzxf3dSQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5731 bytes --]
On Mon, 25 May 2020 at 10:02, Vincent Guittot
<vincent.guittot@linaro.org> wrote:
>
> On Thu, 21 May 2020 at 10:28, Oliver Sang <oliver.sang@intel.com> wrote:
> >
> > On Wed, May 20, 2020 at 03:04:48PM +0200, Vincent Guittot wrote:
> > > On Thu, 14 May 2020 at 19:09, Vincent Guittot
> > > <vincent.guittot@linaro.org> wrote:
> > > >
> > > > Hi Oliver,
> > > >
> > > > On Thu, 14 May 2020 at 16:05, kernel test robot <oliver.sang@intel.com> wrote:
> > > > >
> > > > > Hi Vincent Guittot,
> > > > >
> > > > > Below report FYI.
> > > > > Last year, we actually reported an improvement "[sched/fair] 0b0695f2b3:
> > > > > vm-scalability.median 3.1% improvement" on link [1].
> > > > > but now we found the regression on pts.compress-gzip.
> > > > > This seems align with what showed in "[v4,00/10] sched/fair: rework the CFS
> > > > > load balance" (link [2]), where showed the reworked load balance could have
> > > > > both positive and negative effect for different test suites.
> > > >
> > > > We have tried to run all possible use cases but it's impossible to
> > > > covers all so there were a possibility that one that is not covered,
> > > > would regressed.
> > > >
> > > > > And also from link [3], the patch set risks regressions.
> > > > >
> > > > > We also confirmed this regression on another platform
> > > > > (Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz with 8G memory),
> > > > > below is the data (lower is better).
> > > > > v5.4 4.1
> > > > > fcf0553db6f4c79387864f6e4ab4a891601f395e 4.01
> > > > > 0b0695f2b34a4afa3f6e9aa1ff0e5336d8dad912 4.89
> > > > > v5.5 5.18
> > > > > v5.6 4.62
> > > > > v5.7-rc2 4.53
> > > > > v5.7-rc3 4.59
> > > > >
> > > > > It seems there are some recovery on latest kernels, but not fully back.
> > > > > We were just wondering whether you could share some lights the further works
> > > > > on the load balance after patch set [2] which could cause the performance
> > > > > change?
> > > > > And whether you have plan to refine the load balance algorithm further?
> > > >
> > > > I'm going to have a look at your regression to understand what is
> > > > going wrong and how it can be fixed
> > >
> > > I have run the benchmark on my local setups to try to reproduce the
> > > regression and I don't see the regression. But my setups are different
> > > from your so it might be a problem specific to yours
> >
> > Hi Vincent, which OS are you using? We found the regression on Clear OS,
> > but it cannot reproduce on Debian.
> > On https://www.phoronix.com/scan.php?page=article&item=mac-win-linux2018&num=5
> > it was mentioned that -
> > Gzip compression is much faster out-of-the-box on Clear Linux due to it exploiting
> > multi-threading capabilities compared to the other operating systems Gzip support.
>
> I'm using Debian, I haven't noticed it was only on Clear OS.
> I'm going to look at it. Could you send me traces in the meantime ?
I run more tests to understand the problem. Even if Clear Linux uses
multithreading, the system is not overloaded and there is a
significant amount of idle time. This means that we use the has_spare
capacity path that spreads tasks on the system. At least that is what
I have seen in the KVM image. Beside this, I think that I have been
able to reproduce the problem on my platform with debian using pigz
instead of gzip for the compress-gzip-1.2.0 test. On my platform, I
can see a difference when I enable all CPU idle states whereas there
is no performance difference when only the shallowest idle state is
enabled.
The new load balance rework is more efficient at spreading tasks on
the system and one side effect could be that there is more idle time
between tasks wake up on each CPU. As a result, CPUs have to wake up
from a deeper idle state. This could explain the +54% increase of C6
usage that is reported. Is it possible to get All C-state statistics
?
Could you run the test after disabling deep idle states like C6 and
above and check what is the difference between v5.7-rc7 and v5.4 ?
comparing fcf0553db6 ("sched/fair: Remove meaningless imbalance
calculation") and
0b0695f2b3 ("sched/fair: Rework load_balance()") is not really
useful because they are part of the same rework and should be
considered a one single change.
>
> >
> > >
> > > After analysing the benchmark, it doesn't overload the system and is
> > > mainly based on 1 main gzip thread with few others waking up and
> > > sleeping around.
> > >
> > > I thought that scheduler could be too aggressive when trying to
> > > balance the threads on your system, which could generate more task
> > > migrations and impact the performance. But this doesn't seem to be the
> > > case because perf-stat.i.cpu-migrations is -8%. On the other side, the
> > > context switch is +16% and more interestingly idle state C1E and C6
> > > usages increase more than 50%. I don't know if we can rely or this
> > > value or not but I wonder if it could be that threads are now spread
> > > on different CPUs which generates idle time on the busy CPUs but the
> > > added time to enter/leave these states hurts the performance.
> > >
> > > Could you make some traces of both kernels ? Tracing sched events
> > > should be enough to understand the behavior
> > >
> > > Regards,
> > > Vincent
> > >
> > > >
> > > > Thanks
> > > > Vincent
> > > >
> > > > > thanks
> > > > >
> > > > > [1] https://lists.01.org/hyperkitty/list/lkp(a)lists.01.org/thread/SANC7QLYZKUNMM6O7UNR3OAQAKS5BESE/
> > > > > [2] https://lore.kernel.org/patchwork/cover/1141687/
> > > > > [3] https://www.phoronix.com/scan.php?page=news_item&px=Linux-5.5-Scheduler
^ permalink raw reply
* Re: Scanning while in AP mode
From: Erik =?unknown-8bit?q?Bot=C3=B6?= @ 2020-05-29 17:26 UTC (permalink / raw)
To: iwd
In-Reply-To: <166f7dab-1d55-e726-699f-86b458088ec2@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2436 bytes --]
Hi,
On Fri, May 29, 2020 at 5:19 PM Denis Kenzior <denkenz@gmail.com> wrote:
>
> Hi Erik,
>
> On 5/29/20 8:16 AM, Erik Botö wrote:
> > Hi,
> >
> > I want to scan for networks while having an active AP. From what I
> > understand this can be achieved by using NL80211_SCAN_FLAG_AP.
> >
> > The documentation for it say:
> > * @NL80211_SCAN_FLAG_AP: force a scan even if the interface is configured
> > * as AP and the beaconing has already been configured. This attribute is
> > * dangerous because will destroy stations performance as a lot of frames
> > * will be lost while scanning off-channel, therefore it must be used only
> > * when really needed
> >
> > So I suppose it's not something that should be enabled by default, but
> > it would be useful to be able to start a scan with this if explicitly
> > requested.
> >
> > My use case for this is initial device setup. A new device would be
> > configured to start up in AP-mode and a inital setup service would run
> > that e.g. a phone could connect to and be presented with a list of
> > found networks. Then a network can be selected and credentials setup,
> > then the device will shut down the AP and instead connect as a client
> > to the selected network. Like many IoT devices etc let's you do when
> > configuring.
>
> So let me see if I understood correctly. You want to have wlan0:
>
> 1. Switch to Device.Mode = ap
> 2. Wait until 'something' connects to wlan0
> 3. Run a scan on wlan0 and tell that 'something' what networks are seen
> 4. 'something' configures one of the seen networks
> 5. wlan0 Device.Mode is then switched to 'station' and it connects to
> the network configured in 4.
Yes, that's exactly what I want.
>
> And to enable this you want to add scanning functionality to the
> AccessPoint interface and have scan.c support this SCAN_FLAG_AP flag.
> If above is correct, then...
>
> >
> > What do you think, is it something that could fit into iwd or is there
> > some reasons to not support this?
>
> I don't see any reason not to. Feel free to come up with an initial
> proposal and we can go from there.
Great, I'll write a first attempt at a patch and we'll take it from there.
Cheers,
Erik
>
> Regards,
> -Denis
> _______________________________________________
> iwd mailing list -- iwd(a)lists.01.org
> To unsubscribe send an email to iwd-leave(a)lists.01.org
^ permalink raw reply
* Re: [PATCH] ASoC: mediatek: mt6358: support DMIC one-wire mode
From: Tzung-Bi Shih @ 2020-05-29 17:26 UTC (permalink / raw)
To: Mark Brown
Cc: Hariprasad Kelam, Linux Kernel Mailing List, howie.huang,
Takashi Iwai, ALSA development, Jiaxin Yu, Liam Girdwood,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <20200529130539.GK4610@sirena.org.uk>
On Fri, May 29, 2020 at 9:05 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, May 29, 2020 at 07:22:43PM +0800, Tzung-Bi Shih wrote:
> > On Fri, May 29, 2020 at 7:09 PM Mark Brown <broonie@kernel.org> wrote:
>
> > > What is DMIC one wire mode? This doesn't sound like something I'd
> > > expect to vary at runtime.
>
> > It means: 1 PDM data wire carries 2 channel data (rising edge for left
> > and falling edge for right).
>
> I thought that was normal for DMICs - is this selecting between left and
> right or something?
Not sure what is the common name but use the same context here.
MT6358 accepts up to 2 PDM wires for 2 DMICs.
If one wire mode is on, MT6358 only accepts 1 PDM wire.
If one wire mode is off, MT6358 merges L/R from 2 PDM wires into 1
I2S-like to SoC.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ASoC: mediatek: mt6358: support DMIC one-wire mode
From: Tzung-Bi Shih @ 2020-05-29 17:26 UTC (permalink / raw)
To: Mark Brown
Cc: Hariprasad Kelam, Linux Kernel Mailing List, howie.huang,
Takashi Iwai, ALSA development, Jiaxin Yu, Liam Girdwood,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <20200529130539.GK4610@sirena.org.uk>
On Fri, May 29, 2020 at 9:05 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, May 29, 2020 at 07:22:43PM +0800, Tzung-Bi Shih wrote:
> > On Fri, May 29, 2020 at 7:09 PM Mark Brown <broonie@kernel.org> wrote:
>
> > > What is DMIC one wire mode? This doesn't sound like something I'd
> > > expect to vary at runtime.
>
> > It means: 1 PDM data wire carries 2 channel data (rising edge for left
> > and falling edge for right).
>
> I thought that was normal for DMICs - is this selecting between left and
> right or something?
Not sure what is the common name but use the same context here.
MT6358 accepts up to 2 PDM wires for 2 DMICs.
If one wire mode is on, MT6358 only accepts 1 PDM wire.
If one wire mode is off, MT6358 merges L/R from 2 PDM wires into 1
I2S-like to SoC.
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] ASoC: mediatek: mt6358: support DMIC one-wire mode
From: Tzung-Bi Shih @ 2020-05-29 17:26 UTC (permalink / raw)
To: Mark Brown
Cc: Hariprasad Kelam, Linux Kernel Mailing List, howie.huang,
Takashi Iwai, ALSA development, Jiaxin Yu, Liam Girdwood,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <20200529130539.GK4610@sirena.org.uk>
On Fri, May 29, 2020 at 9:05 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, May 29, 2020 at 07:22:43PM +0800, Tzung-Bi Shih wrote:
> > On Fri, May 29, 2020 at 7:09 PM Mark Brown <broonie@kernel.org> wrote:
>
> > > What is DMIC one wire mode? This doesn't sound like something I'd
> > > expect to vary at runtime.
>
> > It means: 1 PDM data wire carries 2 channel data (rising edge for left
> > and falling edge for right).
>
> I thought that was normal for DMICs - is this selecting between left and
> right or something?
Not sure what is the common name but use the same context here.
MT6358 accepts up to 2 PDM wires for 2 DMICs.
If one wire mode is on, MT6358 only accepts 1 PDM wire.
If one wire mode is off, MT6358 merges L/R from 2 PDM wires into 1
I2S-like to SoC.
^ permalink raw reply
* Re: [PATCH] Enable zynq gpio and xilinx gpio support for the Zynq MP platform.
From: Greg Gallagher @ 2020-05-29 17:27 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Xenomai@xenomai.org, Joshua Karch
In-Reply-To: <530a8c03-8367-6adb-800a-a7a63d60b795@siemens.com>
On Fri, May 29, 2020 at 1:04 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 29.05.20 06:12, Greg Gallagher via Xenomai wrote:
> > Signed-off-by: Greg Gallagher <greg@embeddedgreg.com>
> > Signed-off-by: Joshua Karch <jkarch48@hotmail.com>
>
> Did you mean "Reported-by: Joshua" or should "From:" be rather Josh?
>
> Jan
Good catch, I based this off of code that was sent via email, so it
should be from Josh signed off by me. I'll fix that.
thanks
Greg
>
> > ---
> > kernel/drivers/gpio/Kconfig | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/drivers/gpio/Kconfig b/kernel/drivers/gpio/Kconfig
> > index dbc862754..44f41688c 100644
> > --- a/kernel/drivers/gpio/Kconfig
> > +++ b/kernel/drivers/gpio/Kconfig
> > @@ -34,7 +34,7 @@ config XENO_DRIVERS_GPIO_SUN8I_H3
> > SoC, as found on the NanoPI boards.
> >
> > config XENO_DRIVERS_GPIO_ZYNQ7000
> > - depends on ARCH_ZYNQ
> > + depends on ARCH_ZYNQ || ARCH_ZYNQMP
> > tristate "Support for Zynq7000 GPIOs"
> > help
> >
> > @@ -42,7 +42,7 @@ config XENO_DRIVERS_GPIO_ZYNQ7000
> > Xilinx's Zynq7000 SoC.
> >
> > config XENO_DRIVERS_GPIO_XILINX
> > - depends on ARCH_ZYNQ
> > + depends on ARCH_ZYNQ || ARCH_ZYNQMP
> > tristate "Support for Xilinx GPIOs"
> > help
> >
> >
>
> --
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE
> Corporate Competence Center Embedded Linux
^ permalink raw reply
* [linux-next:master 2565/14131] drivers/ntb/hw/intel/ntb_hw_gen4.c:449:5: warning: no previous prototype for function 'intel_ntb4_link_disable'
From: kbuild test robot @ 2020-05-29 17:28 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2667 bytes --]
Hi Dave,
FYI, the error/warning still remains.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head: e7b08814b16b80a0bf76eeca16317f8c2ed23b8c
commit: 26bfe3d0b227ab6d38692640b44ce48f2d857602 [2565/14131] ntb: intel: Add Icelake (gen4) support for Intel NTB
config: x86_64-randconfig-a012-20200529 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 2d068e534f1671459e1b135852c1b3c10502e929)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
git checkout 26bfe3d0b227ab6d38692640b44ce48f2d857602
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> drivers/ntb/hw/intel/ntb_hw_gen4.c:449:5: warning: no previous prototype for function 'intel_ntb4_link_disable' [-Wmissing-prototypes]
int intel_ntb4_link_disable(struct ntb_dev *ntb)
^
drivers/ntb/hw/intel/ntb_hw_gen4.c:449:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int intel_ntb4_link_disable(struct ntb_dev *ntb)
^
static
1 warning generated.
vim +/intel_ntb4_link_disable +449 drivers/ntb/hw/intel/ntb_hw_gen4.c
448
> 449 int intel_ntb4_link_disable(struct ntb_dev *ntb)
450 {
451 struct intel_ntb_dev *ndev;
452 u32 ntb_cntl;
453 u16 lnkctl;
454
455 ndev = container_of(ntb, struct intel_ntb_dev, ntb);
456
457 dev_dbg(&ntb->pdev->dev, "Disabling link\n");
458
459 /* clear the snoop bits */
460 ntb_cntl = ioread32(ndev->self_mmio + ndev->reg->ntb_ctl);
461 ntb_cntl &= ~(NTB_CTL_E2I_BAR23_SNOOP | NTB_CTL_I2E_BAR23_SNOOP);
462 ntb_cntl &= ~(NTB_CTL_E2I_BAR45_SNOOP | NTB_CTL_I2E_BAR45_SNOOP);
463 iowrite32(ntb_cntl, ndev->self_mmio + ndev->reg->ntb_ctl);
464
465 lnkctl = ioread16(ndev->self_mmio + GEN4_LINK_CTRL_OFFSET);
466 lnkctl |= GEN4_LINK_CTRL_LINK_DISABLE;
467 iowrite16(lnkctl, ndev->self_mmio + GEN4_LINK_CTRL_OFFSET);
468
469 ndev->dev_up = 0;
470
471 return 0;
472 }
473
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 42117 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] migration/rdma: cleanup rdma context before g_free to avoid memleaks
From: Dr. David Alan Gilbert @ 2020-05-29 17:27 UTC (permalink / raw)
To: Pan Nengyuan; +Cc: zhang.zhanghailiang, euler.robot, qemu-devel, quintela
In-Reply-To: <20200508100755.7875-3-pannengyuan@huawei.com>
* Pan Nengyuan (pannengyuan@huawei.com) wrote:
> When error happen in initializing 'rdma_return_path', we should cleanup rdma context
> before g_free(rdma) to avoid some memleaks. This patch fix that.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Queued.
> ---
> migration/rdma.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 72e8b1c95b..ec45d33ba3 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -4094,20 +4094,20 @@ void rdma_start_outgoing_migration(void *opaque,
> rdma_return_path = qemu_rdma_data_init(host_port, errp);
>
> if (rdma_return_path == NULL) {
> - goto err;
> + goto return_path_err;
> }
>
> ret = qemu_rdma_source_init(rdma_return_path,
> s->enabled_capabilities[MIGRATION_CAPABILITY_RDMA_PIN_ALL], errp);
>
> if (ret) {
> - goto err;
> + goto return_path_err;
> }
>
> ret = qemu_rdma_connect(rdma_return_path, errp);
>
> if (ret) {
> - goto err;
> + goto return_path_err;
> }
>
> rdma->return_path = rdma_return_path;
> @@ -4120,6 +4120,8 @@ void rdma_start_outgoing_migration(void *opaque,
> s->to_dst_file = qemu_fopen_rdma(rdma, "wb");
> migrate_fd_connect(s, NULL);
> return;
> +return_path_err:
> + qemu_rdma_cleanup(rdma);
> err:
> g_free(rdma);
> g_free(rdma_return_path);
> --
> 2.18.2
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply
* [PATCH v2] hwmon:(adm1275) Enable adm1278 ADM1278_TEMP1_EN
From: Guenter Roeck @ 2020-05-29 17:30 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20200529124607.GA3469@cnn>
On 5/29/20 5:46 AM, Manikandan Elumalai wrote:
> The adm1278 temperature sysfs attribute need it for one of the openbmc platform .
> This functionality is not enabled by default, so PMON_CONFIG needs to be modified in order to enable it.
>
> Signed-off-by : Manikandan Elumalai <manikandan.hcl.ers.epl@gmail.com>
This is not valid.
>
> v2:
> - Add Signed-off-by.
> - Removed ADM1278_TEMP1_EN check.
checkpatch reports:
> ---WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#14:
The adm1278 temperature sysfs attribute need it for one of the openbmc platform .
CHECK: Alignment should match open parenthesis
#45: FILE: drivers/hwmon/pmbus/adm1275.c:679:
+ ret = i2c_smbus_write_byte_data(client,
+ ADM1275_PMON_CONFIG,
WARNING: suspect code indent for conditional statements (16, 16)
#47: FILE: drivers/hwmon/pmbus/adm1275.c:681:
+ if (ret < 0) {
+ dev_err(&client->dev,
ERROR: Missing Signed-off-by: line(s)
total: 1 errors, 2 warnings, 1 checks, 33 lines checked
Please follow published guidelines when submitting patches.
> drivers/hwmon/pmbus/adm1275.c | 21 +++++++++++++++++----
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
> index 5caa37fb..ab5fceb 100644
> --- a/drivers/hwmon/pmbus/adm1275.c
> +++ b/drivers/hwmon/pmbus/adm1275.c
> @@ -666,7 +666,23 @@ static int adm1275_probe(struct i2c_client *client,
> tindex = 3;
>
> info->func[0] |= PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT |
> - PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
> + PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
> + PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
> +
> + config = i2c_smbus_read_byte_data(client, ADM1275_PMON_CONFIG);
> + if (config < 0)
> + return config;
> +
> + /* Enable TEMP1 by default */
> + config |= ADM1278_TEMP1_EN;
> + ret = i2c_smbus_write_byte_data(client,
> + ADM1275_PMON_CONFIG,
> + config);
> + if (ret < 0) {
> + dev_err(&client->dev,
> + "Failed to enable temperature config\n");
> + return -ENODEV;
> + }
This can be handled in a single operation, together with ADM1278_VOUT_EN
below. There is no need for two separate write operations.
Guenter
>
> /* Enable VOUT if not enabled (it is disabled by default) */
> if (!(config & ADM1278_VOUT_EN)) {
> @@ -681,9 +697,6 @@ static int adm1275_probe(struct i2c_client *client,
> }
> }
>
> - if (config & ADM1278_TEMP1_EN)
> - info->func[0] |=
> - PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
> if (config & ADM1278_VIN_EN)
> info->func[0] |= PMBUS_HAVE_VIN;
> break;
>
^ permalink raw reply
* Re: [PATCH v14 1/1] perf tools: add support for libpfm4
From: Arnaldo Carvalho de Melo @ 2020-05-29 17:23 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Yonghong Song, Andrii Nakryiko,
Greg Kroah-Hartman, Thomas Gleixner, Igor Lubashev,
Alexey Budankov, Florian Fainelli, Adrian Hunter, Andi Kleen,
Jiwei Sun, yuzhoujian, Kan Liang, Jin Yao, Leo Yan, John Garry,
LKML, Networking, bpf, linux-perf-users, Stephane Eranian
In-Reply-To: <CAP-5=fWn1=DtZyfGtYEFd=-zDY1O+9A1fcG_3bDKsuoQDZ4i=Q@mail.gmail.com>
Em Fri, May 29, 2020 at 10:03:51AM -0700, Ian Rogers escreveu:
> On Tue, May 5, 2020 at 11:29 AM Ian Rogers <irogers@google.com> wrote:
> >
> > From: Stephane Eranian <eranian@google.com>
> >
> > This patch links perf with the libpfm4 library if it is available
> > and LIBPFM4 is passed to the build. The libpfm4 library
> > contains hardware event tables for all processors supported by
> > perf_events. It is a helper library that helps convert from a
> > symbolic event name to the event encoding required by the
> > underlying kernel interface. This library is open-source and
> > available from: http://perfmon2.sf.net.
> >
> > With this patch, it is possible to specify full hardware events
> > by name. Hardware filters are also supported. Events must be
> > specified via the --pfm-events and not -e option. Both options
> > are active at the same time and it is possible to mix and match:
> >
> > $ perf stat --pfm-events inst_retired:any_p:c=1:i -e cycles ....
> >
> > Signed-off-by: Stephane Eranian <eranian@google.com>
> > Reviewed-by: Ian Rogers <irogers@google.com>
>
> Ping.
Check my tmp.perf/core branch, I had to make some adjustments, mostly in
the 'perf test' entries as I merged a java demangle test that touched
the same files,
I'm now doing the build tests.
- Arnaldo
> Thanks,
> Ian
>
> > ---
> > tools/perf/Documentation/perf-record.txt | 11 +
> > tools/perf/Documentation/perf-stat.txt | 10 +
> > tools/perf/Documentation/perf-top.txt | 11 +
> > tools/perf/Makefile.config | 13 ++
> > tools/perf/Makefile.perf | 2 +
> > tools/perf/builtin-record.c | 6 +
> > tools/perf/builtin-stat.c | 6 +
> > tools/perf/builtin-top.c | 6 +
> > tools/perf/tests/Build | 1 +
> > tools/perf/tests/builtin-test.c | 9 +
> > tools/perf/tests/pfm.c | 203 ++++++++++++++++
> > tools/perf/tests/tests.h | 3 +
> > tools/perf/util/Build | 2 +
> > tools/perf/util/evsel.c | 2 +-
> > tools/perf/util/evsel.h | 1 +
> > tools/perf/util/parse-events.c | 30 ++-
> > tools/perf/util/parse-events.h | 4 +
> > tools/perf/util/pfm.c | 281 +++++++++++++++++++++++
> > tools/perf/util/pfm.h | 37 +++
> > 19 files changed, 630 insertions(+), 8 deletions(-)
> > create mode 100644 tools/perf/tests/pfm.c
> > create mode 100644 tools/perf/util/pfm.c
> > create mode 100644 tools/perf/util/pfm.h
> >
> > diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> > index 561ef55743e2..492b6b6f2b77 100644
> > --- a/tools/perf/Documentation/perf-record.txt
> > +++ b/tools/perf/Documentation/perf-record.txt
> > @@ -613,6 +613,17 @@ appended unit character - B/K/M/G
> > The number of threads to run when synthesizing events for existing processes.
> > By default, the number of threads equals 1.
> >
> > +ifdef::HAVE_LIBPFM[]
> > +--pfm-events events::
> > +Select a PMU event using libpfm4 syntax (see http://perfmon2.sf.net)
> > +including support for event filters. For example '--pfm-events
> > +inst_retired:any_p:u:c=1:i'. More than one event can be passed to the
> > +option using the comma separator. Hardware events and generic hardware
> > +events cannot be mixed together. The latter must be used with the -e
> > +option. The -e option and this one can be mixed and matched. Events
> > +can be grouped using the {} notation.
> > +endif::HAVE_LIBPFM[]
> > +
> > SEE ALSO
> > --------
> > linkperf:perf-stat[1], linkperf:perf-list[1], linkperf:perf-intel-pt[1]
> > diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
> > index 3fb5028aef08..b69af18dccd0 100644
> > --- a/tools/perf/Documentation/perf-stat.txt
> > +++ b/tools/perf/Documentation/perf-stat.txt
> > @@ -71,6 +71,16 @@ report::
> > --tid=<tid>::
> > stat events on existing thread id (comma separated list)
> >
> > +ifdef::HAVE_LIBPFM[]
> > +--pfm-events events::
> > +Select a PMU event using libpfm4 syntax (see http://perfmon2.sf.net)
> > +including support for event filters. For example '--pfm-events
> > +inst_retired:any_p:u:c=1:i'. More than one event can be passed to the
> > +option using the comma separator. Hardware events and generic hardware
> > +events cannot be mixed together. The latter must be used with the -e
> > +option. The -e option and this one can be mixed and matched. Events
> > +can be grouped using the {} notation.
> > +endif::HAVE_LIBPFM[]
> >
> > -a::
> > --all-cpus::
> > diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
> > index 20227dabc208..ee2024691d46 100644
> > --- a/tools/perf/Documentation/perf-top.txt
> > +++ b/tools/perf/Documentation/perf-top.txt
> > @@ -329,6 +329,17 @@ Default is to monitor all CPUS.
> > The known limitations include exception handing such as
> > setjmp/longjmp will have calls/returns not match.
> >
> > +ifdef::HAVE_LIBPFM[]
> > +--pfm-events events::
> > +Select a PMU event using libpfm4 syntax (see http://perfmon2.sf.net)
> > +including support for event filters. For example '--pfm-events
> > +inst_retired:any_p:u:c=1:i'. More than one event can be passed to the
> > +option using the comma separator. Hardware events and generic hardware
> > +events cannot be mixed together. The latter must be used with the -e
> > +option. The -e option and this one can be mixed and matched. Events
> > +can be grouped using the {} notation.
> > +endif::HAVE_LIBPFM[]
> > +
> > INTERACTIVE PROMPTING KEYS
> > --------------------------
> >
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index 12a8204d63c6..b67804fee1e3 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -1012,6 +1012,19 @@ ifdef LIBCLANGLLVM
> > endif
> > endif
> >
> > +ifdef LIBPFM4
> > + $(call feature_check,libpfm4)
> > + ifeq ($(feature-libpfm4), 1)
> > + CFLAGS += -DHAVE_LIBPFM
> > + EXTLIBS += -lpfm
> > + ASCIIDOC_EXTRA = -aHAVE_LIBPFM=1
> > + $(call detected,CONFIG_LIBPFM4)
> > + else
> > + msg := $(warning libpfm4 not found, disables libpfm4 support. Please install libpfm4-dev);
> > + NO_LIBPFM4 := 1
> > + endif
> > +endif
> > +
> > # Among the variables below, these:
> > # perfexecdir
> > # perf_include_dir
> > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > index 94a495594e99..dc82578c8773 100644
> > --- a/tools/perf/Makefile.perf
> > +++ b/tools/perf/Makefile.perf
> > @@ -118,6 +118,8 @@ include ../scripts/utilities.mak
> > #
> > # Define LIBBPF_DYNAMIC to enable libbpf dynamic linking.
> > #
> > +# Define LIBPFM4 to enable libpfm4 events extension.
> > +#
> >
> > # As per kernel Makefile, avoid funny character set dependencies
> > unexport LC_ALL
> > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > index e4efdbf1a81e..98387cce3207 100644
> > --- a/tools/perf/builtin-record.c
> > +++ b/tools/perf/builtin-record.c
> > @@ -45,6 +45,7 @@
> > #include "util/units.h"
> > #include "util/bpf-event.h"
> > #include "util/util.h"
> > +#include "util/pfm.h"
> > #include "asm/bug.h"
> > #include "perf.h"
> >
> > @@ -2506,6 +2507,11 @@ static struct option __record_options[] = {
> > OPT_UINTEGER(0, "num-thread-synthesize",
> > &record.opts.nr_threads_synthesize,
> > "number of threads to run for event synthesis"),
> > +#ifdef HAVE_LIBPFM
> > + OPT_CALLBACK(0, "pfm-events", &record.evlist, "event",
> > + "libpfm4 event selector. use 'perf list' to list available events",
> > + parse_libpfm_events_option),
> > +#endif
> > OPT_END()
> > };
> >
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index e0c1ad23c768..f6e2dd57f48e 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -66,6 +66,7 @@
> > #include "util/time-utils.h"
> > #include "util/top.h"
> > #include "util/affinity.h"
> > +#include "util/pfm.h"
> > #include "asm/bug.h"
> >
> > #include <linux/time64.h>
> > @@ -935,6 +936,11 @@ static struct option stat_options[] = {
> > "Use with 'percore' event qualifier to show the event "
> > "counts of one hardware thread by sum up total hardware "
> > "threads of same physical core"),
> > +#ifdef HAVE_LIBPFM
> > + OPT_CALLBACK(0, "pfm-events", &evsel_list, "event",
> > + "libpfm4 event selector. use 'perf list' to list available events",
> > + parse_libpfm_events_option),
> > +#endif
> > OPT_END()
> > };
> >
> > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> > index 372c38254654..20c41d9040ee 100644
> > --- a/tools/perf/builtin-top.c
> > +++ b/tools/perf/builtin-top.c
> > @@ -53,6 +53,7 @@
> >
> > #include "util/debug.h"
> > #include "util/ordered-events.h"
> > +#include "util/pfm.h"
> >
> > #include <assert.h>
> > #include <elf.h>
> > @@ -1575,6 +1576,11 @@ int cmd_top(int argc, const char **argv)
> > "WARNING: should be used on grouped events."),
> > OPT_BOOLEAN(0, "stitch-lbr", &top.stitch_lbr,
> > "Enable LBR callgraph stitching approach"),
> > +#ifdef HAVE_LIBPFM
> > + OPT_CALLBACK(0, "pfm-events", &top.evlist, "event",
> > + "libpfm4 event selector. use 'perf list' to list available events",
> > + parse_libpfm_events_option),
> > +#endif
> > OPTS_EVSWITCH(&top.evswitch),
> > OPT_END()
> > };
> > diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> > index c75557aeef0e..4e74a363b0b0 100644
> > --- a/tools/perf/tests/Build
> > +++ b/tools/perf/tests/Build
> > @@ -57,6 +57,7 @@ perf-y += maps.o
> > perf-y += time-utils-test.o
> > perf-y += genelf.o
> > perf-y += api-io.o
> > +perf-y += pfm.o
> >
> > $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build
> > $(call rule_mkdir)
> > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> > index 3471ec52ea11..57c6f8b31624 100644
> > --- a/tools/perf/tests/builtin-test.c
> > +++ b/tools/perf/tests/builtin-test.c
> > @@ -317,6 +317,15 @@ static struct test generic_tests[] = {
> > .desc = "maps__merge_in",
> > .func = test__maps__merge_in,
> > },
> > + {
> > + .desc = "Test libpfm4 support",
> > + .func = test__pfm,
> > + .subtest = {
> > + .skip_if_fail = true,
> > + .get_nr = test__pfm_subtest_get_nr,
> > + .get_desc = test__pfm_subtest_get_desc,
> > + }
> > + },
> > {
> > .func = NULL,
> > },
> > diff --git a/tools/perf/tests/pfm.c b/tools/perf/tests/pfm.c
> > new file mode 100644
> > index 000000000000..76a53126efdf
> > --- /dev/null
> > +++ b/tools/perf/tests/pfm.c
> > @@ -0,0 +1,203 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Test support for libpfm4 event encodings.
> > + *
> > + * Copyright 2020 Google LLC.
> > + */
> > +#include "tests.h"
> > +#include "util/debug.h"
> > +#include "util/evlist.h"
> > +#include "util/pfm.h"
> > +
> > +#include <linux/kernel.h>
> > +
> > +#ifdef HAVE_LIBPFM
> > +static int test__pfm_events(void);
> > +static int test__pfm_group(void);
> > +#endif
> > +
> > +static const struct {
> > + int (*func)(void);
> > + const char *desc;
> > +} pfm_testcase_table[] = {
> > +#ifdef HAVE_LIBPFM
> > + {
> > + .func = test__pfm_events,
> > + .desc = "test of individual --pfm-events",
> > + },
> > + {
> > + .func = test__pfm_group,
> > + .desc = "test groups of --pfm-events",
> > + },
> > +#endif
> > +};
> > +
> > +#ifdef HAVE_LIBPFM
> > +static int count_pfm_events(struct perf_evlist *evlist)
> > +{
> > + struct perf_evsel *evsel;
> > + int count = 0;
> > +
> > + perf_evlist__for_each_entry(evlist, evsel) {
> > + count++;
> > + }
> > + return count;
> > +}
> > +
> > +static int test__pfm_events(void)
> > +{
> > + struct evlist *evlist;
> > + struct option opt;
> > + size_t i;
> > + const struct {
> > + const char *events;
> > + int nr_events;
> > + } table[] = {
> > + {
> > + .events = "",
> > + .nr_events = 0,
> > + },
> > + {
> > + .events = "instructions",
> > + .nr_events = 1,
> > + },
> > + {
> > + .events = "instructions,cycles",
> > + .nr_events = 2,
> > + },
> > + {
> > + .events = "stereolab",
> > + .nr_events = 0,
> > + },
> > + {
> > + .events = "instructions,instructions",
> > + .nr_events = 2,
> > + },
> > + {
> > + .events = "stereolab,instructions",
> > + .nr_events = 0,
> > + },
> > + {
> > + .events = "instructions,stereolab",
> > + .nr_events = 1,
> > + },
> > + };
> > +
> > + for (i = 0; i < ARRAY_SIZE(table); i++) {
> > + evlist = evlist__new();
> > + if (evlist == NULL)
> > + return -ENOMEM;
> > +
> > + opt.value = evlist;
> > + parse_libpfm_events_option(&opt,
> > + table[i].events,
> > + 0);
> > + TEST_ASSERT_EQUAL(table[i].events,
> > + count_pfm_events(&evlist->core),
> > + table[i].nr_events);
> > + TEST_ASSERT_EQUAL(table[i].events,
> > + evlist->nr_groups,
> > + 0);
> > +
> > + evlist__delete(evlist);
> > + }
> > + return 0;
> > +}
> > +
> > +static int test__pfm_group(void)
> > +{
> > + struct evlist *evlist;
> > + struct option opt;
> > + size_t i;
> > + const struct {
> > + const char *events;
> > + int nr_events;
> > + int nr_groups;
> > + } table[] = {
> > + {
> > + .events = "{},",
> > + .nr_events = 0,
> > + .nr_groups = 0,
> > + },
> > + {
> > + .events = "{instructions}",
> > + .nr_events = 1,
> > + .nr_groups = 1,
> > + },
> > + {
> > + .events = "{instructions},{}",
> > + .nr_events = 1,
> > + .nr_groups = 1,
> > + },
> > + {
> > + .events = "{},{instructions}",
> > + .nr_events = 0,
> > + .nr_groups = 0,
> > + },
> > + {
> > + .events = "{instructions},{instructions}",
> > + .nr_events = 2,
> > + .nr_groups = 2,
> > + },
> > + {
> > + .events = "{instructions,cycles},{instructions,cycles}",
> > + .nr_events = 4,
> > + .nr_groups = 2,
> > + },
> > + {
> > + .events = "{stereolab}",
> > + .nr_events = 0,
> > + .nr_groups = 0,
> > + },
> > + {
> > + .events =
> > + "{instructions,cycles},{instructions,stereolab}",
> > + .nr_events = 3,
> > + .nr_groups = 1,
> > + },
> > + };
> > +
> > + for (i = 0; i < ARRAY_SIZE(table); i++) {
> > + evlist = evlist__new();
> > + if (evlist == NULL)
> > + return -ENOMEM;
> > +
> > + opt.value = evlist;
> > + parse_libpfm_events_option(&opt,
> > + table[i].events,
> > + 0);
> > + TEST_ASSERT_EQUAL(table[i].events,
> > + count_pfm_events(&evlist->core),
> > + table[i].nr_events);
> > + TEST_ASSERT_EQUAL(table[i].events,
> > + evlist->nr_groups,
> > + table[i].nr_groups);
> > +
> > + evlist__delete(evlist);
> > + }
> > + return 0;
> > +}
> > +#endif
> > +
> > +const char *test__pfm_subtest_get_desc(int i)
> > +{
> > + if (i < 0 || i >= (int)ARRAY_SIZE(pfm_testcase_table))
> > + return NULL;
> > + return pfm_testcase_table[i].desc;
> > +}
> > +
> > +int test__pfm_subtest_get_nr(void)
> > +{
> > + return (int)ARRAY_SIZE(pfm_testcase_table);
> > +}
> > +
> > +int test__pfm(struct test *test __maybe_unused, int i __maybe_unused)
> > +{
> > +#ifdef HAVE_LIBPFM
> > + if (i < 0 || i >= (int)ARRAY_SIZE(pfm_testcase_table))
> > + return TEST_FAIL;
> > + return pfm_testcase_table[i].func();
> > +#else
> > + return TEST_SKIP;
> > +#endif
> > +}
> > diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> > index d6d4ac34eeb7..a2ae0d2e6087 100644
> > --- a/tools/perf/tests/tests.h
> > +++ b/tools/perf/tests/tests.h
> > @@ -113,6 +113,9 @@ int test__maps__merge_in(struct test *t, int subtest);
> > int test__time_utils(struct test *t, int subtest);
> > int test__jit_write_elf(struct test *test, int subtest);
> > int test__api_io(struct test *test, int subtest);
> > +int test__pfm(struct test *test, int subtest);
> > +const char *test__pfm_subtest_get_desc(int subtest);
> > +int test__pfm_subtest_get_nr(void);
> >
> > bool test__bp_signal_is_supported(void);
> > bool test__bp_account_is_supported(void);
> > diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> > index ca07a162d602..dfda916f0b4c 100644
> > --- a/tools/perf/util/Build
> > +++ b/tools/perf/util/Build
> > @@ -179,6 +179,8 @@ perf-$(CONFIG_LIBBPF) += bpf-event.o
> >
> > perf-$(CONFIG_CXX) += c++/
> >
> > +perf-$(CONFIG_LIBPFM4) += pfm.o
> > +
> > CFLAGS_config.o += -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
> > CFLAGS_llvm-utils.o += -DPERF_INCLUDE_DIR="BUILD_STR($(perf_include_dir_SQ))"
> >
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index f3e60c45d59a..d7ea1a7e74cd 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -2416,7 +2416,7 @@ bool evsel__fallback(struct evsel *evsel, int err, char *msg, size_t msgsize)
> >
> > /* Is there already the separator in the name. */
> > if (strchr(name, '/') ||
> > - strchr(name, ':'))
> > + (strchr(name, ':') && !evsel->is_libpfm_event))
> > sep = "";
> >
> > if (asprintf(&new_name, "%s%su", name, sep) < 0)
> > diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> > index 351c0aaf2a11..5a4f20ddeb49 100644
> > --- a/tools/perf/util/evsel.h
> > +++ b/tools/perf/util/evsel.h
> > @@ -76,6 +76,7 @@ struct evsel {
> > bool ignore_missing_thread;
> > bool forced_leader;
> > bool use_uncore_alias;
> > + bool is_libpfm_event;
> > /* parse modifier helper */
> > int exclude_GH;
> > int sample_read;
> > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > index b7a0518d607d..0ee3338a0449 100644
> > --- a/tools/perf/util/parse-events.c
> > +++ b/tools/perf/util/parse-events.c
> > @@ -36,6 +36,7 @@
> > #include "metricgroup.h"
> > #include "util/evsel_config.h"
> > #include "util/event.h"
> > +#include "util/pfm.h"
> >
> > #define MAX_NAME_LEN 100
> >
> > @@ -344,6 +345,7 @@ static char *get_config_name(struct list_head *head_terms)
> > static struct evsel *
> > __add_event(struct list_head *list, int *idx,
> > struct perf_event_attr *attr,
> > + bool init_attr,
> > char *name, struct perf_pmu *pmu,
> > struct list_head *config_terms, bool auto_merge_stats,
> > const char *cpu_list)
> > @@ -352,7 +354,8 @@ __add_event(struct list_head *list, int *idx,
> > struct perf_cpu_map *cpus = pmu ? pmu->cpus :
> > cpu_list ? perf_cpu_map__new(cpu_list) : NULL;
> >
> > - event_attr_init(attr);
> > + if (init_attr)
> > + event_attr_init(attr);
> >
> > evsel = perf_evsel__new_idx(attr, *idx);
> > if (!evsel)
> > @@ -370,15 +373,25 @@ __add_event(struct list_head *list, int *idx,
> > if (config_terms)
> > list_splice(config_terms, &evsel->config_terms);
> >
> > - list_add_tail(&evsel->core.node, list);
> > + if (list)
> > + list_add_tail(&evsel->core.node, list);
> > +
> > return evsel;
> > }
> >
> > +struct evsel *parse_events__add_event(int idx, struct perf_event_attr *attr,
> > + char *name, struct perf_pmu *pmu)
> > +{
> > + return __add_event(NULL, &idx, attr, false, name, pmu, NULL, false,
> > + NULL);
> > +}
> > +
> > static int add_event(struct list_head *list, int *idx,
> > struct perf_event_attr *attr, char *name,
> > struct list_head *config_terms)
> > {
> > - return __add_event(list, idx, attr, name, NULL, config_terms, false, NULL) ? 0 : -ENOMEM;
> > + return __add_event(list, idx, attr, true, name, NULL, config_terms,
> > + false, NULL) ? 0 : -ENOMEM;
> > }
> >
> > static int add_event_tool(struct list_head *list, int *idx,
> > @@ -390,7 +403,8 @@ static int add_event_tool(struct list_head *list, int *idx,
> > .config = PERF_COUNT_SW_DUMMY,
> > };
> >
> > - evsel = __add_event(list, idx, &attr, NULL, NULL, NULL, false, "0");
> > + evsel = __add_event(list, idx, &attr, true, NULL, NULL, NULL, false,
> > + "0");
> > if (!evsel)
> > return -ENOMEM;
> > evsel->tool_event = tool_event;
> > @@ -1446,8 +1460,8 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
> >
> > if (!head_config) {
> > attr.type = pmu->type;
> > - evsel = __add_event(list, &parse_state->idx, &attr, NULL, pmu, NULL,
> > - auto_merge_stats, NULL);
> > + evsel = __add_event(list, &parse_state->idx, &attr, true, NULL,
> > + pmu, NULL, auto_merge_stats, NULL);
> > if (evsel) {
> > evsel->pmu_name = name ? strdup(name) : NULL;
> > evsel->use_uncore_alias = use_uncore_alias;
> > @@ -1488,7 +1502,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
> > return -EINVAL;
> > }
> >
> > - evsel = __add_event(list, &parse_state->idx, &attr,
> > + evsel = __add_event(list, &parse_state->idx, &attr, true,
> > get_config_name(head_config), pmu,
> > &config_terms, auto_merge_stats, NULL);
> > if (evsel) {
> > @@ -2817,6 +2831,8 @@ void print_events(const char *event_glob, bool name_only, bool quiet_flag,
> > print_sdt_events(NULL, NULL, name_only);
> >
> > metricgroup__print(true, true, NULL, name_only, details_flag);
> > +
> > + print_libpfm_events(name_only, long_desc);
> > }
> >
> > int parse_events__is_hardcoded_term(struct parse_events_term *term)
> > diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> > index 6ead9661238c..04e3f627c081 100644
> > --- a/tools/perf/util/parse-events.h
> > +++ b/tools/perf/util/parse-events.h
> > @@ -17,6 +17,7 @@ struct evlist;
> > struct parse_events_error;
> >
> > struct option;
> > +struct perf_pmu;
> >
> > struct tracepoint_path {
> > char *system;
> > @@ -187,6 +188,9 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
> > bool auto_merge_stats,
> > bool use_alias);
> >
> > +struct evsel *parse_events__add_event(int idx, struct perf_event_attr *attr,
> > + char *name, struct perf_pmu *pmu);
> > +
> > int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
> > char *str,
> > struct list_head **listp);
> > diff --git a/tools/perf/util/pfm.c b/tools/perf/util/pfm.c
> > new file mode 100644
> > index 000000000000..d735acb6c29c
> > --- /dev/null
> > +++ b/tools/perf/util/pfm.c
> > @@ -0,0 +1,281 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Support for libpfm4 event encoding.
> > + *
> > + * Copyright 2020 Google LLC.
> > + */
> > +#include "util/cpumap.h"
> > +#include "util/debug.h"
> > +#include "util/event.h"
> > +#include "util/evlist.h"
> > +#include "util/evsel.h"
> > +#include "util/parse-events.h"
> > +#include "util/pmu.h"
> > +#include "util/pfm.h"
> > +
> > +#include <string.h>
> > +#include <linux/kernel.h>
> > +#include <perfmon/pfmlib_perf_event.h>
> > +
> > +static void libpfm_initialize(void)
> > +{
> > + int ret;
> > +
> > + ret = pfm_initialize();
> > + if (ret != PFM_SUCCESS) {
> > + ui__warning("libpfm failed to initialize: %s\n",
> > + pfm_strerror(ret));
> > + }
> > +}
> > +
> > +int parse_libpfm_events_option(const struct option *opt, const char *str,
> > + int unset __maybe_unused)
> > +{
> > + struct evlist *evlist = *(struct evlist **)opt->value;
> > + struct perf_event_attr attr;
> > + struct perf_pmu *pmu;
> > + struct evsel *evsel, *grp_leader = NULL;
> > + char *p, *q, *p_orig;
> > + const char *sep;
> > + int grp_evt = -1;
> > + int ret;
> > +
> > + libpfm_initialize();
> > +
> > + p_orig = p = strdup(str);
> > + if (!p)
> > + return -1;
> > + /*
> > + * force loading of the PMU list
> > + */
> > + perf_pmu__scan(NULL);
> > +
> > + for (q = p; strsep(&p, ",{}"); q = p) {
> > + sep = p ? str + (p - p_orig - 1) : "";
> > + if (*sep == '{') {
> > + if (grp_evt > -1) {
> > + ui__error(
> > + "nested event groups not supported\n");
> > + goto error;
> > + }
> > + grp_evt++;
> > + }
> > +
> > + /* no event */
> > + if (*q == '\0')
> > + continue;
> > +
> > + memset(&attr, 0, sizeof(attr));
> > + event_attr_init(&attr);
> > +
> > + ret = pfm_get_perf_event_encoding(q, PFM_PLM0|PFM_PLM3,
> > + &attr, NULL, NULL);
> > +
> > + if (ret != PFM_SUCCESS) {
> > + ui__error("failed to parse event %s : %s\n", str,
> > + pfm_strerror(ret));
> > + goto error;
> > + }
> > +
> > + pmu = perf_pmu__find_by_type((unsigned int)attr.type);
> > + evsel = parse_events__add_event(evlist->core.nr_entries,
> > + &attr, q, pmu);
> > + if (evsel == NULL)
> > + goto error;
> > +
> > + evsel->is_libpfm_event = true;
> > +
> > + evlist__add(evlist, evsel);
> > +
> > + if (grp_evt == 0)
> > + grp_leader = evsel;
> > +
> > + if (grp_evt > -1) {
> > + evsel->leader = grp_leader;
> > + grp_leader->core.nr_members++;
> > + grp_evt++;
> > + }
> > +
> > + if (*sep == '}') {
> > + if (grp_evt < 0) {
> > + ui__error(
> > + "cannot close a non-existing event group\n");
> > + goto error;
> > + }
> > + evlist->nr_groups++;
> > + grp_leader = NULL;
> > + grp_evt = -1;
> > + }
> > + }
> > + return 0;
> > +error:
> > + free(p_orig);
> > + return -1;
> > +}
> > +
> > +static const char *srcs[PFM_ATTR_CTRL_MAX] = {
> > + [PFM_ATTR_CTRL_UNKNOWN] = "???",
> > + [PFM_ATTR_CTRL_PMU] = "PMU",
> > + [PFM_ATTR_CTRL_PERF_EVENT] = "perf_event",
> > +};
> > +
> > +static void
> > +print_attr_flags(pfm_event_attr_info_t *info)
> > +{
> > + int n = 0;
> > +
> > + if (info->is_dfl) {
> > + printf("[default] ");
> > + n++;
> > + }
> > +
> > + if (info->is_precise) {
> > + printf("[precise] ");
> > + n++;
> > + }
> > +
> > + if (!n)
> > + printf("- ");
> > +}
> > +
> > +static void
> > +print_libpfm_events_detailed(pfm_event_info_t *info, bool long_desc)
> > +{
> > + pfm_event_attr_info_t ainfo;
> > + const char *src;
> > + int j, ret;
> > +
> > + ainfo.size = sizeof(ainfo);
> > +
> > + printf(" %s\n", info->name);
> > + printf(" [%s]\n", info->desc);
> > + if (long_desc) {
> > + if (info->equiv)
> > + printf(" Equiv: %s\n", info->equiv);
> > +
> > + printf(" Code : 0x%"PRIx64"\n", info->code);
> > + }
> > + pfm_for_each_event_attr(j, info) {
> > + ret = pfm_get_event_attr_info(info->idx, j,
> > + PFM_OS_PERF_EVENT_EXT, &ainfo);
> > + if (ret != PFM_SUCCESS)
> > + continue;
> > +
> > + if (ainfo.type == PFM_ATTR_UMASK) {
> > + printf(" %s:%s\n", info->name, ainfo.name);
> > + printf(" [%s]\n", ainfo.desc);
> > + }
> > +
> > + if (!long_desc)
> > + continue;
> > +
> > + if (ainfo.ctrl >= PFM_ATTR_CTRL_MAX)
> > + ainfo.ctrl = PFM_ATTR_CTRL_UNKNOWN;
> > +
> > + src = srcs[ainfo.ctrl];
> > + switch (ainfo.type) {
> > + case PFM_ATTR_UMASK:
> > + printf(" Umask : 0x%02"PRIx64" : %s: ",
> > + ainfo.code, src);
> > + print_attr_flags(&ainfo);
> > + putchar('\n');
> > + break;
> > + case PFM_ATTR_MOD_BOOL:
> > + printf(" Modif : %s: [%s] : %s (boolean)\n", src,
> > + ainfo.name, ainfo.desc);
> > + break;
> > + case PFM_ATTR_MOD_INTEGER:
> > + printf(" Modif : %s: [%s] : %s (integer)\n", src,
> > + ainfo.name, ainfo.desc);
> > + break;
> > + case PFM_ATTR_NONE:
> > + case PFM_ATTR_RAW_UMASK:
> > + case PFM_ATTR_MAX:
> > + default:
> > + printf(" Attr : %s: [%s] : %s\n", src,
> > + ainfo.name, ainfo.desc);
> > + }
> > + }
> > +}
> > +
> > +/*
> > + * list all pmu::event:umask, pmu::event
> > + * printed events may not be all valid combinations of umask for an event
> > + */
> > +static void
> > +print_libpfm_events_raw(pfm_pmu_info_t *pinfo, pfm_event_info_t *info)
> > +{
> > + pfm_event_attr_info_t ainfo;
> > + int j, ret;
> > + bool has_umask = false;
> > +
> > + ainfo.size = sizeof(ainfo);
> > +
> > + pfm_for_each_event_attr(j, info) {
> > + ret = pfm_get_event_attr_info(info->idx, j,
> > + PFM_OS_PERF_EVENT_EXT, &ainfo);
> > + if (ret != PFM_SUCCESS)
> > + continue;
> > +
> > + if (ainfo.type != PFM_ATTR_UMASK)
> > + continue;
> > +
> > + printf("%s::%s:%s\n", pinfo->name, info->name, ainfo.name);
> > + has_umask = true;
> > + }
> > + if (!has_umask)
> > + printf("%s::%s\n", pinfo->name, info->name);
> > +}
> > +
> > +void print_libpfm_events(bool name_only, bool long_desc)
> > +{
> > + pfm_event_info_t info;
> > + pfm_pmu_info_t pinfo;
> > + int i, p, ret;
> > +
> > + libpfm_initialize();
> > +
> > + /* initialize to zero to indicate ABI version */
> > + info.size = sizeof(info);
> > + pinfo.size = sizeof(pinfo);
> > +
> > + if (!name_only)
> > + puts("\nList of pre-defined events (to be used in --pfm-events):\n");
> > +
> > + pfm_for_all_pmus(p) {
> > + bool printed_pmu = false;
> > +
> > + ret = pfm_get_pmu_info(p, &pinfo);
> > + if (ret != PFM_SUCCESS)
> > + continue;
> > +
> > + /* only print events that are supported by host HW */
> > + if (!pinfo.is_present)
> > + continue;
> > +
> > + /* handled by perf directly */
> > + if (pinfo.pmu == PFM_PMU_PERF_EVENT)
> > + continue;
> > +
> > + for (i = pinfo.first_event; i != -1;
> > + i = pfm_get_event_next(i)) {
> > +
> > + ret = pfm_get_event_info(i, PFM_OS_PERF_EVENT_EXT,
> > + &info);
> > + if (ret != PFM_SUCCESS)
> > + continue;
> > +
> > + if (!name_only && !printed_pmu) {
> > + printf("%s:\n", pinfo.name);
> > + printed_pmu = true;
> > + }
> > +
> > + if (!name_only)
> > + print_libpfm_events_detailed(&info, long_desc);
> > + else
> > + print_libpfm_events_raw(&pinfo, &info);
> > + }
> > + if (!name_only && printed_pmu)
> > + putchar('\n');
> > + }
> > +}
> > diff --git a/tools/perf/util/pfm.h b/tools/perf/util/pfm.h
> > new file mode 100644
> > index 000000000000..7d70dda87012
> > --- /dev/null
> > +++ b/tools/perf/util/pfm.h
> > @@ -0,0 +1,37 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Support for libpfm4 event encoding.
> > + *
> > + * Copyright 2020 Google LLC.
> > + */
> > +#ifndef __PERF_PFM_H
> > +#define __PERF_PFM_H
> > +
> > +#include <subcmd/parse-options.h>
> > +
> > +#ifdef HAVE_LIBPFM
> > +int parse_libpfm_events_option(const struct option *opt, const char *str,
> > + int unset);
> > +
> > +void print_libpfm_events(bool name_only, bool long_desc);
> > +
> > +#else
> > +#include <linux/compiler.h>
> > +
> > +static inline int parse_libpfm_events_option(
> > + const struct option *opt __maybe_unused,
> > + const char *str __maybe_unused,
> > + int unset __maybe_unused)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline void print_libpfm_events(bool name_only __maybe_unused,
> > + bool long_desc __maybe_unused)
> > +{
> > +}
> > +
> > +#endif
> > +
> > +
> > +#endif /* __PERF_PFM_H */
> > --
> > 2.26.2.526.g744177e7f7-goog
> >
--
- Arnaldo
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.