Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [RFC][PATCH] perf: Rewrite core context handling
To:     Peter Zijlstra <peterz@infradead.org>,
        Song Liu <songliubraving@fb.com>
Cc:     Ingo Molnar <mingo@kernel.org>,
        lkml <linux-kernel@vger.kernel.org>,
        "acme@kernel.org" <acme@kernel.org>,
        "alexander.shishkin@linux.intel.com" 
        <alexander.shishkin@linux.intel.com>,
        "jolsa@redhat.com" <jolsa@redhat.com>,
        "eranian@google.com" <eranian@google.com>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "megha.dey@intel.com" <megha.dey@intel.com>,
        "frederic@kernel.org" <frederic@kernel.org>
References: <20181010104559.GO5728@hirez.programming.kicks-ass.net>
 <D2014055-9B25-4152-A582-7AD42A3837AB@fb.com>
 <20181011092913.GA9848@hirez.programming.kicks-ass.net>
From:   Alexey Budankov <alexey.budankov@linux.intel.com>
Organization: Intel Corp.
Message-ID: <162cce00-745c-6e30-f859-497d9307059d@linux.intel.com>
Date:   Fri, 12 Oct 2018 10:04:36 +0300
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <20181011092913.GA9848@hirez.programming.kicks-ass.net>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi,

On 11.10.2018 12:29, Peter Zijlstra wrote:
> On Thu, Oct 11, 2018 at 07:50:23AM +0000, Song Liu wrote:
>> Hi Peter, 
>>
>> I am trying to understand this. Pardon me if any question is silly. 
>>
>> I am not sure I fully understand the motivation here. I guess we
>> see problem when there are two (or more) independent hardware PMUs 
>> per cpu? Then on a given cpu, there are two (or more) 
>> perf_cpu_context, but only one task context? 
> 
> Right.
> 
>> If this is correct (I really doubt...), I guess perf_rotate_context()
>> is the problem? 
> 
> No, everything comes apart. Where would you put the events of the second
> PMU?
> 
> The thing most often proposed it pretending the second PMU is a
> 'software' PMU and sticking the events on the software PMU context.
> 
> But because software PMUs must never fail to schedule an event, that
> results in some quite horrible things -- including that we cannot RR the
> events.
> 
> Similarly the big.little guys have the problem that the PMUs are not the
> same between big and little cores, and they fudge something horrible. By
> having clear ordering on PMU, that can be cleaned up too.
> 
>> And if this is still correct, this patch may not help,
>> as we are doing rotation for each perf_cpu_pmu_context? (or rotation 
>> per perf_event_context is the next step?). 
> 
> We do indeed to rotation per perf_cpu_pmu_context, however:
> 
>  - perf_cpu_pmu_context embeds a cpu scope perf_event_pmu_context,
>  - perf_cpu_pmu_context tracks the currently associated task scope
>    perf_event_pmu_context.
> 
> So it can rotate all current events for a particular PMU.
> 
>> Or step back a little... I see two big changes:
>>
>> 1. struct perf_ctx_context is now per cpu (instead of per pmu per cpu);
>> 2. one perf_event_ctxp per task_struct (instead of 2).  
> 
> Correct, we reduce to 1 cpu context and 1 task context at all times.
> This in fact simplifies quite a bit of things.

And what is currently missing is 
some markup of the per cpu event list into per pmu sublists and 
capability to rotate or not rotate the sublists independently, 
right?

Thanks,
Alexey

> 
>> I think #1 is a bigger change than #2. Is this correct? 
> 
> They're the 'same' change. But yes the primary purpose was 2, but having
> only a single cpu context is a direct consequence.
> 
>> Could you please help me understand it better? 
> 
> I hope this helps to understand, please feel free to ask more.
>