* [PATCH] perf tools: Fix missing feature check for inherit + SAMPLE_READ
@ 2025-11-11 7:59 Namhyung Kim
2025-11-11 18:36 ` Ian Rogers
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Namhyung Kim @ 2025-11-11 7:59 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Chen, Zide
It should also have PERF_SAMPLE_TID to enable inherit and PERF_SAMPLE_READ
on recent kernels. Not having _TID makes the feature check wrongly detect
the inherit and _READ support.
It was reported that the following command failed due to the error in
the missing feature check on Intel SPR machines.
$ perf record -e '{cpu/mem-loads-aux/S,cpu/mem-loads,ldlat=3/PS}' -- ls
Error:
Failure to open event 'cpu/mem-loads,ldlat=3/PS' on PMU 'cpu' which will be removed.
Invalid event (cpu/mem-loads,ldlat=3/PS) in per-thread mode, enable system wide with '-a'.
Fixes: 3b193a57baf15c468 ("perf tools: Detect missing kernel features properly")
Reported-by: "Chen, Zide" <zide.chen@intel.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/evsel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 67a898cda86ab559..989c56d4a23f74f4 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2474,7 +2474,7 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu
/* Please add new feature detection here. */
attr.inherit = true;
- attr.sample_type = PERF_SAMPLE_READ;
+ attr.sample_type = PERF_SAMPLE_READ | PERF_SAMPLE_TID;
if (has_attr_feature(&attr, /*flags=*/0))
goto found;
perf_missing_features.inherit_sample_read = true;
--
2.51.2.1041.gc1ab5b90ca-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] perf tools: Fix missing feature check for inherit + SAMPLE_READ 2025-11-11 7:59 [PATCH] perf tools: Fix missing feature check for inherit + SAMPLE_READ Namhyung Kim @ 2025-11-11 18:36 ` Ian Rogers 2025-11-11 19:13 ` Chen, Zide 2025-11-12 17:56 ` Namhyung Kim 2 siblings, 0 replies; 9+ messages in thread From: Ian Rogers @ 2025-11-11 18:36 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Chen, Zide On Mon, Nov 10, 2025 at 11:59 PM Namhyung Kim <namhyung@kernel.org> wrote: > > It should also have PERF_SAMPLE_TID to enable inherit and PERF_SAMPLE_READ > on recent kernels. Not having _TID makes the feature check wrongly detect > the inherit and _READ support. > > It was reported that the following command failed due to the error in > the missing feature check on Intel SPR machines. > > $ perf record -e '{cpu/mem-loads-aux/S,cpu/mem-loads,ldlat=3/PS}' -- ls > Error: > Failure to open event 'cpu/mem-loads,ldlat=3/PS' on PMU 'cpu' which will be removed. > Invalid event (cpu/mem-loads,ldlat=3/PS) in per-thread mode, enable system wide with '-a'. > > Fixes: 3b193a57baf15c468 ("perf tools: Detect missing kernel features properly") > Reported-by: "Chen, Zide" <zide.chen@intel.com> > Signed-off-by: Namhyung Kim <namhyung@kernel.org> Reviewed-by: Ian Rogers <irogers@google.com> Perhaps also: Closes: https://lore.kernel.org/lkml/20251022220802.1335131-1-zide.chen@intel.com/ Thanks, Ian > --- > tools/perf/util/evsel.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > index 67a898cda86ab559..989c56d4a23f74f4 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -2474,7 +2474,7 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu > /* Please add new feature detection here. */ > > attr.inherit = true; > - attr.sample_type = PERF_SAMPLE_READ; > + attr.sample_type = PERF_SAMPLE_READ | PERF_SAMPLE_TID; > if (has_attr_feature(&attr, /*flags=*/0)) > goto found; > perf_missing_features.inherit_sample_read = true; > -- > 2.51.2.1041.gc1ab5b90ca-goog > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] perf tools: Fix missing feature check for inherit + SAMPLE_READ 2025-11-11 7:59 [PATCH] perf tools: Fix missing feature check for inherit + SAMPLE_READ Namhyung Kim 2025-11-11 18:36 ` Ian Rogers @ 2025-11-11 19:13 ` Chen, Zide 2025-11-11 19:40 ` Namhyung Kim 2025-11-12 17:56 ` Namhyung Kim 2 siblings, 1 reply; 9+ messages in thread From: Chen, Zide @ 2025-11-11 19:13 UTC (permalink / raw) To: Namhyung Kim, Arnaldo Carvalho de Melo, Ian Rogers, James Clark Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users On 11/10/2025 11:59 PM, Namhyung Kim wrote: > It should also have PERF_SAMPLE_TID to enable inherit and PERF_SAMPLE_READ > on recent kernels. Not having _TID makes the feature check wrongly detect > the inherit and _READ support. > > It was reported that the following command failed due to the error in > the missing feature check on Intel SPR machines. > > $ perf record -e '{cpu/mem-loads-aux/S,cpu/mem-loads,ldlat=3/PS}' -- ls > Error: > Failure to open event 'cpu/mem-loads,ldlat=3/PS' on PMU 'cpu' which will be removed. > Invalid event (cpu/mem-loads,ldlat=3/PS) in per-thread mode, enable system wide with '-a'. > > Fixes: 3b193a57baf15c468 ("perf tools: Detect missing kernel features properly") > Reported-by: "Chen, Zide" <zide.chen@intel.com> > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/util/evsel.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > index 67a898cda86ab559..989c56d4a23f74f4 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -2474,7 +2474,7 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu > /* Please add new feature detection here. */ > > attr.inherit = true; > - attr.sample_type = PERF_SAMPLE_READ; > + attr.sample_type = PERF_SAMPLE_READ | PERF_SAMPLE_TID; Seems this could have some unintended side effects. For example, consider a :ppp event with PERF_SAMPLE_READ and inherit attributes running on a system where the maximum precise_ip is 2: - It fails to open the event on the first attempt; - It goes through the inherit_sample_read detection and fails again after removing inherit; - Finally, it succeeds after falling back to precision 2 — but the inherit attribute has been unexpectedly removed. I may have missed something, but I don’t quite understand why commit 3b193a57baf15 ("perf tools: Detect missing kernel features properly") performs the check on a dummy evsel instead of the original one. In this way, it might incorrectly fall back an attribute that doesn’t actually help. This means evsel__detect_missing_features() could theoretically roll back a feature that might not actually work. Given that it cannot restore the original evsel state after a failed attempt, side effects may occur. > if (has_attr_feature(&attr, /*flags=*/0)) > goto found; > perf_missing_features.inherit_sample_read = true; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] perf tools: Fix missing feature check for inherit + SAMPLE_READ 2025-11-11 19:13 ` Chen, Zide @ 2025-11-11 19:40 ` Namhyung Kim 2025-11-11 20:03 ` Chen, Zide 0 siblings, 1 reply; 9+ messages in thread From: Namhyung Kim @ 2025-11-11 19:40 UTC (permalink / raw) To: Chen, Zide Cc: Arnaldo Carvalho de Melo, Ian Rogers, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users On Tue, Nov 11, 2025 at 11:13:20AM -0800, Chen, Zide wrote: > > > On 11/10/2025 11:59 PM, Namhyung Kim wrote: > > It should also have PERF_SAMPLE_TID to enable inherit and PERF_SAMPLE_READ > > on recent kernels. Not having _TID makes the feature check wrongly detect > > the inherit and _READ support. > > > > It was reported that the following command failed due to the error in > > the missing feature check on Intel SPR machines. > > > > $ perf record -e '{cpu/mem-loads-aux/S,cpu/mem-loads,ldlat=3/PS}' -- ls > > Error: > > Failure to open event 'cpu/mem-loads,ldlat=3/PS' on PMU 'cpu' which will be removed. > > Invalid event (cpu/mem-loads,ldlat=3/PS) in per-thread mode, enable system wide with '-a'. > > > > Fixes: 3b193a57baf15c468 ("perf tools: Detect missing kernel features properly") > > Reported-by: "Chen, Zide" <zide.chen@intel.com> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > --- > > tools/perf/util/evsel.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > > index 67a898cda86ab559..989c56d4a23f74f4 100644 > > --- a/tools/perf/util/evsel.c > > +++ b/tools/perf/util/evsel.c > > @@ -2474,7 +2474,7 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu > > /* Please add new feature detection here. */ > > > > attr.inherit = true; > > - attr.sample_type = PERF_SAMPLE_READ; > > + attr.sample_type = PERF_SAMPLE_READ | PERF_SAMPLE_TID; > > > Seems this could have some unintended side effects. For example, > consider a :ppp event with PERF_SAMPLE_READ and inherit attributes > running on a system where the maximum precise_ip is 2: > > - It fails to open the event on the first attempt; > - It goes through the inherit_sample_read detection and fails again > after removing inherit; This is not what we want. The kernel supports inherit + SAMPLE_READ so it should not remove the inherit bit. > - Finally, it succeeds after falling back to precision 2 — but the > inherit attribute has been unexpectedly removed. So it'll fallback to precision 2 without removing inherit. > > I may have missed something, but I don’t quite understand why commit > 3b193a57baf15 ("perf tools: Detect missing kernel features properly") > performs the check on a dummy evsel instead of the original one. In this > way, it might incorrectly fall back an attribute that doesn’t actually help. Because different platforms have different limitations on hardware events. You cannot simply use current event for kernel feature check since it can result in wrong decisions due to the limitation. So we picked the software event to avoid the hardware characteristics and to focus on kernel features. > > This means evsel__detect_missing_features() could theoretically roll > back a feature that might not actually work. Given that it cannot > restore the original evsel state after a failed attempt, side effects > may occur. The purpose is to turn off the non-supported features only and try with other settings like precise_ip and exclude_kernels and so on. Thanks, Namhyung ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] perf tools: Fix missing feature check for inherit + SAMPLE_READ 2025-11-11 19:40 ` Namhyung Kim @ 2025-11-11 20:03 ` Chen, Zide 2025-11-11 22:31 ` Namhyung Kim 0 siblings, 1 reply; 9+ messages in thread From: Chen, Zide @ 2025-11-11 20:03 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, Ian Rogers, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users On 11/11/2025 11:40 AM, Namhyung Kim wrote: > On Tue, Nov 11, 2025 at 11:13:20AM -0800, Chen, Zide wrote: >> >> >> On 11/10/2025 11:59 PM, Namhyung Kim wrote: >>> It should also have PERF_SAMPLE_TID to enable inherit and PERF_SAMPLE_READ >>> on recent kernels. Not having _TID makes the feature check wrongly detect >>> the inherit and _READ support. >>> >>> It was reported that the following command failed due to the error in >>> the missing feature check on Intel SPR machines. >>> >>> $ perf record -e '{cpu/mem-loads-aux/S,cpu/mem-loads,ldlat=3/PS}' -- ls >>> Error: >>> Failure to open event 'cpu/mem-loads,ldlat=3/PS' on PMU 'cpu' which will be removed. >>> Invalid event (cpu/mem-loads,ldlat=3/PS) in per-thread mode, enable system wide with '-a'. >>> >>> Fixes: 3b193a57baf15c468 ("perf tools: Detect missing kernel features properly") >>> Reported-by: "Chen, Zide" <zide.chen@intel.com> >>> Signed-off-by: Namhyung Kim <namhyung@kernel.org> >>> --- >>> tools/perf/util/evsel.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c >>> index 67a898cda86ab559..989c56d4a23f74f4 100644 >>> --- a/tools/perf/util/evsel.c >>> +++ b/tools/perf/util/evsel.c >>> @@ -2474,7 +2474,7 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu >>> /* Please add new feature detection here. */ >>> >>> attr.inherit = true; >>> - attr.sample_type = PERF_SAMPLE_READ; >>> + attr.sample_type = PERF_SAMPLE_READ | PERF_SAMPLE_TID; >> >> >> Seems this could have some unintended side effects. For example, >> consider a :ppp event with PERF_SAMPLE_READ and inherit attributes >> running on a system where the maximum precise_ip is 2: >> >> - It fails to open the event on the first attempt; >> - It goes through the inherit_sample_read detection and fails again >> after removing inherit; > > This is not what we want. The kernel supports inherit + SAMPLE_READ > so it should not remove the inherit bit. > > >> - Finally, it succeeds after falling back to precision 2 — but the >> inherit attribute has been unexpectedly removed. > > So it'll fallback to precision 2 without removing inherit. > >> >> I may have missed something, but I don’t quite understand why commit >> 3b193a57baf15 ("perf tools: Detect missing kernel features properly") >> performs the check on a dummy evsel instead of the original one. In this >> way, it might incorrectly fall back an attribute that doesn’t actually help. > > Because different platforms have different limitations on hardware > events. You cannot simply use current event for kernel feature check > since it can result in wrong decisions due to the limitation. So we > picked the software event to avoid the hardware characteristics and to > focus on kernel features. > >> >> This means evsel__detect_missing_features() could theoretically roll >> back a feature that might not actually work. Given that it cannot >> restore the original evsel state after a failed attempt, side effects >> may occur. > > The purpose is to turn off the non-supported features only and try with > other settings like precise_ip and exclude_kernels and so on. OK, thanks! > Thanks, > Namhyung > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] perf tools: Fix missing feature check for inherit + SAMPLE_READ 2025-11-11 20:03 ` Chen, Zide @ 2025-11-11 22:31 ` Namhyung Kim 2025-11-11 22:36 ` Chen, Zide 0 siblings, 1 reply; 9+ messages in thread From: Namhyung Kim @ 2025-11-11 22:31 UTC (permalink / raw) To: Chen, Zide Cc: Arnaldo Carvalho de Melo, Ian Rogers, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users On Tue, Nov 11, 2025 at 12:03:22PM -0800, Chen, Zide wrote: > > > On 11/11/2025 11:40 AM, Namhyung Kim wrote: > > On Tue, Nov 11, 2025 at 11:13:20AM -0800, Chen, Zide wrote: > >> > >> > >> On 11/10/2025 11:59 PM, Namhyung Kim wrote: > >>> It should also have PERF_SAMPLE_TID to enable inherit and PERF_SAMPLE_READ > >>> on recent kernels. Not having _TID makes the feature check wrongly detect > >>> the inherit and _READ support. > >>> > >>> It was reported that the following command failed due to the error in > >>> the missing feature check on Intel SPR machines. > >>> > >>> $ perf record -e '{cpu/mem-loads-aux/S,cpu/mem-loads,ldlat=3/PS}' -- ls > >>> Error: > >>> Failure to open event 'cpu/mem-loads,ldlat=3/PS' on PMU 'cpu' which will be removed. > >>> Invalid event (cpu/mem-loads,ldlat=3/PS) in per-thread mode, enable system wide with '-a'. > >>> > >>> Fixes: 3b193a57baf15c468 ("perf tools: Detect missing kernel features properly") > >>> Reported-by: "Chen, Zide" <zide.chen@intel.com> > >>> Signed-off-by: Namhyung Kim <namhyung@kernel.org> > >>> --- > >>> tools/perf/util/evsel.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > >>> index 67a898cda86ab559..989c56d4a23f74f4 100644 > >>> --- a/tools/perf/util/evsel.c > >>> +++ b/tools/perf/util/evsel.c > >>> @@ -2474,7 +2474,7 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu > >>> /* Please add new feature detection here. */ > >>> > >>> attr.inherit = true; > >>> - attr.sample_type = PERF_SAMPLE_READ; > >>> + attr.sample_type = PERF_SAMPLE_READ | PERF_SAMPLE_TID; > >> > >> > >> Seems this could have some unintended side effects. For example, > >> consider a :ppp event with PERF_SAMPLE_READ and inherit attributes > >> running on a system where the maximum precise_ip is 2: > >> > >> - It fails to open the event on the first attempt; > >> - It goes through the inherit_sample_read detection and fails again > >> after removing inherit; > > > > This is not what we want. The kernel supports inherit + SAMPLE_READ > > so it should not remove the inherit bit. > > > > > >> - Finally, it succeeds after falling back to precision 2 — but the > >> inherit attribute has been unexpectedly removed. > > > > So it'll fallback to precision 2 without removing inherit. > > > >> > >> I may have missed something, but I don’t quite understand why commit > >> 3b193a57baf15 ("perf tools: Detect missing kernel features properly") > >> performs the check on a dummy evsel instead of the original one. In this > >> way, it might incorrectly fall back an attribute that doesn’t actually help. > > > > Because different platforms have different limitations on hardware > > events. You cannot simply use current event for kernel feature check > > since it can result in wrong decisions due to the limitation. So we > > picked the software event to avoid the hardware characteristics and to > > focus on kernel features. > > > >> > >> This means evsel__detect_missing_features() could theoretically roll > >> back a feature that might not actually work. Given that it cannot > >> restore the original evsel state after a failed attempt, side effects > >> may occur. > > > > The purpose is to turn off the non-supported features only and try with > > other settings like precise_ip and exclude_kernels and so on. > > OK, thanks! Can you please confirm if this patch fixes your problem? Thanks, Namhyung ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] perf tools: Fix missing feature check for inherit + SAMPLE_READ 2025-11-11 22:31 ` Namhyung Kim @ 2025-11-11 22:36 ` Chen, Zide 2025-11-12 0:43 ` Namhyung Kim 0 siblings, 1 reply; 9+ messages in thread From: Chen, Zide @ 2025-11-11 22:36 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, Ian Rogers, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users On 11/11/2025 2:31 PM, Namhyung Kim wrote: > On Tue, Nov 11, 2025 at 12:03:22PM -0800, Chen, Zide wrote: >> >> >> On 11/11/2025 11:40 AM, Namhyung Kim wrote: >>> On Tue, Nov 11, 2025 at 11:13:20AM -0800, Chen, Zide wrote: >>>> >>>> >>>> On 11/10/2025 11:59 PM, Namhyung Kim wrote: >>>>> It should also have PERF_SAMPLE_TID to enable inherit and PERF_SAMPLE_READ >>>>> on recent kernels. Not having _TID makes the feature check wrongly detect >>>>> the inherit and _READ support. >>>>> >>>>> It was reported that the following command failed due to the error in >>>>> the missing feature check on Intel SPR machines. >>>>> >>>>> $ perf record -e '{cpu/mem-loads-aux/S,cpu/mem-loads,ldlat=3/PS}' -- ls >>>>> Error: >>>>> Failure to open event 'cpu/mem-loads,ldlat=3/PS' on PMU 'cpu' which will be removed. >>>>> Invalid event (cpu/mem-loads,ldlat=3/PS) in per-thread mode, enable system wide with '-a'. >>>>> >>>>> Fixes: 3b193a57baf15c468 ("perf tools: Detect missing kernel features properly") >>>>> Reported-by: "Chen, Zide" <zide.chen@intel.com> >>>>> Signed-off-by: Namhyung Kim <namhyung@kernel.org> >>>>> --- >>>>> tools/perf/util/evsel.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c >>>>> index 67a898cda86ab559..989c56d4a23f74f4 100644 >>>>> --- a/tools/perf/util/evsel.c >>>>> +++ b/tools/perf/util/evsel.c >>>>> @@ -2474,7 +2474,7 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu >>>>> /* Please add new feature detection here. */ >>>>> >>>>> attr.inherit = true; >>>>> - attr.sample_type = PERF_SAMPLE_READ; >>>>> + attr.sample_type = PERF_SAMPLE_READ | PERF_SAMPLE_TID; >>>> >>>> >>>> Seems this could have some unintended side effects. For example, >>>> consider a :ppp event with PERF_SAMPLE_READ and inherit attributes >>>> running on a system where the maximum precise_ip is 2: >>>> >>>> - It fails to open the event on the first attempt; >>>> - It goes through the inherit_sample_read detection and fails again >>>> after removing inherit; >>> >>> This is not what we want. The kernel supports inherit + SAMPLE_READ >>> so it should not remove the inherit bit. >>> >>> >>>> - Finally, it succeeds after falling back to precision 2 — but the >>>> inherit attribute has been unexpectedly removed. >>> >>> So it'll fallback to precision 2 without removing inherit. >>> >>>> >>>> I may have missed something, but I don’t quite understand why commit >>>> 3b193a57baf15 ("perf tools: Detect missing kernel features properly") >>>> performs the check on a dummy evsel instead of the original one. In this >>>> way, it might incorrectly fall back an attribute that doesn’t actually help. >>> >>> Because different platforms have different limitations on hardware >>> events. You cannot simply use current event for kernel feature check >>> since it can result in wrong decisions due to the limitation. So we >>> picked the software event to avoid the hardware characteristics and to >>> focus on kernel features. >>> >>>> >>>> This means evsel__detect_missing_features() could theoretically roll >>>> back a feature that might not actually work. Given that it cannot >>>> restore the original evsel state after a failed attempt, side effects >>>> may occur. >>> >>> The purpose is to turn off the non-supported features only and try with >>> other settings like precise_ip and exclude_kernels and so on. >> >> OK, thanks! > > Can you please confirm if this patch fixes your problem? Yes, it works! > Thanks, > Namhyung > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] perf tools: Fix missing feature check for inherit + SAMPLE_READ 2025-11-11 22:36 ` Chen, Zide @ 2025-11-12 0:43 ` Namhyung Kim 0 siblings, 0 replies; 9+ messages in thread From: Namhyung Kim @ 2025-11-12 0:43 UTC (permalink / raw) To: Chen, Zide Cc: Arnaldo Carvalho de Melo, Ian Rogers, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users On Tue, Nov 11, 2025 at 02:36:19PM -0800, Chen, Zide wrote: > > > On 11/11/2025 2:31 PM, Namhyung Kim wrote: > > On Tue, Nov 11, 2025 at 12:03:22PM -0800, Chen, Zide wrote: > >> > >> > >> On 11/11/2025 11:40 AM, Namhyung Kim wrote: > >>> On Tue, Nov 11, 2025 at 11:13:20AM -0800, Chen, Zide wrote: > >>>> > >>>> > >>>> On 11/10/2025 11:59 PM, Namhyung Kim wrote: > >>>>> It should also have PERF_SAMPLE_TID to enable inherit and PERF_SAMPLE_READ > >>>>> on recent kernels. Not having _TID makes the feature check wrongly detect > >>>>> the inherit and _READ support. > >>>>> > >>>>> It was reported that the following command failed due to the error in > >>>>> the missing feature check on Intel SPR machines. > >>>>> > >>>>> $ perf record -e '{cpu/mem-loads-aux/S,cpu/mem-loads,ldlat=3/PS}' -- ls > >>>>> Error: > >>>>> Failure to open event 'cpu/mem-loads,ldlat=3/PS' on PMU 'cpu' which will be removed. > >>>>> Invalid event (cpu/mem-loads,ldlat=3/PS) in per-thread mode, enable system wide with '-a'. > >>>>> > >>>>> Fixes: 3b193a57baf15c468 ("perf tools: Detect missing kernel features properly") > >>>>> Reported-by: "Chen, Zide" <zide.chen@intel.com> > >>>>> Signed-off-by: Namhyung Kim <namhyung@kernel.org> > >>>>> --- > >>>>> tools/perf/util/evsel.c | 2 +- > >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > >>>>> index 67a898cda86ab559..989c56d4a23f74f4 100644 > >>>>> --- a/tools/perf/util/evsel.c > >>>>> +++ b/tools/perf/util/evsel.c > >>>>> @@ -2474,7 +2474,7 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu > >>>>> /* Please add new feature detection here. */ > >>>>> > >>>>> attr.inherit = true; > >>>>> - attr.sample_type = PERF_SAMPLE_READ; > >>>>> + attr.sample_type = PERF_SAMPLE_READ | PERF_SAMPLE_TID; > >>>> > >>>> > >>>> Seems this could have some unintended side effects. For example, > >>>> consider a :ppp event with PERF_SAMPLE_READ and inherit attributes > >>>> running on a system where the maximum precise_ip is 2: > >>>> > >>>> - It fails to open the event on the first attempt; > >>>> - It goes through the inherit_sample_read detection and fails again > >>>> after removing inherit; > >>> > >>> This is not what we want. The kernel supports inherit + SAMPLE_READ > >>> so it should not remove the inherit bit. > >>> > >>> > >>>> - Finally, it succeeds after falling back to precision 2 — but the > >>>> inherit attribute has been unexpectedly removed. > >>> > >>> So it'll fallback to precision 2 without removing inherit. > >>> > >>>> > >>>> I may have missed something, but I don’t quite understand why commit > >>>> 3b193a57baf15 ("perf tools: Detect missing kernel features properly") > >>>> performs the check on a dummy evsel instead of the original one. In this > >>>> way, it might incorrectly fall back an attribute that doesn’t actually help. > >>> > >>> Because different platforms have different limitations on hardware > >>> events. You cannot simply use current event for kernel feature check > >>> since it can result in wrong decisions due to the limitation. So we > >>> picked the software event to avoid the hardware characteristics and to > >>> focus on kernel features. > >>> > >>>> > >>>> This means evsel__detect_missing_features() could theoretically roll > >>>> back a feature that might not actually work. Given that it cannot > >>>> restore the original evsel state after a failed attempt, side effects > >>>> may occur. > >>> > >>> The purpose is to turn off the non-supported features only and try with > >>> other settings like precise_ip and exclude_kernels and so on. > >> > >> OK, thanks! > > > > Can you please confirm if this patch fixes your problem? > > Yes, it works! Thanks, I'll add your Tested-by then! Namhyung ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] perf tools: Fix missing feature check for inherit + SAMPLE_READ 2025-11-11 7:59 [PATCH] perf tools: Fix missing feature check for inherit + SAMPLE_READ Namhyung Kim 2025-11-11 18:36 ` Ian Rogers 2025-11-11 19:13 ` Chen, Zide @ 2025-11-12 17:56 ` Namhyung Kim 2 siblings, 0 replies; 9+ messages in thread From: Namhyung Kim @ 2025-11-12 17:56 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark, Namhyung Kim Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Chen, Zide On Mon, 10 Nov 2025 23:59:44 -0800, Namhyung Kim wrote: > It should also have PERF_SAMPLE_TID to enable inherit and PERF_SAMPLE_READ > on recent kernels. Not having _TID makes the feature check wrongly detect > the inherit and _READ support. > > It was reported that the following command failed due to the error in > the missing feature check on Intel SPR machines. > > [...] Applied to perf-tools-next, thanks! Best regards, Namhyung ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-11-12 17:56 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-11 7:59 [PATCH] perf tools: Fix missing feature check for inherit + SAMPLE_READ Namhyung Kim 2025-11-11 18:36 ` Ian Rogers 2025-11-11 19:13 ` Chen, Zide 2025-11-11 19:40 ` Namhyung Kim 2025-11-11 20:03 ` Chen, Zide 2025-11-11 22:31 ` Namhyung Kim 2025-11-11 22:36 ` Chen, Zide 2025-11-12 0:43 ` Namhyung Kim 2025-11-12 17:56 ` Namhyung Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).