From: Song Liu <songliubraving@fb.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: lkml <linux-kernel@vger.kernel.org>,
kbuild test robot <lkp@intel.com>,
Kernel Team <Kernel-team@fb.com>, Tejun Heo <tj@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Jiri Olsa <jolsa@kernel.org>,
Alexey Budankov <alexey.budankov@linux.intel.com>
Subject: Re: [PATCH v2 1/1] perf: Sharing PMU counters across compatible events
Date: Tue, 11 Sep 2018 13:29:32 +0000 [thread overview]
Message-ID: <90BE3370-3006-426A-80DC-279F07192E70@fb.com> (raw)
In-Reply-To: <20180910081336.GB31644@krava>
> On Sep 10, 2018, at 1:13 AM, Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Thu, Aug 30, 2018 at 06:35:37PM +0000, Song Liu wrote:
>>
>>
>>> On Aug 30, 2018, at 8:13 AM, Jiri Olsa <jolsa@redhat.com> wrote:
>>>
>>> On Wed, Aug 15, 2018 at 10:03:13AM -0700, Song Liu wrote:
>>>
>>> SNIP
>>>
>>>>
>>>> + perf_event_remove_dup(event, ctx);
>>>> /*
>>>> * We can have double detach due to exit/hot-unplug + close.
>>>> */
>>>> @@ -1982,6 +2123,92 @@ event_filter_match(struct perf_event *event)
>>>> perf_cgroup_match(event) && pmu_filter_match(event);
>>>> }
>>>>
>>>> +/* PMU sharing aware version of event->pmu->add() */
>>>> +static int event_pmu_add(struct perf_event *event,
>>>> + struct perf_event_context *ctx)
>>>> +{
>>>> + struct perf_event_dup *dup;
>>>> + int ret;
>>>> +
>>>> + /* no sharing, just do event->pmu->add() */
>>>> + if (event->dup_id == -1)
>>>> + return event->pmu->add(event, PERF_EF_START);
>>>> +
>>>> + dup = &ctx->dup_events[event->dup_id];
>>>> +
>>>> + if (dup->active_event_count) {
>>>> + /* already enabled */
>>>> + dup->active_event_count++;
>>>> + dup->master->pmu->read(dup->master);
>>>> + event->dup_base_count = dup_read_count(dup);
>>>> + event->dup_base_child_count = dup_read_child_count(dup);
>>>> + return 0;
>>>> + }
>>>> +
>>>> + /* try add master */
>>>> + ret = event->pmu->add(dup->master, PERF_EF_START);
>>>> +
>>>> + if (!ret) {
>>>> + dup->active_event_count = 1;
>>>> + event->pmu->read(dup->master);
>>>> + event->dup_base_count = dup_read_count(dup);
>>>> + event->dup_base_child_count = dup_read_child_count(dup);
>>>
>>> should you read the base before calling pmu->add ?
>>> should be same for any dup event not just master
>>>
>>> jirka
>>
>> I am not sure I am following. The pmu is disabled when we call
>> event_pmu_add(). Why do we need to read before calling pmu->add()?
>> And this is the first added dup event for this master, so we don't
>> need to worry about others.
>>
>> Does this make sense?
>
> I was just thinking since the pmu is disable we could
> we don't need to read the event on 2 places.. it's almost
> identic code
How about something like:
+/* PMU sharing aware version of event->pmu->add() */
+static int event_pmu_add(struct perf_event *event,
+ struct perf_event_context *ctx)
+{
+ struct perf_event_dup *dup;
+ int ret;
+
+ /* no sharing, just do event->pmu->add() */
+ if (event->dup_id == -1)
+ return event->pmu->add(event, PERF_EF_START);
+
+ dup = &ctx->dup_events[event->dup_id];
+
+ if (dup->active_event_count = 0) {
+ /* try add master */
+ ret = event->pmu->add(dup->master, PERF_EF_START);
+ if (ret)
+ return ret;
+ }
+
+ dup->active_event_count++;
+ event->pmu->read(dup->master);
+ event->dup_base_count = dup_read_count(dup);
+ event->dup_base_child_count = dup_read_child_count(dup);
+
+ return 0;
+}
next prev parent reply other threads:[~2018-09-11 13:32 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-15 17:03 [PATCH v2 0/1] perf: Sharing PMU counters across compatible events Song Liu
2018-08-15 17:03 ` [PATCH v2 1/1] " Song Liu
2018-08-30 15:13 ` Jiri Olsa
2018-08-30 18:35 ` Song Liu
2018-09-10 8:13 ` Jiri Olsa
2018-09-11 13:29 ` Song Liu [this message]
2018-09-23 21:44 ` Jiri Olsa
2018-08-30 15:18 ` Jiri Olsa
2018-08-30 18:51 ` Song Liu
2018-09-10 8:15 ` Jiri Olsa
2018-09-11 13:38 ` Song Liu
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=90BE3370-3006-426A-80DC-279F07192E70@fb.com \
--to=songliubraving@fb.com \
--cc=Kernel-team@fb.com \
--cc=alexey.budankov@linux.intel.com \
--cc=jolsa@kernel.org \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lkp@intel.com \
--cc=peterz@infradead.org \
--cc=tj@kernel.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 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 NNTP newsgroup(s).