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;
From:   Song Liu <songliubraving@fb.com>
To:     Peter Zijlstra <peterz@infradead.org>
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>,
        "alexey.budankov@linux.intel.com" <alexey.budankov@linux.intel.com>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "megha.dey@intel.com" <megha.dey@intel.com>,
        "frederic@kernel.org" <frederic@kernel.org>
Subject: Re: [RFC][PATCH] perf: Rewrite core context handling
Thread-Topic: [RFC][PATCH] perf: Rewrite core context handling
Thread-Index: AQHUYIaFJ/VvvMmSxkC06uIJGGK3pKUZrRIAgAAbn4CAANwqgA==
Date:   Thu, 11 Oct 2018 22:37:14 +0000
Message-ID: <70079805-1CAE-4CAA-813A-F8DDB929F22B@fb.com>
References: <20181010104559.GO5728@hirez.programming.kicks-ass.net>
 <D2014055-9B25-4152-A582-7AD42A3837AB@fb.com>
 <20181011092913.GA9848@hirez.programming.kicks-ass.net>
In-Reply-To: <20181011092913.GA9848@hirez.programming.kicks-ass.net>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: fb.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-ID: <6D66425F5E7A0240B3DA861E33DF4A9E@namprd15.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 084ed7d7-021f-4543-8334-08d62fca17ff
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Oct 2018 22:37:15.0142
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR15MB1693
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-10-11_10:,,
 signatures=0
X-Proofpoint-Spam-Reason: safe
X-FB-Internal: Safe
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Thanks Peter! These are really really helpful.=20

I am trying to think through the case of a group of two events on two=20
separate hardware PMUs. In current implementation, this will not trigger
move_group, so they will not be able to rotate together? And actually,=20
they may not be able to run at all? Maybe this case is never supported?=20

On the other hand, would something like this work:

    perf_cpu_context <-[1:2]-> perf_event_context <-[1:n]-> perf_event=20
              |                                                |
              `----[1:n]---->     pmu    <----- [1:n]----------'=20

1. Every cpu has only one perf_cpu_context. No perf_cpu_pmu_context.=20
2. perf_cpu_context has two perf_event_context, one for the cpu, the=20
   other for the task.=20
3. Each perf_event_context has 3 perf_event_groups, pinned_groups,=20
   flexible_groups, and software_groups (for sw event only groups).=20
4. All flexible_groups of the same cpu rotate a the same time. If=20
   there are two hardware PMUs on the cpu, the rotation will look=20
   like: 1) stop both PMUs; 2) rotate events; 3) start both PMUs.=20

I feel this will make the implementation simpler. Is it too broken in=20
some cases? Or did I miss anything obvious? One thing I noticed is=20
that we need to drop per PMU config perf_event_mux_interval_ms.=20

Please let me know whether this makes sense at all. I will read=20
more of current version in the meanwhile.=20

Thanks again,
Song


> On Oct 11, 2018, at 2:29 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>=20
> On Thu, Oct 11, 2018 at 07:50:23AM +0000, Song Liu wrote:
>> Hi Peter,=20
>>=20
>> I am trying to understand this. Pardon me if any question is silly.=20
>>=20
>> I am not sure I fully understand the motivation here. I guess we
>> see problem when there are two (or more) independent hardware PMUs=20
>> per cpu? Then on a given cpu, there are two (or more)=20
>> perf_cpu_context, but only one task context?=20
>=20
> Right.
>=20
>> If this is correct (I really doubt...), I guess perf_rotate_context()
>> is the problem?=20
>=20
> No, everything comes apart. Where would you put the events of the second
> PMU?
>=20
> The thing most often proposed it pretending the second PMU is a
> 'software' PMU and sticking the events on the software PMU context.
>=20
> 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.
>=20
> 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.
>=20
>> And if this is still correct, this patch may not help,
>> as we are doing rotation for each perf_cpu_pmu_context? (or rotation=20
>> per perf_event_context is the next step?).=20
>=20
> We do indeed to rotation per perf_cpu_pmu_context, however:
>=20
> - 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.
>=20
> So it can rotate all current events for a particular PMU.
>=20
>> Or step back a little... I see two big changes:
>>=20
>> 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). =20
>=20
> Correct, we reduce to 1 cpu context and 1 task context at all times.
> This in fact simplifies quite a bit of things.
>=20
>> I think #1 is a bigger change than #2. Is this correct?=20
>=20
> They're the 'same' change. But yes the primary purpose was 2, but having
> only a single cpu context is a direct consequence.
>=20
>> Could you please help me understand it better?=20
>=20
> I hope this helps to understand, please feel free to ask more.