[PATCH v1] tools lib api cpu: Remove unused file

Ian Rogers posted 1 patch 1 year, 1 month ago
tools/lib/api/Build    |  1 -
tools/lib/api/Makefile |  2 +-
tools/lib/api/cpu.c    | 19 -------------------
tools/lib/api/cpu.h    |  7 -------
4 files changed, 1 insertion(+), 28 deletions(-)
delete mode 100644 tools/lib/api/cpu.c
delete mode 100644 tools/lib/api/cpu.h
[PATCH v1] tools lib api cpu: Remove unused file
Posted by Ian Rogers 1 year, 1 month ago
No use in tools could be found, remove to simplify the code base.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/lib/api/Build    |  1 -
 tools/lib/api/Makefile |  2 +-
 tools/lib/api/cpu.c    | 19 -------------------
 tools/lib/api/cpu.h    |  7 -------
 4 files changed, 1 insertion(+), 28 deletions(-)
 delete mode 100644 tools/lib/api/cpu.c
 delete mode 100644 tools/lib/api/cpu.h

diff --git a/tools/lib/api/Build b/tools/lib/api/Build
index 6e2373db5598..2be979407615 100644
--- a/tools/lib/api/Build
+++ b/tools/lib/api/Build
@@ -1,6 +1,5 @@
 libapi-y += fd/
 libapi-y += fs/
-libapi-y += cpu.o
 libapi-y += debug.o
 libapi-y += str_error_r.o
 
diff --git a/tools/lib/api/Makefile b/tools/lib/api/Makefile
index 7f6396087b46..bc08bccc94cb 100644
--- a/tools/lib/api/Makefile
+++ b/tools/lib/api/Makefile
@@ -95,7 +95,7 @@ install_lib: $(LIBFILE)
 		$(call do_install_mkdir,$(libdir_SQ)); \
 		cp -fpR $(LIBFILE) $(DESTDIR)$(libdir_SQ)
 
-HDRS := cpu.h debug.h io.h
+HDRS := debug.h io.h
 FD_HDRS := fd/array.h
 FS_HDRS := fs/fs.h fs/tracing_path.h
 INSTALL_HDRS_PFX := $(DESTDIR)$(prefix)/include/api
diff --git a/tools/lib/api/cpu.c b/tools/lib/api/cpu.c
deleted file mode 100644
index 4af6d4b7aa07..000000000000
--- a/tools/lib/api/cpu.c
+++ /dev/null
@@ -1,19 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <stdio.h>
-
-#include "cpu.h"
-#include "fs/fs.h"
-
-int cpu__get_max_freq(unsigned long long *freq)
-{
-	char entry[PATH_MAX];
-	int cpu;
-
-	if (sysfs__read_int("devices/system/cpu/online", &cpu) < 0)
-		return -1;
-
-	snprintf(entry, sizeof(entry),
-		 "devices/system/cpu/cpu%d/cpufreq/cpuinfo_max_freq", cpu);
-
-	return sysfs__read_ull(entry, freq);
-}
diff --git a/tools/lib/api/cpu.h b/tools/lib/api/cpu.h
deleted file mode 100644
index 90a102fb20de..000000000000
--- a/tools/lib/api/cpu.h
+++ /dev/null
@@ -1,7 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __API_CPU__
-#define __API_CPU__
-
-int cpu__get_max_freq(unsigned long long *freq);
-
-#endif /* __API_CPU__ */
-- 
2.47.1.613.gc27f4b7a9f-goog
Re: [PATCH v1] tools lib api cpu: Remove unused file
Posted by Ian Rogers 1 year, 1 month ago
On Thu, Dec 19, 2024 at 12:54 PM Ian Rogers <irogers@google.com> wrote:
>
> No use in tools could be found, remove to simplify the code base.
>
> Signed-off-by: Ian Rogers <irogers@google.com>

Ping.

Thanks,
Ian
Re: [PATCH v1] tools lib api cpu: Remove unused file
Posted by Liang, Kan 1 year, 1 month ago

On 2025-01-06 5:32 p.m., Ian Rogers wrote:
> On Thu, Dec 19, 2024 at 12:54 PM Ian Rogers <irogers@google.com> wrote:
>>
>> No use in tools could be found, remove to simplify the code base.
>>

