Re: [PATCH 1/2] perf: Move task_pt_regs sampling into arch code

From: Andy Lutomirski <luto@amacapital.net>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "Stephane Eranian" <eranian@google.com>,
	"Ingo Molnar" <mingo@redhat.com>, "Jiri Olsa" <jolsa@redhat.com>,
	root <chenggang.qin@gmail.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"秦承刚(承刚)" <chenggang.qcg@taobao.com>,
	"Wu Fengguang" <fengguang.wu@intel.com>,
	"Mike Galbraith" <efault@gmx.de>,
	"Namhyung Kim" <namhyung@gmail.com>,
	"Arjan van de Ven" <arjan@linux.intel.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"David Ahern" <dsahern@gmail.com>,
	"Paul Mackerras" <paulus@samba.org>,
	"秦承刚(承刚)" <chenggang.qcg@alibaba-inc.com>,
	"Yanmin Zhang" <yanmin.zhang@intel.com>
Subject: Re: [PATCH 1/2] perf: Move task_pt_regs sampling into arch code
Date: Mon, 5 Jan 2015 10:28:39 -0800	[thread overview]
Message-ID: <CALCETrXR08PnHz_yFO=i95UYyHvPVV1FT1EoW5rdGX8Ac_F-eg@mail.gmail.com> (raw)
In-Reply-To: <20150105164447.GG29390@twins.programming.kicks-ass.net>

On Mon, Jan 5, 2015 at 8:44 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Jan 05, 2015 at 08:13:49AM -0800, Andy Lutomirski wrote:
>> > Just wondering how bad it would be to fill out the actual pt_regs that
>> > was previously partially initialized?
>>
>> Bad, for at least two reasons.
>>
>> One is that we don't actually know which regs are initialized.  bx,
>> bp, r12 etc are particularly bad in this regard, due to the FORK_LIKE
>> mechanism and similar optimizations.
>
> Right, but you need to deal with that anyhow.

I cheated by assuming they're uninitialized.  If we actually wrote -1
over them and they were initialized, then we'd be screwed.

>
>> The other is that the uninitialized part of task_pt_regs can be used
>> for something else entirely.  If we have a syscall instruction
>> immediately followed by a regular interrupt, then the interrupt's
>> hardware frame will overlap task_pt_regs.  (I'm not going to claim
>> that this design is sensible, but it is what it is.  IIRC Denys
>> Vlasenko had some patches to partially clean this up.)
>
> Ah, urgh. Yes painful that.
>
>> It would be possible to rework the code to avoid an extra pt_regs
>> copy, but I don't see an obvious way to do it cleanly.
>
> Yeah, we'll see how this works, I was just curious on the exact need for
> the copy, but if as you say, the original structure might not even exist
> properly (even though we have a pointer to it) that's bad (TM).

How much freedom do we have to redesign the whole mechanism?  Instead
of recording the user state from the PMI, we could set a flag so that
we'd sample the user state before return to user mode.  This would
reliably get all of the registers, except in the case where the PMI
was part way through the return-to-user code.  In that case, we'd want
to set another flag to catch the next entry to avoid writing a bogus
sample.

I don't think we could do this without changing the ABI, though.

This would waste ~300 cycles if we set the flag from a fast-path
syscall, at least until my magic sysret optimization goes in.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC