Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751140AbdL3TcS (ORCPT ); Sat, 30 Dec 2017 14:32:18 -0500 Received: from mail-pf0-f181.google.com ([209.85.192.181]:36317 "EHLO mail-pf0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750847AbdL3TcR (ORCPT ); Sat, 30 Dec 2017 14:32:17 -0500 X-Google-Smtp-Source: ACJfBotQqbF6RR3EkVHdc0VhJa/LGveu6cBXF8kcm+aVIUt6+KjaJrFuKx3WdHmgbue+gLB5dCvIvQ== Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (1.0) Subject: Re: x86/pti: smp_processor_id() called while preemptible in resume-from-sleep From: Andy Lutomirski X-Mailer: iPhone Mail (15C153) In-Reply-To: Date: Sat, 30 Dec 2017 11:32:13 -0800 Cc: Dave Hansen , Thomas Gleixner , Dominik Brodowski , Andy Lutomirski , LKML , the arch/x86 maintainers Message-Id: <6CCF28F9-CC33-412A-AC3C-208A36600D37@amacapital.net> References: <20171230132927.GA2731@light.dominikbrodowski.net> <20171230153054.GA1604@light.dominikbrodowski.net> To: Linus Torvalds Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id vBUJWMMk032379 Content-Length: 1442 Lines: 43 --Andy > On Dec 30, 2017, at 11:15 AM, Linus Torvalds wrote: > > On Sat, Dec 30, 2017 at 11:03 AM, Dave Hansen > wrote: >> On 12/30/2017 10:40 AM, Linus Torvalds wrote: >>> The __native_flush_tlb() function looks _very_ broken. >> ... >>> So I'd suggest moving the preempt_disable() up to the top of that >>> function, regardless of whether we could then remove that seemingly >>> stale TLB flush in that crazy >>> smpboot_setup/restore_warm_reset_vector() dance... >> >> If someone is calling __native_flush_tlb(), shouldn't they already be in >> a state where they can't be preempted? It's fundamentally a one-cpu >> thing, both the actual CPU TLB flush _and_ the per-cpu variables. > > Hmm. I think you're right. > >> It seems like we might want to _remove_ the explicit >> preempt_dis/enable() from here: >> >> preempt_disable(); >> native_write_cr3(__native_read_cr3()); >> preempt_enable(); >> >> and add some warnings to ensure it's disabled when we enter >> __native_flush_tlb(). > > Agreed, that would certainly also be consistent. > > The current code that disables preemption only selectively seems > insane to me. Either all or nothing, not this crazy half-way thing. Agreed. The current code is bogus. I'd rather have a warning if preemptible. I'm reasonably confident that IRQs on but preempt off is okay. > > Linus