From: "Zhang, Rui" <rui.zhang@intel.com>
To: "rafael@kernel.org" <rafael@kernel.org>
Cc: "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
"Wysocki, Rafael J" <rafael.j.wysocki@intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Pandruvada, Srinivas" <srinivas.pandruvada@intel.com>
Subject: Re: [PATCH V3 1/2] powercap: intel_rapl: Introduce APIs for PMU support
Date: Sun, 28 Apr 2024 09:35:49 +0000 [thread overview]
Message-ID: <299a61dffc891af6f5896ba77e500803322fdb04.camel@intel.com> (raw)
In-Reply-To: <CAJZ5v0iaf8Own=NuHZuYfVpKPBM-ZYHf-SYEgwsjREg-XfuNbA@mail.gmail.com>
> > +static u64 rapl_event_update(struct perf_event *event)
> > +{
> > + struct hw_perf_event *hwc = &event->hw;
> > + struct rapl_package_pmu_data *data =
> > event_to_pmu_data(event);
> > + u64 prev_raw_count, new_raw_count;
> > + s64 delta, sdelta;
> > + s64 tmp;
> > +
> > + do {
> > + prev_raw_count = local64_read(&hwc->prev_count);
> > + new_raw_count = event_read_counter(event);
> > + tmp = local64_cmpxchg(&hwc->prev_count,
> > prev_raw_count, new_raw_count);
> > + } while (tmp != prev_raw_count);
>
> I think that it is only safe to call this function for draining an
> event going away, because otherwise the above may turn into an
> endless
> loop, and the function is called under a spinlock.
IMO, the logic above is needed by the hardware events that can generate
interrupts and update hwc->prev_count in the interrupt handler.
For the other PMUs, I suspect this loop actually makes any difference.
However, currently, this is the common logic followed by all PMU
drivers.
>
> I would add a comment (above the loop) explaining that this is about
> draining, so the counter is expected to stop incrementing shortly.
>
> The rest of the patch LGTM.
>
Thanks,
rui
next prev parent reply other threads:[~2024-04-28 9:35 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-22 16:20 [PATCH V3 0/2] powercap: Introduce TPMI RAPL PMU support Zhang Rui
2024-04-22 16:20 ` [PATCH V3 1/2] powercap: intel_rapl: Introduce APIs for " Zhang Rui
2024-04-26 17:55 ` Rafael J. Wysocki
2024-04-28 9:35 ` Zhang, Rui [this message]
2024-04-22 16:20 ` [PATCH V3 2/2] powercap: intel_rapl_tpmi: Enable " Zhang Rui
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=299a61dffc891af6f5896ba77e500803322fdb04.camel@intel.com \
--to=rui.zhang@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rafael.j.wysocki@intel.com \
--cc=rafael@kernel.org \
--cc=srinivas.pandruvada@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).