All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: "Mi, Dapeng" <dapeng1.mi@linux.intel.com>
Cc: Ian Rogers <irogers@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Kan Liang <kan.liang@linux.intel.com>,
	Kevin Tian <kevin.tian@intel.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	Dapeng Mi <dapeng1.mi@intel.com>
Subject: Re: [Patch v2 0/6] Perf kvm commands bug fix
Date: Thu, 18 Sep 2025 16:59:38 -0300	[thread overview]
Message-ID: <aMxkqnXdrvv9BN8s@x1> (raw)
In-Reply-To: <189e060b-207b-469f-9b6e-314380d12a42@linux.intel.com>

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@linux.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@kernel.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?

- Arnaldo

  reply	other threads:[~2025-09-18 19:59 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-11  5:55 [Patch v2 0/6] Perf kvm commands bug fix Dapeng Mi
2025-08-11  5:55 ` [Patch v2 1/6] perf tools kvm: Add missed memory allocation check and free Dapeng Mi
2025-08-11  5:55 ` [Patch v2 2/6] perf tools kwork: " Dapeng Mi
2025-08-11  5:55 ` [Patch v2 3/6] perf tools kvm: Fix the potential out of range memory access issue Dapeng Mi
2025-08-11  5:55 ` [Patch v2 4/6] perf tools: Add helper x86__is_intel_cpu() Dapeng Mi
2025-08-11  5:55 ` [Patch v2 5/6] perf tools kvm: Use "cycles" to sample guest for "kvm record" on Intel Dapeng Mi
2025-08-11  5:55 ` [Patch v2 6/6] perf tools kvm: Use "cycles" to sample guest for "kvm top" " Dapeng Mi
2025-08-15 20:15 ` [Patch v2 0/6] Perf kvm commands bug fix Namhyung Kim
2025-09-03  6:32   ` Mi, Dapeng
2025-09-17 17:56   ` Arnaldo Carvalho de Melo
2025-09-17 21:12 ` Ian Rogers
2025-09-17 23:52   ` Mi, Dapeng
2025-09-18 19:59     ` Arnaldo Carvalho de Melo [this message]
2025-09-19  0:09       ` Mi, Dapeng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aMxkqnXdrvv9BN8s@x1 \
    --to=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=dapeng1.mi@intel.com \
    --cc=dapeng1.mi@linux.intel.com \
    --cc=irogers@google.com \
    --cc=kan.liang@linux.intel.com \
    --cc=kevin.tian@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.