This was to support the patch which generate per-sample
freq/CPU%/CORE_BUSY%.
https://lore.kernel.org/lkml/1442413316-33518-1-git-send-email-kan.liang@intel.com/

However, the patch set was not merged. I don't remember the exact reason.

It looks like there is no one interested in it. There should be no
reason to keep it.

Acked-by: Kan Liang <kan.liang@linux.intel.com>

Thanks,
Kan
>> Signed-off-by: Ian Rogers <irogers@google.com>
> 
> Ping.
> 
> Thanks,
> Ian
> 

Re: [PATCH v1] tools lib api cpu: Remove unused file
Posted by Ian Rogers 1 year, 1 month ago
On Tue, Jan 7, 2025 at 6:22 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
> On 2025-01-06 5:32 p.m., Ian Rogers wrote:
> > On Thu, Dec 19, 2024 at 12:54 PM Ian Rogers <irogers@google.com> wrote:
> >>
> >> No use in tools could be found, remove to simplify the code base.
> >>
>
> This was to support the patch which generate per-sample
> freq/CPU%/CORE_BUSY%.
> https://lore.kernel.org/lkml/1442413316-33518-1-git-send-email-kan.liang@intel.com/
>
> However, the patch set was not merged. I don't remember the exact reason.
>
> It looks like there is no one interested in it. There should be no
> reason to keep it.
>
> Acked-by: Kan Liang <kan.liang@linux.intel.com>

Thanks Kan! That series looks nice. There is an overlap with computing
frequencies and metrics like cpu_operating_frequency:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/pmu-events/arch/x86/sapphirerapids/spr-metrics.json?h=perf-tools-next#n45

I wonder two things:
1) perhaps we can add the --perf-freq flag but make the implementation
use metrics rather than hard coded events. Outside of perf stat the
json metrics aren't really plumbed up, so this would be a bunch of
work.
2) the metrics compute the TSC frequency from cpuid, and fall back to
/proc/cpuinfo:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/arch/x86/util/tsc.c?h=perf-tools-next
Perhaps there should be another fallback to
/sys/devices/system/cpu/cpu%d/cpufreq/cpuinfo_max_freq, as in this
series.

Thanks,
Ian
Re: [PATCH v1] tools lib api cpu: Remove unused file
Posted by Liang, Kan 1 year, 1 month ago

On 2025-01-07 11:01 a.m., Ian Rogers wrote:
> On Tue, Jan 7, 2025 at 6:22 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>>
>> On 2025-01-06 5:32 p.m., Ian Rogers wrote:
>>> On Thu, Dec 19, 2024 at 12:54 PM Ian Rogers <irogers@google.com> wrote:
>>>>
>>>> No use in tools could be found, remove to simplify the code base.
>>>>
>>
>> This was to support the patch which generate per-sample
>> freq/CPU%/CORE_BUSY%.
>> https://lore.kernel.org/lkml/1442413316-33518-1-git-send-email-kan.liang@intel.com/
>>
>> However, the patch set was not merged. I don't remember the exact reason.
>>
>> It looks like there is no one interested in it. There should be no
>> reason to keep it.
>>
>> Acked-by: Kan Liang <kan.liang@linux.intel.com>
> 
> Thanks Kan! That series looks nice. There is an overlap with computing
> frequencies and metrics like cpu_operating_frequency:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/pmu-events/arch/x86/sapphirerapids/spr-metrics.json?h=perf-tools-next#n45
> 
> I wonder two things:
> 1) perhaps we can add the --perf-freq flag but make the implementation
> use metrics rather than hard coded events. Outside of perf stat the
> json metrics aren't really plumbed up, so this would be a bunch of
> work.

At that time, the metrics was not well supported. Everything is
hardcoded. Yes, now, we should utilize the json metrics for it.

> 2) the metrics compute the TSC frequency from cpuid, and fall back to
> /proc/cpuinfo:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/arch/x86/util/tsc.c?h=perf-tools-next
> Perhaps there should be another fallback to
> /sys/devices/system/cpu/cpu%d/cpufreq/cpuinfo_max_freq, as in this
> series.

As my understanding, the TSC frequency should be a non-turbo frequency.
The cpuinfo_max_freq should be a turbo frequency. It may not be used as
a fallback.

Thanks,
Kan
> 
> Thanks,
> Ian