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;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Andy Lutomirski <luto@amacapital.net>
Mime-Version: 1.0 (1.0)
Subject: Re: [patch 4/8] x86/entry: Move irq tracing on syscall entry to C-code
Date:   Sat, 29 Feb 2020 11:25:24 -0800
Message-Id: <4EFF3B04-2C8A-4D63-BB63-B5804EBFFE2F@amacapital.net>
References: <87lfolfo79.fsf@nanos.tec.linutronix.de>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Andy Lutomirski <luto@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>, x86@kernel.org,
        Steven Rostedt <rostedt@goodmis.org>,
        Brian Gerst <brgerst@gmail.com>,
        Juergen Gross <JGross@suse.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Arnd Bergmann <arnd@arndb.de>,
        "Paul E. McKenney" <paulmck@kernel.org>
In-Reply-To: <87lfolfo79.fsf@nanos.tec.linutronix.de>
To:     Thomas Gleixner <tglx@linutronix.de>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Feb 29, 2020, at 6:44 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>=20
> =EF=BB=BFThomas Gleixner <tglx@linutronix.de> writes:
>> Andy Lutomirski <luto@amacapital.net> writes:
>>>>> On Feb 26, 2020, at 12:17 AM, Peter Zijlstra <peterz@infradead.org> wr=
ote:
>>>>> =EF=BB=BFOn Tue, Feb 25, 2020 at 09:43:46PM -0800, Andy Lutomirski wro=
te:
>>>>>> Your earlier patches suggest quite strongly that tracing isn't safe
>>>>>> until enter_from_user_mode().  But trace_hardirqs_off() calls
>>>>>> trace_irq_disable_rcuidle(), which looks [0] like a tracepoint.
>>>>>>=20
>>>>>> Did you perhaps mean to do this *after* enter_from_user_mode()?
>>>>>=20
>>>>> aside from the fact that enter_from_user_mode() itself also has a
>>>>> tracepoint, the crucial detail is that we must not trace/kprobe the
>>>>> function calling this.
>>>>>=20
>>>>> Specifically for #PF, because we need read_cr2() before this. See late=
r
>>>>> patches.
>>>=20
>>> Indeed. I=E2=80=99m fine with this patch, but I still don=E2=80=99t unde=
rstand what
>>> the changelog is about.
>>=20
>> Yeah, the changelog is not really helpful. Let me fix that.
>>=20
>>> And I=E2=80=99m still rather baffled by most of the notrace annotations i=
n the
>>> series.
>>=20
>> As discussed on IRC, this might be too broad, but then I rather have the
>> actual C-entry points neither traceable nor probable in general and
>> relax this by calling functions which can be traced and probed.
>>=20
>> My rationale for this decision was that enter_from_user_mode() is marked
>> notrace/noprobe as well, so I kept the protection scope the same as we
>> had in the ASM maze which is marked noprobe already.
>=20
> I have second thoughts vs. tracing in this context.
>=20
> While the tracer itself seems to handle this correctly, what about
> things like BPF programs which can be attached to tracepoints and
> function trace entries?

I think that everything using the tracing code, including BPF, should either=
 do its own rcuidle stuff or explicitly not execute if we=E2=80=99re not in C=
ONTEXT_KERNEL.  That is, we probably need to patch BPF.

>=20
> Is that really safe _before_ context tracking has updated RCU state?
>=20
> Thanks,
>=20
>        tglx
>=20
>=20