linux-kernel - Re: [PATCH v3 4/4] perf/core,x86: synchronize PMU task contexts on optimized context switches

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <63666e0d-176a-0780-97b8-9be7ef6f7aed@linux.intel.com>
Date:   Mon, 21 Oct 2019 13:00:29 +0300
From:   Alexey Budankov <alexey.budankov@...ux.intel.com>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Ingo Molnar <mingo@...hat.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Andi Kleen <ak@...ux.intel.com>,
        Kan Liang <kan.liang@...ux.intel.com>,
        Stephane Eranian <eranian@...gle.com>,
        Ian Rogers <irogers@...gle.com>,
        Song Liu <songliubraving@...com>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 4/4] perf/core,x86: synchronize PMU task contexts on
 optimized context switches


On 21.10.2019 10:59, Ingo Molnar wrote:
> 
> * Alexey Budankov <alexey.budankov@...ux.intel.com> wrote:
> 
>> +			/*
>> +			 * PMU specific parts of task perf context may require
>> +			 * additional synchronization, at least for proper Intel
>> +			 * LBR callstack data profiling;
>> +			 */
>> +			pmu->sync_task_ctx(ctx->task_ctx_data,
>> +					   next_ctx->task_ctx_data);
> 
> Firstly, I'm pretty sure you never run this on a CPU where 
> pmu->sync_task_ctx is NULL, right? ;-)

Yes, right.

> 
> Secondly, even on Intel CPUs in many cases we'll just call into a ~2 deep 
> function pointer based call hierarchy, just to find that nothing needs to 
> be done, because there's no LBR call stack maintained:
> 
> +       if (!one || !another)
> +               return;
> 
> So while it's technically a layering violation, it might make sense to 
> elevate this check to the generic layer and say that synchronization 
> calls by the core layer will always provide two valid pointers?

This would also keep performance benefit of avoiding double indirect 
pointer calls for cases when LBR callstack is not requested.

As far it has the comment saying that check is intentionally placed 
at the core layer it doesn't look like a violation but rather 
performance optimization.

Let me come up with v4 to see how it would look like.

Thanks for observation, 
Alexey

> 
> Thanks,
> 
> 	Ingo
>