linux-kernel - Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi

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]

Date:   Mon, 29 May 2017 13:46:38 +0300
From:   Alexey Budankov <alexey.budankov@...ux.intel.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Kan Liang <kan.liang@...el.com>,
        Dmitri Prokhorov <Dmitry.Prohorov@...el.com>,
        Valery Cherepennikov <valery.cherepennikov@...el.com>,
        David Carrillo-Cisneros <davidcc@...gle.com>,
        Stephane Eranian <eranian@...gle.com>,
        Mark Rutland <mark.rutland@....com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process,
 profiling of STREAM benchmark on Intel Xeon Phi

On 29.05.2017 13:33, Peter Zijlstra wrote:
> On Mon, May 29, 2017 at 12:24:53PM +0300, Alexey Budankov wrote:
>> On 29.05.2017 10:45, Peter Zijlstra wrote:
>>> On Sat, May 27, 2017 at 02:19:51PM +0300, Alexey Budankov wrote:
>>>> Solution:
>>>>
>>>> cpu indexed trees for perf_event_context::pinned_groups and
>>>> perf_event_context::flexible_groups lists are introduced. Every tree node
>>>> keeps a list of groups allocated for the same cpu. A tree references only
>>>> groups located at the appropriate group list. The tree provides capability
>>>> to iterate over groups allocated for a specific cpu only, what is exactly
>>>> required by multiplexing timer interrupt handler. The handler runs per-cpu
>>>> and enables/disables groups using group_sched_in()/group_sched_out() that
>>>> call event_filter_match() function filtering out groups allocated for cpus
>>>> different from the one executing the handler. Additionally for every
>>>> filtered out group group_sched_out() updates tstamps values to the current
>>>> interrupt time. This updating work is now done only once by
>>>> update_context_time() called by ctx_sched_out() before cpu groups
>>>> iteration. For this trick to work it is required that tstamps of filtered
>>>> out events would point to perf_event_context::tstamp_data object instead
>>>> of perf_event::tstamp_data ones, as it is initialized from an event
>>>> allocation. tstamps references are switched by
>>>> group_sched_in()/group_sched_out() every time a group is checked for its
>>>> suitability for currently running cpu. When a thread enters some cpu on
>>>> a context switch a long run through pinned and flexible groups is
>>>> performed by perf_event_sched_in(, mux=0) with new parameter mux set to 0
>>>> and filtered out groups tstamps are switched to
>>>> perf_event_context::tstamp_data object. Then a series of multiplexing
>>>> interrupts happens and the handler rotates the flexible groups calling
>>>> ctx_sched_out(,mux=1)/perf_event_sched_in(,mux=1) iterating over the cpu
>>>> tree lists only and avoiding long runs through the complete group lists.
>>>> This is where speedup comes from. Eventually when the thread leaves the cpu
>>>> ctx_sched_out(,mux=0) is called restoring tstamps pointers to the events'
>>>> perf_event::tstamp_data objects.
>>>
>>> This is unreadable.. Please use whitespace.
>>
>> Do you mean do NOT use whitespaces? Could you explain in more detail what
>> you mean?
> 
> No, add _more_ whitespace. Use things like paragraphs and such. Reading
> a massive blob of text like that is painful. Also use full and complete
> sentences. For example, the very first sentence:
> 
>   'cpu indexed trees for perf_event_context::pinned_groups and
>   perf_event_context::flexible_groups lists are introduced.'
> 
> feels incomplete and leaves one wondering what for etc.. And it only
> gets worse.
> 

Makes sense. Will do. Thanks.

> 
>>> Yeah, this doesn't go into a changelog. Have you _ever_ seen a changelog
>>> with such crud in?
>>>
>>
>> I have not. Could you please advise how to format this information to be
>> suitable for changelog, if it's required at all?
> 
> Don't include it. It should be fairly obvious from the diff itself what
> changed after all.
> 

Ok. Clear.