Re: [Patch v2 0/6] Perf kvm commands bug fix

[Mi, Dapeng]

On 9/19/2025 3:59 AM, Arnaldo Carvalho de Melo wrote:
> On Thu, Sep 18, 2025 at 07:52:43AM +0800, Mi, Dapeng wrote:
>> On 9/18/2025 5:12 AM, Ian Rogers wrote:
>>> On Sun, Aug 10, 2025 at 10:56 PM Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx> wrote:
>>>> his patch-set fixes perf kvm commands issues, like missed memory
>>>> allocation check/free, out of range memory access and especially the
>>>> issue that fails to sample guest with "perf kvm record/top" commands on
>>>> Intel platforms.
>>>>
>>>> The commit 634d36f82517 ("perf record: Just use "cycles:P" as the
>>>>  default event") changes to use PEBS event to do sampling by default
>>>> including guest sampling. This breaks host to sample guest with commands
>>>> "perf kvm record/top" on Intel platforms.
>>> Huh? That change is:
>>> ```
>>> $ git show 634d36f82517
>>> commit 634d36f82517eb5c6a9b9ec7fe3ba19dbbcb7809
>>> Author: Namhyung Kim <namhyung@xxxxxxxxxx>
>>> Date:   Tue Oct 15 23:23:58 2024 -0700
>>>
>>>     perf record: Just use "cycles:P" as the default event
>>>
>>>     The fallback logic can add ":u" modifier if needed.
>>> ...
>>> -               bool can_profile_kernel = perf_event_paranoid_check(1);
>>> -
>>> -               err = parse_event(rec->evlist, can_profile_kernel ?
>>> "cycles:P" : "cycles:Pu");
>>> +               err = parse_event(rec->evlist, "cycles:P");
>>> ...
>>> ```
>>> isn't the precision the same before and after? I think you've blamed
>>> the wrong patch.
>>>
>>> The change to use cycles:P looks to come from commit 7b100989b4f6
>>> ("perf evlist: Remove __evlist__add_default") but the old code was
>>> doing things like "evsel->precise_max = true;" so I think I was just
>>> carrying forward behavior. The use of precise_max comes from commit
>>> 4e8a5c155137 ("perf evsel: Fix max perf_event_attr.precise_ip
>>> detection") from over 6 years ago, and the behavior before that also
>>> appears to have been to use the maximum supported precision.
>>>
>>> Apart from the blame and commit message being off I think the change
>>> is okay, delta my usual complaint that weak symbols are the devil's
>>> work.
>> Hmm, yeah, you're right. Thanks for correcting this. 
> Hi Dapeng,
>
> 	Can you please fix the patch descriptions and Fixes references
> and resubmit?

Sure. would do. Thanks.


>
> - Arnaldo