* [Buildroot] [PATCH] pcm-tools: new package
2018-01-23 14:03 ` [Buildroot] [PATCH] pcm-tools: new package Carlos Santos
@ 2018-03-31 14:57 ` Romain Naour
2018-04-01 14:16 ` Thomas Petazzoni
2018-12-07 1:20 ` [Buildroot] [PATCH v2] " Carlos Santos
2 siblings, 0 replies; 10+ messages in thread
From: Romain Naour @ 2018-03-31 14:57 UTC (permalink / raw)
To: buildroot
Hi Carlos,
Le 23/01/2018 ? 15:03, Carlos Santos a ?crit?:
> Processor Counter Monitor (PCM) is an application programming interface
> (API) and a set of tools based on the API to monitor performance and
> energy metrics of Intel(R) Core(TM), Xeon(R), Atom(TM) and Xeon Phi(TM)
> processors.
>
> The package pulls a patch from upstream that prevents output truncation.
>
> Two Buildroot-specific changes make it use the pci.ids file provided by
> the hwdata pacakge and look for executable files at /usr/bin.
>
> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> ---
> package/Config.in | 1 +
> ...-snprintf-outut-size-larger-than-destinat.patch | 40 ++++++++++++++++++++++
> ...0002-Look-for-pci.ids-at-usr-share-hwdata.patch | 34 ++++++++++++++++++
> .../0003-Look-for-pcm-core-at-usr-bin.patch | 28 +++++++++++++++
> package/pcm-tools/Config.in | 33 ++++++++++++++++++
> package/pcm-tools/pcm-tools.hash | 3 ++
> package/pcm-tools/pcm-tools.mk | 31 +++++++++++++++++
> 7 files changed, 170 insertions(+)
> create mode 100644 package/pcm-tools/0001-Appease-GCC-snprintf-outut-size-larger-than-destinat.patch
> create mode 100644 package/pcm-tools/0002-Look-for-pci.ids-at-usr-share-hwdata.patch
> create mode 100644 package/pcm-tools/0003-Look-for-pcm-core-at-usr-bin.patch
Patches should be generated with git format-patch -N.
./utils/check-package package/pcm-tools/*
package/pcm-tools/0001-Appease-GCC-snprintf-outut-size-larger-than-destinat.patch:4:
generate your patches with 'git format-patch -N'
package/pcm-tools/0002-Look-for-pci.ids-at-usr-share-hwdata.patch:4: generate
your patches with 'git format-patch -N'
package/pcm-tools/0003-Look-for-pcm-core-at-usr-bin.patch:4: generate your
patches with 'git format-patch -N'
> create mode 100644 package/pcm-tools/Config.in
> create mode 100644 package/pcm-tools/pcm-tools.hash
> create mode 100644 package/pcm-tools/pcm-tools.mk
>
> diff --git a/package/Config.in b/package/Config.in
> index ca2a4107b1..3b359348b2 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -113,6 +113,7 @@ menu "Debugging, profiling and benchmark"
> source "package/nmon/Config.in"
> source "package/oprofile/Config.in"
> source "package/pax-utils/Config.in"
> + source "package/pcm-tools/Config.in"
> source "package/pv/Config.in"
> source "package/racehound/Config.in"
> source "package/ramsmp/Config.in"
> diff --git a/package/pcm-tools/0001-Appease-GCC-snprintf-outut-size-larger-than-destinat.patch b/package/pcm-tools/0001-Appease-GCC-snprintf-outut-size-larger-than-destinat.patch
> new file mode 100644
> index 0000000000..5d77ebfcb6
> --- /dev/null
> +++ b/package/pcm-tools/0001-Appease-GCC-snprintf-outut-size-larger-than-destinat.patch
> @@ -0,0 +1,40 @@
> +From d07d6602ac2d4c90f7e448bdc5b780e8aa9faf7c Mon Sep 17 00:00:00 2001
> +From: Carlos Santos <casantos@datacom.ind.br>
> +Date: Sun, 21 Jan 2018 23:44:46 -0200
> +Subject: [PATCH 1/3] Appease GCC: snprintf outut size larger than destination
> + size
> +
> +GCC ignores that PCI function numbers range from 0 to 7, generations are
> +up to 4 (5, by 2019 Q2) and link speeds are up to 16x. So it warns that
> +snprintf output may be truncated in build_pci_header (in pcm-iio.cpp).
> +
> +Solve this by increasing the buffer sizes, since the additional three
> +bytes are harmless. We could satisfy GCC by masking p.bdf.funcno and
> +p.link_speed to 0x07, thus limitting the output to one decimal digit,
> +and p.link_width to 0x1f (two decimal digits). This approach, however,
> +would hide invalid arguments, preventing the user from noticing the
> +error.
> +
> +Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> +---
> + pcm-iio.cpp | 4 ++--
> + 1 file changed, 2 insertions(+), 2 deletions(-)
> +
> +diff --git a/pcm-iio.cpp b/pcm-iio.cpp
> +index f336bf3..1904296 100644
> +--- a/pcm-iio.cpp
> ++++ b/pcm-iio.cpp
> +@@ -153,8 +153,8 @@ vector<struct data> prepare_data(const vector<uint64_t> &values, const vector<st
> + string build_pci_header(const PCIDB & pciDB, uint32_t column_width, struct pci p, int part = -1, uint32_t level = 0)
> + {
> + string s = "|";
> +- char bdf_buf[8];
> +- char speed_buf[9];
> ++ char bdf_buf[10];
> ++ char speed_buf[10];
> + char vid_did_buf[10];
> + char device_name_buf[128];
> +
> +--
> +2.14.3
> +
> diff --git a/package/pcm-tools/0002-Look-for-pci.ids-at-usr-share-hwdata.patch b/package/pcm-tools/0002-Look-for-pci.ids-at-usr-share-hwdata.patch
> new file mode 100644
> index 0000000000..4c04589867
> --- /dev/null
> +++ b/package/pcm-tools/0002-Look-for-pci.ids-at-usr-share-hwdata.patch
> @@ -0,0 +1,34 @@
> +From 9213e92ec5aaee0e319b94f9501458e44e7873ba Mon Sep 17 00:00:00 2001
> +From: Carlos Santos <casantos@datacom.ind.br>
> +Date: Tue, 23 Jan 2018 08:27:49 -0200
> +Subject: [PATCH 2/3] Look for pci.ids at /usr/share/hwdata
> +
> +For Buildroot, on which pci.ids is provided by the "hwdata" package.
> +
> +Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> +---
> + lspci.h | 4 ++--
> + 1 file changed, 2 insertions(+), 2 deletions(-)
> +
> +diff --git a/lspci.h b/lspci.h
> +index 29312a7..1d922fc 100644
> +--- a/lspci.h
> ++++ b/lspci.h
> +@@ -179,12 +179,12 @@ void print_pci(struct pci p, const PCIDB & pciDB)
> +
> + void load_PCIDB(PCIDB & pciDB)
> + {
> +- std::ifstream in("pci.ids");
> ++ std::ifstream in("/usr/share/hwdata/pci.ids");
> + std::string line, item;
> +
> + if (!in.is_open())
> + {
> +- std::cerr << "pci.ids file is not available. Download it from https://raw.githubusercontent.com/pciutils/pciids/master/pci.ids " << endl;
> ++ std::cerr << "/usr/share/hwdata/pci.ids file is not available. Ensure that the \"hwdata\" package is properly installed." << endl;
> + return;
> + }
> +
> +--
> +2.14.3
> +
> diff --git a/package/pcm-tools/0003-Look-for-pcm-core-at-usr-bin.patch b/package/pcm-tools/0003-Look-for-pcm-core-at-usr-bin.patch
> new file mode 100644
> index 0000000000..92dc06c197
> --- /dev/null
> +++ b/package/pcm-tools/0003-Look-for-pcm-core-at-usr-bin.patch
> @@ -0,0 +1,28 @@
> +From bef36c901e7f2cca3c21ee057b75f73065830774 Mon Sep 17 00:00:00 2001
> +From: Carlos Santos <casantos@datacom.ind.br>
> +Date: Tue, 23 Jan 2018 10:56:55 -0200
> +Subject: [PATCH 3/3] Look for pcm-core at /usr/bin
> +
> +For Buildroot, on which pcm-core.x ins installed as /usr/bin/pcm-core.
> +
> +Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> +---
> + pmu-query.py | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/pmu-query.py b/pmu-query.py
> +index 4c596c7..71aa1b1 100755
> +--- a/pmu-query.py
> ++++ b/pmu-query.py
> +@@ -43,7 +43,7 @@ if filename == None:
> + elif platform.system() == 'Windows':
> + p = subprocess.Popen(['pcm-core.exe -c'],stdout=subprocess.PIPE,shell=True)
> + else:
> +- p = subprocess.Popen(['./pcm-core.x -c'],stdout=subprocess.PIPE,shell=True)
> ++ p = subprocess.Popen(['/usr/bin/pcm-core -c'],stdout=subprocess.PIPE,shell=True)
Maybe you can just remove "./" instead of hard-code the binary path.
> +
> + (output, err) = p.communicate()
> + p_status = p.wait()
> +--
> +2.14.3
> +
> diff --git a/package/pcm-tools/Config.in b/package/pcm-tools/Config.in
> new file mode 100644
> index 0000000000..a59c41c3e6
> --- /dev/null
> +++ b/package/pcm-tools/Config.in
> @@ -0,0 +1,33 @@
> +config BR2_PACKAGE_PCM_TOOLS
> + bool "pcm-tools"
> + depends on BR2_i386 || BR2_x86_64
> + select BR2_PACKAGE_HWDATA
> + select BR2_PACKAGE_HWDATA_PCI_IDS> + help
> + Processor Counter Monitor (PCM) is an application programming
> + interface (API) and a set of tools based on the API to monitor
> + performance and energy metrics of Intel(R) Core(TM), Xeon(R),
> + Atom(TM) and Xeon Phi(TM) processors.
> +
> + Requires a kernel with X86_MSR enabled.
> +
> + https://github.com/opcm/pcm
> +
> +if BR2_PACKAGE_PCM_TOOLS
> +
> +# The pmu-query script is not compatible with Python 3
> +config BR2_PACKAGE_PCM_TOOLS_PMU_QUERY_REQS
> + bool
> + default y
> + depends on BR2_PACKAGE_PYTHON && BR2_PACKAGE_PYTHON_SSL && BR2_PACKAGE_PYTHON_HASHLIB # urllib2
> + depends on BR2_PACKAGE_CA_CERTIFICATES # https> +
> +config BR2_PACKAGE_PCM_TOOLS_PMU_QUERY
> + bool "install the pmu-query script"
> + default y
> + depends on BR2_PACKAGE_PCM_TOOLS_PMU_QUERY_REQS
> +
BR2_PACKAGE_CA_CERTIFICATES, BR2_PACKAGE_PYTHON_SSL and
BR2_PACKAGE_PYTHON_HASHLIB should be selected here instead.
> +comment "pmu-query needs Python 2.x w/ ssl + hashlib modules, ca-certificates"
> + depends on !BR2_PACKAGE_PCM_TOOLS_PMU_QUERY_REQS
> +
> +endif
> diff --git a/package/pcm-tools/pcm-tools.hash b/package/pcm-tools/pcm-tools.hash
> new file mode 100644
> index 0000000000..7646dfddee
> --- /dev/null
> +++ b/package/pcm-tools/pcm-tools.hash
> @@ -0,0 +1,3 @@
> +# Locally calculated
> +sha256 772c7b6310ba9c89931ff6108cc71fa0cbe20540ab6a53503f5f61594c9f661d pcm-tools-201710.tar.gz
> +sha256 fac73f62c4d665c82622862a2be2b89713e0f480c93e593af2d8ef29a13d814b LICENSE
> diff --git a/package/pcm-tools/pcm-tools.mk b/package/pcm-tools/pcm-tools.mk
> new file mode 100644
> index 0000000000..53c73621c9
> --- /dev/null
> +++ b/package/pcm-tools/pcm-tools.mk
> @@ -0,0 +1,31 @@
> +################################################################################
> +#
> +# pcm-tools
> +#
> +################################################################################
> +
> +PCM_TOOLS_VERSION = 201710
> +PCM_TOOLS_SITE = $(call github,opcm,pcm,$(PCM_TOOLS_VERSION))
> +PCM_TOOLS_LICENSE = BSD-3-Clause
> +PCM_TOOLS_LICENSE_FILES = LICENSE
> +
> +PCM_TOOLS_EXE_FILES = \
> + pcm-core pcm-iio pcm-lspci pcm-memory pcm-msr pcm-numa \
> + pcm-pcie pcm-power pcm-sensor pcm-tsx pcm
> +
> +define PCM_TOOLS_BUILD_CMDS
> + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D) \
> + UNAME=Linux HOST=_LINUX CC="$(TARGET_CC)"
> +endef
> +
> +define PCM_TOOLS_INSTALL_TARGET_CMDS
> + $(Q) set -x -e; \
> + for f in $(PCM_TOOLS_EXE_FILES); do \
> + $(INSTALL) -D -m 755 $(@D)/$$f.x $(TARGET_DIR)/usr/bin/$$f; \
> + done;> + if test "$(BR2_PACKAGE_PCM_TOOLS_PMU_QUERY)" = "y"; then \
> + $(INSTALL) -D -m 755 $(@D)/pmu-query.py $(TARGET_DIR)/usr/bin/pmu-query; \
> + fi
> +endef
This can be done by another define BR2_PACKAGE_PCM_TOOLS_INSTALL_PMU_QUERY
ifeq ($(BR2_PACKAGE_PCM_TOOLS_PMU_QUERY),y)
define BR2_PACKAGE_PCM_TOOLS_INSTALL_PMU_QUERY
$(INSTALL) -D -m 755 $(@D)/pmu-query.py $(TARGET_DIR)/usr/bin/pmu-query;
endef
endif
then
define PCM_TOOLS_INSTALL_TARGET_CMDS
...
$(BR2_PACKAGE_PCM_TOOLS_INSTALL_PMU_QUERY)
endef
Best regards,
Romain
> +
> +$(eval $(generic-package))
>
^ permalink raw reply [flat|nested] 10+ messages in thread* [Buildroot] [PATCH] pcm-tools: new package
2018-01-23 14:03 ` [Buildroot] [PATCH] pcm-tools: new package Carlos Santos
2018-03-31 14:57 ` Romain Naour
@ 2018-04-01 14:16 ` Thomas Petazzoni
2018-04-03 2:02 ` Carlos Santos
2018-12-07 1:20 ` [Buildroot] [PATCH v2] " Carlos Santos
2 siblings, 1 reply; 10+ messages in thread
From: Thomas Petazzoni @ 2018-04-01 14:16 UTC (permalink / raw)
To: buildroot
Hello,
Here are some additional comments, on top of the ones made by Romain.
On Tue, 23 Jan 2018 12:03:24 -0200, Carlos Santos wrote:
> diff --git a/package/pcm-tools/0001-Appease-GCC-snprintf-outut-size-larger-than-destinat.patch b/package/pcm-tools/0001-Appease-GCC-snprintf-outut-size-larger-than-destinat.patch
> new file mode 100644
> index 0000000000..5d77ebfcb6
> --- /dev/null
> +++ b/package/pcm-tools/0001-Appease-GCC-snprintf-outut-size-larger-than-destinat.patch
> @@ -0,0 +1,40 @@
> +From d07d6602ac2d4c90f7e448bdc5b780e8aa9faf7c Mon Sep 17 00:00:00 2001
> +From: Carlos Santos <casantos@datacom.ind.br>
> +Date: Sun, 21 Jan 2018 23:44:46 -0200
> +Subject: [PATCH 1/3] Appease GCC: snprintf outut size larger than destination
> + size
> +
> +GCC ignores that PCI function numbers range from 0 to 7, generations are
> +up to 4 (5, by 2019 Q2) and link speeds are up to 16x. So it warns that
> +snprintf output may be truncated in build_pci_header (in pcm-iio.cpp).
> +
> +Solve this by increasing the buffer sizes, since the additional three
> +bytes are harmless. We could satisfy GCC by masking p.bdf.funcno and
> +p.link_speed to 0x07, thus limitting the output to one decimal digit,
> +and p.link_width to 0x1f (two decimal digits). This approach, however,
> +would hide invalid arguments, preventing the user from noticing the
> +error.
> +
> +Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
Have the patches been submitted upstream ?
> diff --git a/package/pcm-tools/0002-Look-for-pci.ids-at-usr-share-hwdata.patch b/package/pcm-tools/0002-Look-for-pci.ids-at-usr-share-hwdata.patch
> new file mode 100644
> index 0000000000..4c04589867
> --- /dev/null
> +++ b/package/pcm-tools/0002-Look-for-pci.ids-at-usr-share-hwdata.patch
> @@ -0,0 +1,34 @@
> +From 9213e92ec5aaee0e319b94f9501458e44e7873ba Mon Sep 17 00:00:00 2001
> +From: Carlos Santos <casantos@datacom.ind.br>
> +Date: Tue, 23 Jan 2018 08:27:49 -0200
> +Subject: [PATCH 2/3] Look for pci.ids at /usr/share/hwdata
> +
> +For Buildroot, on which pci.ids is provided by the "hwdata" package.
It's not really nice to have Buildroot specific patches. Can you find a
solution that is acceptable upstream ? Config option perhaps ?
> diff --git a/package/pcm-tools/0003-Look-for-pcm-core-at-usr-bin.patch b/package/pcm-tools/0003-Look-for-pcm-core-at-usr-bin.patch
> new file mode 100644
> index 0000000000..92dc06c197
> --- /dev/null
> +++ b/package/pcm-tools/0003-Look-for-pcm-core-at-usr-bin.patch
> @@ -0,0 +1,28 @@
> +From bef36c901e7f2cca3c21ee057b75f73065830774 Mon Sep 17 00:00:00 2001
> +From: Carlos Santos <casantos@datacom.ind.br>
> +Date: Tue, 23 Jan 2018 10:56:55 -0200
> +Subject: [PATCH 3/3] Look for pcm-core at /usr/bin
> +
> +For Buildroot, on which pcm-core.x ins installed as /usr/bin/pcm-core.
> +
> +Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> +---
> + pmu-query.py | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/pmu-query.py b/pmu-query.py
> +index 4c596c7..71aa1b1 100755
> +--- a/pmu-query.py
> ++++ b/pmu-query.py
> +@@ -43,7 +43,7 @@ if filename == None:
> + elif platform.system() == 'Windows':
> + p = subprocess.Popen(['pcm-core.exe -c'],stdout=subprocess.PIPE,shell=True)
> + else:
> +- p = subprocess.Popen(['./pcm-core.x -c'],stdout=subprocess.PIPE,shell=True)
> ++ p = subprocess.Popen(['/usr/bin/pcm-core -c'],stdout=subprocess.PIPE,shell=True)
Same here, can we find an upstreamable solution, ideally ?
> +if BR2_PACKAGE_PCM_TOOLS
> +
> +# The pmu-query script is not compatible with Python 3
> +config BR2_PACKAGE_PCM_TOOLS_PMU_QUERY_REQS
> + bool
> + default y
> + depends on BR2_PACKAGE_PYTHON && BR2_PACKAGE_PYTHON_SSL && BR2_PACKAGE_PYTHON_HASHLIB # urllib2
> + depends on BR2_PACKAGE_CA_CERTIFICATES # https
There's really no need for a separate
BR2_PACKAGE_PCM_TOOLS_PMU_QUERY_REQS option, just put the
selects/depends on inside BR2_PACKAGE_PCM_TOOLS_PMU_QUERY.
As Romain said, only "depends on BR2_PACKAGE_PYTHON" should remain a
"depends on", everything else should be a "select".
> +
> +config BR2_PACKAGE_PCM_TOOLS_PMU_QUERY
> + bool "install the pmu-query script"
> + default y
> + depends on BR2_PACKAGE_PCM_TOOLS_PMU_QUERY_REQS
> +
> +comment "pmu-query needs Python 2.x w/ ssl + hashlib modules, ca-certificates"
> + depends on !BR2_PACKAGE_PCM_TOOLS_PMU_QUERY_REQS
Which simplifies that quite a lot.
> +PCM_TOOLS_VERSION = 201710
> +PCM_TOOLS_SITE = $(call github,opcm,pcm,$(PCM_TOOLS_VERSION))
> +PCM_TOOLS_LICENSE = BSD-3-Clause
> +PCM_TOOLS_LICENSE_FILES = LICENSE
> +
> +PCM_TOOLS_EXE_FILES = \
> + pcm-core pcm-iio pcm-lspci pcm-memory pcm-msr pcm-numa \
> + pcm-pcie pcm-power pcm-sensor pcm-tsx pcm
> +
> +define PCM_TOOLS_BUILD_CMDS
> + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D) \
> + UNAME=Linux HOST=_LINUX CC="$(TARGET_CC)"
TARGET_CONFIGURE_OPTS already contains CC="$(TARGET_CC)"
> +define PCM_TOOLS_INSTALL_TARGET_CMDS
> + $(Q) set -x -e; \
> + for f in $(PCM_TOOLS_EXE_FILES); do \
> + $(INSTALL) -D -m 755 $(@D)/$$f.x $(TARGET_DIR)/usr/bin/$$f; \
> + done;
> + if test "$(BR2_PACKAGE_PCM_TOOLS_PMU_QUERY)" = "y"; then \
> + $(INSTALL) -D -m 755 $(@D)/pmu-query.py $(TARGET_DIR)/usr/bin/pmu-query; \
> + fi
> +endef
That's not a very Buildroot-ish way of doing this. Here is a more
Buildroot-ish solution:
ifeq ($(BR2_PACKAGE_PCM_TOOLS_PMU_QUERY),y)
define PCM_TOOLS_INSTALL_PMU_QUERY
$(INSTALL) -D -m 755 $(@D)/pmu-query.py $(TARGET_DIR)/usr/bin/pmu-query
endef
endif
define PCM_TOOLS_INSTALL_TARGET_CMDS
$(foreach f,$(PCM_TOOLS_EXE_FILES),\
$(INSTALL) -D -m 755 $(@D)/$(f).x $(TARGET_DIR)/usr/bin/$(f)
)
$(PCM_TOOLS_INSTALL_PMU_QUERY)
endef
Could you rework your patch to take into Romain's comments and the
above comments ?
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH] pcm-tools: new package
2018-04-01 14:16 ` Thomas Petazzoni
@ 2018-04-03 2:02 ` Carlos Santos
2018-12-07 1:32 ` Carlos Santos
0 siblings, 1 reply; 10+ messages in thread
From: Carlos Santos @ 2018-04-03 2:02 UTC (permalink / raw)
To: buildroot
> From: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: "buildroot" <buildroot@buildroot.org>
> Sent: Sunday, April 1, 2018 11:16:54 AM
> Subject: Re: [Buildroot] [PATCH] pcm-tools: new package
> Hello,
>
> Here are some additional comments, on top of the ones made by Romain.
>
> On Tue, 23 Jan 2018 12:03:24 -0200, Carlos Santos wrote:
>
>> diff --git
>> a/package/pcm-tools/0001-Appease-GCC-snprintf-outut-size-larger-than-destinat.patch
>> b/package/pcm-tools/0001-Appease-GCC-snprintf-outut-size-larger-than-destinat.patch
>> new file mode 100644
>> index 0000000000..5d77ebfcb6
>> --- /dev/null
>> +++
>> b/package/pcm-tools/0001-Appease-GCC-snprintf-outut-size-larger-than-destinat.patch
>> @@ -0,0 +1,40 @@
>> +From d07d6602ac2d4c90f7e448bdc5b780e8aa9faf7c Mon Sep 17 00:00:00 2001
>> +From: Carlos Santos <casantos@datacom.ind.br>
>> +Date: Sun, 21 Jan 2018 23:44:46 -0200
>> +Subject: [PATCH 1/3] Appease GCC: snprintf outut size larger than destination
>> + size
>> +
>> +GCC ignores that PCI function numbers range from 0 to 7, generations are
>> +up to 4 (5, by 2019 Q2) and link speeds are up to 16x. So it warns that
>> +snprintf output may be truncated in build_pci_header (in pcm-iio.cpp).
>> +
>> +Solve this by increasing the buffer sizes, since the additional three
>> +bytes are harmless. We could satisfy GCC by masking p.bdf.funcno and
>> +p.link_speed to 0x07, thus limitting the output to one decimal digit,
>> +and p.link_width to 0x1f (two decimal digits). This approach, however,
>> +would hide invalid arguments, preventing the user from noticing the
>> +error.
>> +
>> +Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
>
> Have the patches been submitted upstream ?
The first patch was already accepted upstream.
>> diff --git a/package/pcm-tools/0002-Look-for-pci.ids-at-usr-share-hwdata.patch
>> b/package/pcm-tools/0002-Look-for-pci.ids-at-usr-share-hwdata.patch
>> new file mode 100644
>> index 0000000000..4c04589867
>> --- /dev/null
>> +++ b/package/pcm-tools/0002-Look-for-pci.ids-at-usr-share-hwdata.patch
>> @@ -0,0 +1,34 @@
>> +From 9213e92ec5aaee0e319b94f9501458e44e7873ba Mon Sep 17 00:00:00 2001
>> +From: Carlos Santos <casantos@datacom.ind.br>
>> +Date: Tue, 23 Jan 2018 08:27:49 -0200
>> +Subject: [PATCH 2/3] Look for pci.ids at /usr/share/hwdata
>> +
>> +For Buildroot, on which pci.ids is provided by the "hwdata" package.
>
> It's not really nice to have Buildroot specific patches. Can you find a
> solution that is acceptable upstream ? Config option perhaps ?
They are necessary to deal with specific path where files are installed
in Buildroot builds. Anyway, I will try to find a way to make them more
generic.
>> diff --git a/package/pcm-tools/0003-Look-for-pcm-core-at-usr-bin.patch
>> b/package/pcm-tools/0003-Look-for-pcm-core-at-usr-bin.patch
>> new file mode 100644
>> index 0000000000..92dc06c197
>> --- /dev/null
>> +++ b/package/pcm-tools/0003-Look-for-pcm-core-at-usr-bin.patch
>> @@ -0,0 +1,28 @@
>> +From bef36c901e7f2cca3c21ee057b75f73065830774 Mon Sep 17 00:00:00 2001
>> +From: Carlos Santos <casantos@datacom.ind.br>
>> +Date: Tue, 23 Jan 2018 10:56:55 -0200
>> +Subject: [PATCH 3/3] Look for pcm-core at /usr/bin
>> +
>> +For Buildroot, on which pcm-core.x ins installed as /usr/bin/pcm-core.
>> +
>> +Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
>> +---
>> + pmu-query.py | 2 +-
>> + 1 file changed, 1 insertion(+), 1 deletion(-)
>> +
>> +diff --git a/pmu-query.py b/pmu-query.py
>> +index 4c596c7..71aa1b1 100755
>> +--- a/pmu-query.py
>> ++++ b/pmu-query.py
>> +@@ -43,7 +43,7 @@ if filename == None:
>> + elif platform.system() == 'Windows':
>> + p = subprocess.Popen(['pcm-core.exe
>> -c'],stdout=subprocess.PIPE,shell=True)
>> + else:
>> +- p = subprocess.Popen(['./pcm-core.x
>> -c'],stdout=subprocess.PIPE,shell=True)
>> ++ p = subprocess.Popen(['/usr/bin/pcm-core
>> -c'],stdout=subprocess.PIPE,shell=True)
>
> Same here, can we find an upstreamable solution, ideally ?
>
>> +if BR2_PACKAGE_PCM_TOOLS
>> +
>> +# The pmu-query script is not compatible with Python 3
>> +config BR2_PACKAGE_PCM_TOOLS_PMU_QUERY_REQS
>> + bool
>> + default y
>> + depends on BR2_PACKAGE_PYTHON && BR2_PACKAGE_PYTHON_SSL &&
>> BR2_PACKAGE_PYTHON_HASHLIB # urllib2
>> + depends on BR2_PACKAGE_CA_CERTIFICATES # https
>
> There's really no need for a separate
> BR2_PACKAGE_PCM_TOOLS_PMU_QUERY_REQS option, just put the
> selects/depends on inside BR2_PACKAGE_PCM_TOOLS_PMU_QUERY.
>
> As Romain said, only "depends on BR2_PACKAGE_PYTHON" should remain a
> "depends on", everything else should be a "select".
OK
>> +
>> +config BR2_PACKAGE_PCM_TOOLS_PMU_QUERY
>> + bool "install the pmu-query script"
>> + default y
>> + depends on BR2_PACKAGE_PCM_TOOLS_PMU_QUERY_REQS
>> +
>> +comment "pmu-query needs Python 2.x w/ ssl + hashlib modules, ca-certificates"
>> + depends on !BR2_PACKAGE_PCM_TOOLS_PMU_QUERY_REQS
>
> Which simplifies that quite a lot.
>
>
>> +PCM_TOOLS_VERSION = 201710
>> +PCM_TOOLS_SITE = $(call github,opcm,pcm,$(PCM_TOOLS_VERSION))
>> +PCM_TOOLS_LICENSE = BSD-3-Clause
>> +PCM_TOOLS_LICENSE_FILES = LICENSE
>> +
>> +PCM_TOOLS_EXE_FILES = \
>> + pcm-core pcm-iio pcm-lspci pcm-memory pcm-msr pcm-numa \
>> + pcm-pcie pcm-power pcm-sensor pcm-tsx pcm
>> +
>> +define PCM_TOOLS_BUILD_CMDS
>> + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D) \
>> + UNAME=Linux HOST=_LINUX CC="$(TARGET_CC)"
>
> TARGET_CONFIGURE_OPTS already contains CC="$(TARGET_CC)"
>
>> +define PCM_TOOLS_INSTALL_TARGET_CMDS
>> + $(Q) set -x -e; \
>> + for f in $(PCM_TOOLS_EXE_FILES); do \
>> + $(INSTALL) -D -m 755 $(@D)/$$f.x $(TARGET_DIR)/usr/bin/$$f; \
>> + done;
>> + if test "$(BR2_PACKAGE_PCM_TOOLS_PMU_QUERY)" = "y"; then \
>> + $(INSTALL) -D -m 755 $(@D)/pmu-query.py $(TARGET_DIR)/usr/bin/pmu-query; \
>> + fi
>> +endef
>
> That's not a very Buildroot-ish way of doing this. Here is a more
> Buildroot-ish solution:
>
> ifeq ($(BR2_PACKAGE_PCM_TOOLS_PMU_QUERY),y)
> define PCM_TOOLS_INSTALL_PMU_QUERY
> $(INSTALL) -D -m 755 $(@D)/pmu-query.py $(TARGET_DIR)/usr/bin/pmu-query
> endef
> endif
>
> define PCM_TOOLS_INSTALL_TARGET_CMDS
> $(foreach f,$(PCM_TOOLS_EXE_FILES),\
> $(INSTALL) -D -m 755 $(@D)/$(f).x $(TARGET_DIR)/usr/bin/$(f)
> )
> $(PCM_TOOLS_INSTALL_PMU_QUERY)
> endef
>
> Could you rework your patch to take into Romain's comments and the
> above comments ?
I will submit an updated version of the series. Thanks for your review.
--
Carlos Santos (Casantos) - DATACOM, P&D
?The greatest triumph that modern PR can offer is the transcendent
success of having your words and actions judged by your reputation,
rather than the other way about.? ? Christopher Hitchens
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH] pcm-tools: new package
2018-04-03 2:02 ` Carlos Santos
@ 2018-12-07 1:32 ` Carlos Santos
2018-12-07 11:15 ` Thomas Petazzoni
0 siblings, 1 reply; 10+ messages in thread
From: Carlos Santos @ 2018-12-07 1:32 UTC (permalink / raw)
To: buildroot
Superseded-by: https://patchwork.ozlabs.org/patch/1009149/
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH] pcm-tools: new package
2018-12-07 1:32 ` Carlos Santos
@ 2018-12-07 11:15 ` Thomas Petazzoni
2018-12-07 11:41 ` Carlos Santos
0 siblings, 1 reply; 10+ messages in thread
From: Thomas Petazzoni @ 2018-12-07 11:15 UTC (permalink / raw)
To: buildroot
Hello Carlos,
On Thu, 6 Dec 2018 23:32:15 -0200 (BRST), Carlos Santos wrote:
> Superseded-by: https://patchwork.ozlabs.org/patch/1009149/
I am wondering: is this Superseded-by really understood by patchwork to
make a patch as Superseded? I don't see Superseded-by used in the
patchwork code base.
I see that the patch was marked as Superseded in patchwork, was it done
manually by you, or automatically thanks to this e-mail ?
If it was done manually by you, then I think it's not really useful to
send this Superseded-by e-mail: we see in patchwork the latest version
anyway, which is what is important.
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH] pcm-tools: new package
2018-12-07 11:15 ` Thomas Petazzoni
@ 2018-12-07 11:41 ` Carlos Santos
0 siblings, 0 replies; 10+ messages in thread
From: Carlos Santos @ 2018-12-07 11:41 UTC (permalink / raw)
To: buildroot
----- Original Message -----
> From: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
> To: "Carlos Santos" <casantos@datacom.com.br>
> Cc: "buildroot" <buildroot@buildroot.org>, "Romain Naour" <romain.naour@smile.fr>
> Sent: Sexta-feira, 7 de dezembro de 2018 9:15:59
> Subject: Re: [Buildroot] [PATCH] pcm-tools: new package
> Hello Carlos,
>
> On Thu, 6 Dec 2018 23:32:15 -0200 (BRST), Carlos Santos wrote:
>
>> Superseded-by: https://patchwork.ozlabs.org/patch/1009149/
>
> I am wondering: is this Superseded-by really understood by patchwork to
> make a patch as Superseded? I don't see Superseded-by used in the
> patchwork code base.
>
> I see that the patch was marked as Superseded in patchwork, was it done
> manually by you, or automatically thanks to this e-mail ?
Manually by me.
> If it was done manually by you, then I think it's not really useful to
> send this Superseded-by e-mail: we see in patchwork the latest version
> anyway, which is what is important.
I use them to leave a track of my patches, since patchwork does not help
much (smells like 1980, in fact, even tough it was written much later).
--
Carlos Santos (Casantos) - DATACOM, P&D
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH v2] pcm-tools: new package
2018-01-23 14:03 ` [Buildroot] [PATCH] pcm-tools: new package Carlos Santos
2018-03-31 14:57 ` Romain Naour
2018-04-01 14:16 ` Thomas Petazzoni
@ 2018-12-07 1:20 ` Carlos Santos
2018-12-08 9:43 ` Peter Korsgaard
2 siblings, 1 reply; 10+ messages in thread
From: Carlos Santos @ 2018-12-07 1:20 UTC (permalink / raw)
To: buildroot
Processor Counter Monitor (PCM) is an application programming interface
(API) and a set of tools based on the API to monitor performance and
energy metrics of Intel(R) Core(TM), Xeon(R), Atom(TM) and Xeon Phi(TM)
processors.
This package contains a patch on the pmu-query.py script to look for the
pcm-core program at the default path. It's not nice to have a Buildroot
specific patch but let's use one while we look for a solution that is
acceptable upstream.
Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
Supersedes: https://patchwork.ozlabs.org/patch/864801/
CC: Romain Naour <romain.naour@gmail.com>
CC: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
Changes v1->v2
- Update to version 201812
- Do not build the test daemon
- Drop the patch on the test daemon, that we don't build
- Drop a patch already applied upstream
- Update the pmu-query.py patch
- Make the changes suggested by Romain Naour and Thomas Petazzoni
---
package/Config.in | 1 +
...ook-for-pcm-core-at-the-default-path.patch | 46 +++++++++++++++++++
package/pcm-tools/Config.in | 30 ++++++++++++
package/pcm-tools/pcm-tools.hash | 3 ++
package/pcm-tools/pcm-tools.mk | 35 ++++++++++++++
5 files changed, 115 insertions(+)
create mode 100644 package/pcm-tools/0001-Look-for-pcm-core-at-the-default-path.patch
create mode 100644 package/pcm-tools/Config.in
create mode 100644 package/pcm-tools/pcm-tools.hash
create mode 100644 package/pcm-tools/pcm-tools.mk
diff --git a/package/Config.in b/package/Config.in
index fd9ef0737a..9c22439885 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -115,6 +115,7 @@ menu "Debugging, profiling and benchmark"
source "package/nmon/Config.in"
source "package/oprofile/Config.in"
source "package/pax-utils/Config.in"
+ source "package/pcm-tools/Config.in"
source "package/pv/Config.in"
source "package/racehound/Config.in"
source "package/ramsmp/Config.in"
diff --git a/package/pcm-tools/0001-Look-for-pcm-core-at-the-default-path.patch b/package/pcm-tools/0001-Look-for-pcm-core-at-the-default-path.patch
new file mode 100644
index 0000000000..933eec0237
--- /dev/null
+++ b/package/pcm-tools/0001-Look-for-pcm-core-at-the-default-path.patch
@@ -0,0 +1,46 @@
+From 53b6161d2413406778fa222274069c82846f0297 Mon Sep 17 00:00:00 2001
+From: Carlos Santos <casantos@datacom.com.br>
+Date: Thu, 6 Dec 2018 21:17:02 -0200
+Subject: [PATCH] Look for pcm-core at the default path
+
+On Buildroot, pcm-core.x is installed as /usr/bin/pcm-core. Remove the
+platform test, since we know that it's neither CigWin nor Windows, and
+use the default path.
+
+It's not nice to have a Buildroot specific patch but let's use one while
+we look for a solution that is acceptable upstream.
+
+Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
+---
+ pmu-query.py | 8 +-------
+ 1 file changed, 1 insertion(+), 7 deletions(-)
+
+diff --git a/pmu-query.py b/pmu-query.py
+index 4c596c7..dc39df6 100755
+--- a/pmu-query.py
++++ b/pmu-query.py
+@@ -3,7 +3,6 @@ import urllib2
+ import json, csv
+ import subprocess
+ import sys
+-import platform
+ import getopt
+
+ all_flag = False
+@@ -38,12 +37,7 @@ if filename == None:
+ except StopIteration:
+ break
+
+- if platform.system() == 'CYGWIN_NT-6.1':
+- p = subprocess.Popen(['./pcm-core.exe -c'],stdout=subprocess.PIPE,shell=True)
+- elif platform.system() == 'Windows':
+- p = subprocess.Popen(['pcm-core.exe -c'],stdout=subprocess.PIPE,shell=True)
+- else:
+- p = subprocess.Popen(['./pcm-core.x -c'],stdout=subprocess.PIPE,shell=True)
++ p = subprocess.Popen(['/usr/bin/pcm-core -c'],stdout=subprocess.PIPE,shell=True)
+
+ (output, err) = p.communicate()
+ p_status = p.wait()
+--
+2.19.2
+
diff --git a/package/pcm-tools/Config.in b/package/pcm-tools/Config.in
new file mode 100644
index 0000000000..2be6a1667a
--- /dev/null
+++ b/package/pcm-tools/Config.in
@@ -0,0 +1,30 @@
+config BR2_PACKAGE_PCM_TOOLS
+ bool "pcm-tools"
+ depends on BR2_i386 || BR2_x86_64
+ select BR2_PACKAGE_HWDATA
+ select BR2_PACKAGE_HWDATA_PCI_IDS
+ help
+ Processor Counter Monitor (PCM) is an application programming
+ interface (API) and a set of tools based on the API to monitor
+ performance and energy metrics of Intel(R) Core(TM), Xeon(R),
+ Atom(TM) and Xeon Phi(TM) processors.
+
+ Requires a kernel with X86_MSR enabled.
+
+ https://github.com/opcm/pcm
+
+if BR2_PACKAGE_PCM_TOOLS
+
+# The pmu-query script is not compatible with Python 3
+config BR2_PACKAGE_PCM_TOOLS_PMU_QUERY
+ bool "install the pmu-query script"
+ default y
+ depends on BR2_PACKAGE_PYTHON
+ select BR2_PACKAGE_CA_CERTIFICATES # https
+ select BR2_PACKAGE_PYTHON_HASHLIB # urllib2
+ select BR2_PACKAGE_PYTHON_SSL # urllib2
+
+comment "pmu-query needs Python 2.x"
+ depends on !BR2_PACKAGE_PYTHON
+
+endif
diff --git a/package/pcm-tools/pcm-tools.hash b/package/pcm-tools/pcm-tools.hash
new file mode 100644
index 0000000000..77f1482462
--- /dev/null
+++ b/package/pcm-tools/pcm-tools.hash
@@ -0,0 +1,3 @@
+# Locally calculated
+sha256 798eb1bc5d9c34fa107de21b2100e8d4326cb45b613bc35baa1e1efb1dd13b04 pcm-tools-201812.tar.gz
+sha256 fac73f62c4d665c82622862a2be2b89713e0f480c93e593af2d8ef29a13d814b LICENSE
diff --git a/package/pcm-tools/pcm-tools.mk b/package/pcm-tools/pcm-tools.mk
new file mode 100644
index 0000000000..c0d6d8bc52
--- /dev/null
+++ b/package/pcm-tools/pcm-tools.mk
@@ -0,0 +1,35 @@
+################################################################################
+#
+# pcm-tools
+#
+################################################################################
+
+PCM_TOOLS_VERSION = 201812
+PCM_TOOLS_SITE = $(call github,opcm,pcm,$(PCM_TOOLS_VERSION))
+PCM_TOOLS_LICENSE = BSD-3-Clause
+PCM_TOOLS_LICENSE_FILES = LICENSE
+
+PCM_TOOLS_EXE_FILES = \
+ pcm-core pcm-iio pcm-lspci pcm-memory pcm-msr pcm-numa \
+ pcm-pcicfg pcm-pcie pcm-power pcm-sensor pcm-tsx pcm
+
+define PCM_TOOLS_BUILD_CMDS
+ touch $(@D)/daemon-binaries
+ $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D) \
+ UNAME=Linux HOST=_LINUX
+endef
+
+ifeq ($(BR2_PACKAGE_PCM_TOOLS_PMU_QUERY),y)
+define PCM_TOOLS_INSTALL_PMU_QUERY
+ $(INSTALL) -D -m 755 $(@D)/pmu-query.py $(TARGET_DIR)/usr/bin/pmu-query
+endef
+endif
+
+define PCM_TOOLS_INSTALL_TARGET_CMDS
+ $(foreach f,$(PCM_TOOLS_EXE_FILES),\
+ $(INSTALL) -D -m 755 $(@D)/$(f).x $(TARGET_DIR)/usr/bin/$(f)
+ )
+ $(PCM_TOOLS_INSTALL_PMU_QUERY)
+endef
+
+$(eval $(generic-package))
--
2.19.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH v2] pcm-tools: new package
2018-12-07 1:20 ` [Buildroot] [PATCH v2] " Carlos Santos
@ 2018-12-08 9:43 ` Peter Korsgaard
0 siblings, 0 replies; 10+ messages in thread
From: Peter Korsgaard @ 2018-12-08 9:43 UTC (permalink / raw)
To: buildroot
>>>>> "Carlos" == Carlos Santos <casantos@datacom.com.br> writes:
> Processor Counter Monitor (PCM) is an application programming interface
> (API) and a set of tools based on the API to monitor performance and
> energy metrics of Intel(R) Core(TM), Xeon(R), Atom(TM) and Xeon Phi(TM)
> processors.
> This package contains a patch on the pmu-query.py script to look for the
> pcm-core program at the default path. It's not nice to have a Buildroot
> specific patch but let's use one while we look for a solution that is
> acceptable upstream.
> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> ---
> Supersedes: https://patchwork.ozlabs.org/patch/864801/
> CC: Romain Naour <romain.naour@gmail.com>
> CC: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
> Changes v1->v2
> - Update to version 201812
> - Do not build the test daemon
> - Drop the patch on the test daemon, that we don't build
> - Drop a patch already applied upstream
> - Update the pmu-query.py patch
> - Make the changes suggested by Romain Naour and Thomas Petazzoni
> +config BR2_PACKAGE_PCM_TOOLS
> + bool "pcm-tools"
> + depends on BR2_i386 || BR2_x86_64
> + select BR2_PACKAGE_HWDATA
> + select BR2_PACKAGE_HWDATA_PCI_IDS
It is written in C++, so I've added the BR2_INSTALL_LIBSTDCPP dependency
and a comment.
> + help
> + Processor Counter Monitor (PCM) is an application programming
> + interface (API) and a set of tools based on the API to monitor
> + performance and energy metrics of Intel(R) Core(TM), Xeon(R),
> + Atom(TM) and Xeon Phi(TM) processors.
> +
> + Requires a kernel with X86_MSR enabled.
It makes more sense to just forcible enable it in linux.mk if pcm-tools
is enabled, similar to how we do it for other packages.
Committed with these 2 fixes, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 10+ messages in thread