From: David Carrillo-Cisneros <davidcc@google.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
"x86@kernel.org" <x86@kernel.org>, Ingo Molnar <mingo@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
Andi Kleen <ak@linux.intel.com>, Kan Liang <kan.liang@intel.com>,
Borislav Petkov <bp@suse.de>,
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
Dave Hansen <dave.hansen@linux.intel.com>,
Vikas Shivappa <vikas.shivappa@linux.intel.com>,
Mark Rutland <mark.rutland@arm.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Vince Weaver <vince@deater.net>, Paul Turner <pjt@google.com>,
Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH v2 2/2] perf/core: Remove perf_cpu_context::unique_pmu
Date: Fri, 20 Jan 2017 12:30:38 -0800 [thread overview]
Message-ID: <CALcN6mitifD4y=1E=TdLZ5ByaNcXds0XEK2=erA7mLFeZBMyBA@mail.gmail.com> (raw)
In-Reply-To: <20170120092033.GK6515@twins.programming.kicks-ass.net>
On Fri, Jan 20, 2017 at 1:20 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Jan 18, 2017 at 11:24:54AM -0800, David Carrillo-Cisneros wrote:
>> cpuctx->unique_pmu was originally introduced as a way to identify cpuctxs
>> with shared pmus in order to avoid visiting the same cpuctx more than once
>> in a for_each_pmu loop.
>>
>> cpuctx->unique_pmu == cpuctx->pmu in non-software task contexts since they
>> have only one pmu per cpuctx. Since perf_pmu_sched_task is only called in
>> hw contexts, this patch replaces cpuctx->unique_pmu by cpuctx->pmu in it.
>>
>> The change above, together with the previous patch in this series, removed
>> the remaining uses of cpuctx->unique_pmu, so we remove it altogether.
>>
>> Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
>> Acked-by: Mark Rutland <mark.rutland@arm.com>
>
>
>> @@ -8572,37 +8572,10 @@ static struct perf_cpu_context __percpu *find_pmu_context(int ctxn)
>> return NULL;
>> }
>>
>> -static void update_pmu_context(struct pmu *pmu, struct pmu *old_pmu)
>> -{
>> - int cpu;
>> -
>> - for_each_possible_cpu(cpu) {
>> - struct perf_cpu_context *cpuctx;
>> -
>> - cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
>> -
>> - if (cpuctx->unique_pmu == old_pmu)
>> - cpuctx->unique_pmu = pmu;
>> - }
>> -}
>> -
>> static void free_pmu_context(struct pmu *pmu)
>> {
>> - struct pmu *i;
>> -
>> mutex_lock(&pmus_lock);
>> - /*
>> - * Like a real lame refcount.
>> - */
>> - list_for_each_entry(i, &pmus, entry) {
>> - if (i->pmu_cpu_context == pmu->pmu_cpu_context) {
>> - update_pmu_context(i, pmu);
>> - goto out;
>> - }
>> - }
>> -
>> free_percpu(pmu->pmu_cpu_context);
>> -out:
>> mutex_unlock(&pmus_lock);
>> }
>
> This very much relies on us never calling perf_pmu_unregister() on the
> software PMUs afaict. A condition not mention in the Changelog.
>
What's a good way to solve this? Update the Changelog or add code to
update ctx->pmu?
This issue would go away cleanly if we were to remove the context
sharing across pmu's. Would you support work in that direction?
Thanks,
David
next prev parent reply other threads:[~2017-01-20 20:31 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-18 19:24 [PATCH v2 0/2] Optimize cgroup ctx switch and remove cpuctx->unique_pmu David Carrillo-Cisneros
2017-01-18 19:24 ` [PATCH v2 1/2] perf/core: Make cgroup switch visit only cpuctxs with cgroup events David Carrillo-Cisneros
2017-01-30 11:57 ` [tip:perf/core] " tip-bot for David Carrillo-Cisneros
2017-01-18 19:24 ` [PATCH v2 2/2] perf/core: Remove perf_cpu_context::unique_pmu David Carrillo-Cisneros
2017-01-20 9:20 ` Peter Zijlstra
2017-01-20 14:18 ` Mark Rutland
2017-01-20 20:30 ` David Carrillo-Cisneros [this message]
2017-01-25 15:23 ` Peter Zijlstra
2017-01-30 11:58 ` [tip:perf/core] " tip-bot for David Carrillo-Cisneros
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='CALcN6mitifD4y=1E=TdLZ5ByaNcXds0XEK2=erA7mLFeZBMyBA@mail.gmail.com' \
--to=davidcc@google.com \
--cc=acme@kernel.org \
--cc=ak@linux.intel.com \
--cc=bp@suse.de \
--cc=dave.hansen@linux.intel.com \
--cc=eranian@google.com \
--cc=kan.liang@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=pjt@google.com \
--cc=srinivas.pandruvada@linux.intel.com \
--cc=tglx@linutronix.de \
--cc=vikas.shivappa@linux.intel.com \
--cc=vince@deater.net \
--cc=x86@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).