From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x743.google.com ([2607:f8b0:4864:20::743]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kGUbF-0003a8-Jt for linux-um@lists.infradead.org; Thu, 10 Sep 2020 21:59:26 +0000 Received: by mail-qk1-x743.google.com with SMTP id d20so7777545qka.5 for ; Thu, 10 Sep 2020 14:59:24 -0700 (PDT) Date: Thu, 10 Sep 2020 18:59:21 -0300 From: Jason Gunthorpe Subject: Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding Message-ID: <20200910215921.GP87483@ziepe.ca> References: <20200909142904.00b72921@thinkpad> <20200909192534.442f8984@thinkpad> <20200909180324.GI87483@ziepe.ca> <20200910093925.GB29166@oc3871087118.ibm.com> <20200910181319.GO87483@ziepe.ca> <20200910211010.46d064a7@thinkpad> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-um" Errors-To: linux-um-bounces+geert=linux-m68k.org@lists.infradead.org To: Linus Torvalds Cc: Peter Zijlstra , Benjamin Herrenschmidt , Dave Hansen , Dave Hansen , Paul Mackerras , linux-sparc , Alexander Gordeev , Claudio Imbrenda , Will Deacon , linux-arch , linux-s390 , Vasily Gorbik , Richard Weinberger , linux-x86 , Russell King , Christian Borntraeger , Ingo Molnar , Catalin Marinas , Andrey Ryabinin , Gerald Schaefer , Heiko Carstens , Arnd Bergmann , John Hubbard , Jeff Dike , linux-um , Borislav Petkov , Andy Lutomirski , Thomas Gleixner , linux-arm , linux-mm , LKML , Michael Ellerman , Andrew Morton , linux-power , Mike Rapoport On Thu, Sep 10, 2020 at 12:32:05PM -0700, Linus Torvalds wrote: > Yeah, I get hung up on naming sometimes. I don't tend to care much > about private local variables ("i" is a perfectly fine variable name), > but these kinds of somewhat subtle cross-architecture definitions I > feel matter. One of the first replys to this patch was to ask "when would I use _orig vs normal", so you are not alone. The name should convey it.. So, I suggest pXX_offset_unlocked() Since it is safe to call without the page table lock, while pXX_offset() requires the page table lock to be held as the internal *pXX is a data race otherwise. Patch 1 might be OK for a stable backport, but to get to a clear pXX_offset_unlocked() all the arches would want to be changed to implement that API and the generic code would provide the wrapper: #define pXX_offset(pXXp, address) pXX_offset_unlocked(pXXp, *(pXXp), address) Arches would not have a *pXX inside their code. Then we can talk about auditing call sites of pXX_offset and think about using the _unlocked version in places where the page table lock is not held. For instance mm/pagewalk.c should be changed. So should huge_pte_offset() and probably other places. These places might already be exsting data-race bugs. It is code-as-documentation indicating an unlocked page table walk. Now it is not just a S390 story but a change that makes the data concurrency much clearer, so I think I prefer this version to the addr_end one too. Regards, Jason _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um