Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Namhyung Kim <namhyung@kernel.org>]

On Tue, Nov 11, 2025 at 11:11:45AM -0800, Chen, Zide wrote:
> 
> 
> On 11/10/2025 11:50 PM, Namhyung Kim wrote:
> > On Fri, Nov 07, 2025 at 02:31:23PM -0800, Chen, Zide wrote:
> >>
> >>
> >> On 11/7/2025 1:42 PM, Namhyung Kim wrote:
> >>> On Thu, Nov 06, 2025 at 05:23:09PM -0800, Chen, Zide wrote:
> >>>>
> >>>>
> >>>> On 11/6/2025 10:52 AM, Namhyung Kim wrote:

> >>> Can you run these commands and show the output here?
> >>>
> >>>   $ perf record -e task-clock:S  true
> >>>   $ perf evlist -v
> >>
> >> On 6.18.0-rc4:
> >>
> >> $ perf record -e task-clock:S  true
> >> [ perf record: Woken up 2 times to write data ]
> >> [ perf record: Captured and wrote 0.006 MB perf.data ]
> >>
> >> $ perf evlist -v
> >> task-clock:Su: type: 1 (PERF_TYPE_SOFTWARE), size: 136, config: 0x1
> >> (PERF_COUNT_SW_TASK_CLOCK), { sample_period, sample_freq }: 4000,
> >> sample_type: IP|TID|TIME|READ|ID|PERIOD, read_format: ID|LOST, disabled:
> >> 1, inherit: 1, exclude_kernel: 1, exclude_hv: 1, mmap: 1, comm: 1, freq:
> >> 1, enable_on_exec: 1, task: 1, sample_id_all: 1, mmap2: 1, comm_exec: 1,
> >> ksymbol: 1, bpf_event: 1, build_id: 1
> > 
> > Thanks for sharing this.  Yep, it has the inherit bit.
> > 
> > I think there's a bug in the missing feature test.  Indeed, it should
> > also have PERF_SAMPLE_TID for the test according to the kernel comment.
> > 
> > 	/*
> > 	 * We do not support PERF_SAMPLE_READ on inherited events unless
> > 	 * PERF_SAMPLE_TID is also selected, which allows inherited events to
> > 	 * collect per-thread samples.
> > 	 * See perf_output_read().
> > 	 */
> > 	if (has_inherit_and_sample_read(attr) && !(attr->sample_type & PERF_SAMPLE_TID))
> > 		return ERR_PTR(-EINVAL);
> 
> It seems that the purpose of the inherit_sample_read fallback is to
> remove the inherit attribute when both PERF_SAMPLE_READ and inherit are
> present, but PERF_SAMPLE_TID is not. The new change may not be able to
> accomplish this?

No, the purpose of the missing feature check is to detect whether the
current kernel supports this feature or not.  The correct check should
pass both READ and TID together.

Thanks,
Namhyung