From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 18 Apr 2019 20:41:44 +0200 From: Martin Schwidefsky Subject: Re: Linux 5.1-rc5 In-Reply-To: References: <20190415051919.GA31481@infradead.org> <20190416110906.6c773aff@mschwideX1> <20190416140658.2cb73a3f@mschwideX1> <20190417094637.51ad4c67@mschwideX1> <20190417100244.42e29736@mschwideX1> <20190418100218.0a4afd51@mschwideX1> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-Id: <20190418204144.16adf2a0@mschwideX1> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" List-Archive: To: Linus Torvalds Cc: Christoph Hellwig , linuxppc-dev@lists.ozlabs.org, Linux List Kernel Mailing , linux-s390 List-ID: On Thu, 18 Apr 2019 08:49:32 -0700 Linus Torvalds wrote: > On Thu, Apr 18, 2019 at 1:02 AM Martin Schwidefsky > wrote: > > > > The problematic lines in the generic gup code are these three: > > > > 1845: pmdp = pmd_offset(&pud, addr); > > 1888: pudp = pud_offset(&p4d, addr); > > 1916: p4dp = p4d_offset(&pgd, addr); > > > > Passing the pointer of a *copy* of a page table entry to pxd_offset() does > > not work with the page table folding on s390. > > Hmm. I wonder why. x86 too does the folding thing for the p4d and pud case. > > The folding works with the local copy just the same way it works with > the orignal value. The difference is that with the static page table folding pgd_offset() does the index calculation of the actual hardware top-level table. With dynamic page table folding as s390 is doing it, if the task does not use a 5-level page table pgd_offset() will see a pgd_index() of 0, the indexing of the actual top-level table is done later with p4d_offset(), pud_offset() or pmd_offset(). As an example, with a three level page table we have three indexes x/y/z. The common code "thinks" 5 indexing steps, with static folding the index sequence is x 0 0 y z. With dynamic folding the sequence is 0 0 x y z. By moving the first indexing operation to pgd_offset the static sequence does not add an index to a non-dereferenced pointer to a stack variable, the dynamic sequence does. > But I see that s390 does some other kind of folding and does that > addition of the p*d_index() unconditionally. > > I guess that does mean that s390 will just have to have its own walker. > > For the issue of the page refcount overflow it really isn't a huge > deal. Adding the refcount checking is simple (see the example patch I > gave for powerpc - you'll just have a couple of extra cases since you > do it all, rather than just the special hugetlb cases). > > Obviously in general it would have been nicer to share as much code as > possible, but let's not make things unnecessarily complex if s390 is > just fundamentally different.. It would have been nice to use the generic code (less bugs) but not at the price of over-complicating things. And that page table folding thing always makes my head hurt. -- blue skies, Martin. "Reality continues to ruin my life." - Calvin.