All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: acme@kernel.org, adrian.hunter@intel.com, james.clark@linaro.org,
	jolsa@kernel.org, linux-kernel@vger.kernel.org,
	linux-perf-users@vger.kernel.org, mingo@redhat.com,
	nigro.fra@gmail.com, peterz@infradead.org, tmricht@linux.ibm.com
Subject: Re: [PATCH v4 1/2] perf tool_pmu: Make tool PMU events respect enable/disable
Date: Wed, 20 May 2026 16:06:39 -0700	[thread overview]
Message-ID: <ag4-f39wo4OKgYew@google.com> (raw)
In-Reply-To: <CAP-5=fVsCXqC2-0-huGq5qdkdqY1KuKO=mdv7ZorTjdgyWF+Ag@mail.gmail.com>

On Tue, May 19, 2026 at 01:13:35AM -0700, Ian Rogers wrote:
> On Mon, May 18, 2026 at 11:43 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Mon, May 18, 2026 at 06:41:05PM -0700, Ian Rogers wrote:
> > > Tool PMU events (duration_time, user_time, system_time) currently
> > > count from when the event is opened to when it is read. This causes
> > > issues with features like the delay option (-D) or control fd, where
> > > events are opened but should not start counting immediately.
> > >
> > > Make these events behave more like regular counters by implementing
> > > proper enable and disable support. Add accumulated_time to struct
> > > evsel to track time while enabled, and implement enable/disable CPU
> > > callbacks to start/stop counting.
> > >
> > > Also generalize userspace PMU mixed group handling. Userspace synthetic
> > > PMUs (type > PERF_PMU_TYPE_PE_END) do not have kernel implementations and
> > > cannot be grouped in the kernel (opened with group_fd = -1), and are
> > > skipped by kernel enable/disable calls. Iterate over group members in
> > > userspace and manually enable/disable any members if the leader or the
> > > member is a non-perf-event open PMU, and synchronize their disabled flags.
> >
> > Can we divide the commit into smaller pieces?  I think we can have
> >
> >  * preparation for accumulated_time
> >  * implement enable/disable for tool PMU
> >  * wire them to evsel__{enable,disable}[_cpu]
> >  * support group members properly
> >
> > What do you think?
> 
> That sounds needlessly painful, involving many irrelevant intermediate
> states with dead functions and variables. Most of the patch is
> confined to tool_pmu, we could make the evsel patch smaller by
> removing comments. The changes in evsel are pretty minimal and the
> patch is largely confined to tool_pmu.

I didn't think it'd add many temporary code especially for group
handling.  But I'm fine with the single patch if breaking it up would
cause a lot of pains to you.

Thanks,
Namhyung


  reply	other threads:[~2026-05-20 23:06 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-18 18:39 [PATCH v1 0/2] perf tool_pmu: Support enable/disable for tool PMU events Ian Rogers
2026-05-18 18:39 ` [PATCH v1 1/2] perf tool_pmu: Make tool PMU events respect enable/disable Ian Rogers
2026-05-18 19:26   ` sashiko-bot
2026-05-18 18:39 ` [PATCH v1 2/2] perf tests: Add test for stat delay option with duration_time Ian Rogers
2026-05-18 19:33   ` sashiko-bot
2026-05-18 20:14 ` [PATCH v2 0/2] perf tool_pmu: Support enable/disable for tool PMU events Ian Rogers
2026-05-18 20:14   ` [PATCH v2 1/2] perf tool_pmu: Make tool PMU events respect enable/disable Ian Rogers
2026-05-18 20:49     ` sashiko-bot
2026-05-18 20:14   ` [PATCH v2 2/2] perf tests: Add test for stat delay option with duration_time Ian Rogers
2026-05-18 20:55     ` sashiko-bot
2026-05-18 22:37   ` [PATCH v3 0/2] perf tool_pmu: Support enable/disable for tool PMU events Ian Rogers
2026-05-18 22:37     ` [PATCH v3 1/2] perf tool_pmu: Make tool PMU events respect enable/disable Ian Rogers
2026-05-18 23:02       ` sashiko-bot
2026-05-18 22:37     ` [PATCH v3 2/2] perf tests: Add test for stat delay option with duration_time Ian Rogers
2026-05-19  1:41     ` [PATCH v4 0/2] perf tool_pmu: Support enable/disable for tool PMU events Ian Rogers
2026-05-19  1:41       ` [PATCH v4 1/2] perf tool_pmu: Make tool PMU events respect enable/disable Ian Rogers
2026-05-19  6:43         ` Namhyung Kim
2026-05-19  8:13           ` Ian Rogers
2026-05-20 23:06             ` Namhyung Kim [this message]
2026-05-23  0:40         ` Arnaldo Carvalho de Melo
2026-05-19  1:41       ` [PATCH v4 2/2] perf tests: Add test for stat delay option with duration_time Ian Rogers
2026-05-22 22:10       ` [PATCH v4 0/2] perf tool_pmu: Support enable/disable for tool PMU events Ian Rogers

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=ag4-f39wo4OKgYew@google.com \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=nigro.fra@gmail.com \
    --cc=peterz@infradead.org \
    --cc=tmricht@linux.ibm.com \
    /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.