From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.1 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 210DBC10F14 for ; Thu, 3 Oct 2019 11:52:33 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C9621215EA for ; Thu, 3 Oct 2019 11:52:32 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="poByP+b3" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C9621215EA Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 728C76B0005; Thu, 3 Oct 2019 07:52:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6B2EE6B0006; Thu, 3 Oct 2019 07:52:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 551468E0003; Thu, 3 Oct 2019 07:52:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0180.hostedemail.com [216.40.44.180]) by kanga.kvack.org (Postfix) with ESMTP id 2F0066B0005 for ; Thu, 3 Oct 2019 07:52:32 -0400 (EDT) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with SMTP id 96D38613E for ; Thu, 3 Oct 2019 11:52:31 +0000 (UTC) X-FDA: 76002310902.05.gold36_501df6e5fc549 X-HE-Tag: gold36_501df6e5fc549 X-Filterd-Recvd-Size: 7312 Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by imf06.hostedemail.com (Postfix) with ESMTP for ; Thu, 3 Oct 2019 11:52:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=merlin.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=2RsgaW4dRBY/+tPdpgEQuWJrn0m0JZs7JZKItphvsrQ=; b=poByP+b3n6HNOdDnf/qGJvHAS pRXqXpuNZ5w4zB1+HX+MqRhfIs4qQZ01AcR9SEjX50dtaei+hC1Zaj3CiApO0agSJncV9ovMLgDUw 9/uEPRGsGKaokZWwQISYwto97A6/ZRQyCqhOpAspu0qBX5fhYnphFLXbGxE6KrWlZMHmwAp6z3B4B 5IDz5zPe7k8WbDoNC6I9oDE70/AnswPYgYH2TfiNQyBjzAnQunl4O//21nwspoAuQlYWRw134hOZZ +LCm7w5mWztLLvOsBIjYGDagQrOBPTbgN7YbGIxWo6778ed9wR/5evpPxlYBSacHl6Dds3Fr+ZCe8 jqnFb5bog==; Received: from j217100.upc-j.chello.nl ([24.132.217.100] helo=noisy.programming.kicks-ass.net) by merlin.infradead.org with esmtpsa (Exim 4.92.2 #3 (Red Hat Linux)) id 1iFze6-0006QH-Dw; Thu, 03 Oct 2019 11:51:46 +0000 Received: from hirez.programming.kicks-ass.net (hirez.programming.kicks-ass.net [192.168.1.225]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by noisy.programming.kicks-ass.net (Postfix) with ESMTPS id 393FE301B59; Thu, 3 Oct 2019 13:50:52 +0200 (CEST) Received: by hirez.programming.kicks-ass.net (Postfix, from userid 1000) id 6CEBE20D8F669; Thu, 3 Oct 2019 13:51:41 +0200 (CEST) Date: Thu, 3 Oct 2019 13:51:41 +0200 From: Peter Zijlstra To: Leonardo Bras Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, kvm-ppc@vger.kernel.org, linux-arch@vger.kernel.org, linux-mm@kvack.org, Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Arnd Bergmann , "Aneesh Kumar K.V" , Christophe Leroy , Nicholas Piggin , Andrew Morton , Mahesh Salgaonkar , Reza Arbab , Santosh Sivaraj , Balbir Singh , Thomas Gleixner , Greg Kroah-Hartman , Mike Rapoport , Allison Randal , Jason Gunthorpe , Dan Williams , Vlastimil Babka , Christoph Lameter , Logan Gunthorpe , Andrey Ryabinin , Alexey Dobriyan , Souptick Joarder , Mathieu Desnoyers , Ralph Campbell , Jesper Dangaard Brouer , Jann Horn , Davidlohr Bueso , Ingo Molnar , Christian Brauner , Michal Hocko , Elena Reshetova , Roman Gushchin , Andrea Arcangeli , Al Viro , "Dmitry V. Levin" , =?iso-8859-1?B?Suly9G1l?= Glisse , Song Liu , Bartlomiej Zolnierkiewicz , Ira Weiny , "Kirill A. Shutemov" , John Hubbard , Keith Busch Subject: Re: [PATCH v5 01/11] asm-generic/pgtable: Adds generic functions to monitor lockless pgtable walks Message-ID: <20191003115141.GJ4581@hirez.programming.kicks-ass.net> References: <20191003013325.2614-1-leonardo@linux.ibm.com> <20191003013325.2614-2-leonardo@linux.ibm.com> <20191003071145.GM4536@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191003071145.GM4536@hirez.programming.kicks-ass.net> User-Agent: Mutt/1.10.1 (2018-07-13) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Oct 03, 2019 at 09:11:45AM +0200, Peter Zijlstra wrote: > On Wed, Oct 02, 2019 at 10:33:15PM -0300, Leonardo Bras wrote: > > diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h > > index 818691846c90..3043ea9812d5 100644 > > --- a/include/asm-generic/pgtable.h > > +++ b/include/asm-generic/pgtable.h > > @@ -1171,6 +1171,64 @@ static inline bool arch_has_pfn_modify_check(void) > > #endif > > #endif > > > > +#ifndef __HAVE_ARCH_LOCKLESS_PGTBL_WALK_CONTROL > > +static inline unsigned long begin_lockless_pgtbl_walk(struct mm_struct *mm) > > +{ > > + unsigned long irq_mask; > > + > > + if (IS_ENABLED(CONFIG_LOCKLESS_PAGE_TABLE_WALK_TRACKING)) > > + atomic_inc(&mm->lockless_pgtbl_walkers); > > This will not work for file backed THP. Also, this is a fairly serious > contention point all on its own. Kiryl says we have tmpfs-thp, this would be broken vs that, as would your (PowerPC) use of mm_cpumask() for that IPI. > > + /* > > + * Interrupts must be disabled during the lockless page table walk. > > + * That's because the deleting or splitting involves flushing TLBs, > > + * which in turn issues interrupts, that will block when disabled. > > + */ > > + local_irq_save(irq_mask); > > + > > + /* > > + * This memory barrier pairs with any code that is either trying to > > + * delete page tables, or split huge pages. Without this barrier, > > + * the page tables could be read speculatively outside of interrupt > > + * disabling. > > + */ > > + smp_mb(); > > I don't think this is something smp_mb() can guarantee. smp_mb() is > defined to order memory accesses, in this case the store of the old > flags vs whatever comes after this. > > It cannot (in generic) order against completion of prior instructions, > like clearing the interrupt enabled flags. > > Possibly you want barrier_nospec(). I'm still really confused about this barrier. It just doesn't make sense. If an interrupt happens before the local_irq_disable()/save(), then it will discard any and all speculation that would be in progress to handle the exception. If there isn't an interrupt (or it happens after disable) it is irrelevant. Specifically, that serialize-IPI thing wants to ensure in-progress lookups are complete, and I can't find a scenario where local_irq_disable/enable() needs additional help vs IPIs. The moment an interrupt lands it kills speculation and forces things into program-order. Did you perhaps want something like: if (IS_ENABLED(CONFIG_LOCKLESS_PAGE_TABLE_WALK_TRACKING)) { atomic_inc(&foo); smp_mb__after_atomic(); } ... if (IS_ENABLED(CONFIG_LOCKLESS_PAGE_TABLE_WALK_TRACKING)) { smp_mb__before_atomic(); atomic_dec(&foo); } To ensure everything happens inside of the increment? And I still think all that wrong, you really shouldn't need to wait on munmap().