lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aea7c0f4-e951-484a-962d-f3a9b5b26b57@linux.intel.com>
Date: Fri, 19 Sep 2025 08:09:08 +0800
From: "Mi, Dapeng" <dapeng1.mi@...ux.intel.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Ian Rogers <irogers@...gle.com>, Peter Zijlstra <peterz@...radead.org>,
 Ingo Molnar <mingo@...hat.com>, Namhyung Kim <namhyung@...nel.org>,
 Adrian Hunter <adrian.hunter@...el.com>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Kan Liang <kan.liang@...ux.intel.com>, Kevin Tian <kevin.tian@...el.com>,
 linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
 Dapeng Mi <dapeng1.mi@...el.com>
Subject: Re: [Patch v2 0/6] Perf kvm commands bug fix


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@...ux.intel.com> 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@...nel.org>
>>> 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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