[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <96b80926-cf5b-1afa-9b7a-949a2188e61f@csgroup.eu>
Date: Tue, 8 Sep 2020 14:40:10 +0200
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Christian Borntraeger <borntraeger@...ibm.com>,
Gerald Schaefer <gerald.schaefer@...ux.ibm.com>,
Jason Gunthorpe <jgg@...pe.ca>,
John Hubbard <jhubbard@...dia.com>
Cc: Peter Zijlstra <peterz@...radead.org>,
Dave Hansen <dave.hansen@...ux.intel.com>,
linux-mm <linux-mm@...ck.org>, Paul Mackerras <paulus@...ba.org>,
linux-sparc <sparclinux@...r.kernel.org>,
Alexander Gordeev <agordeev@...ux.ibm.com>,
Claudio Imbrenda <imbrenda@...ux.ibm.com>,
Will Deacon <will@...nel.org>,
linux-arch <linux-arch@...r.kernel.org>,
linux-s390 <linux-s390@...r.kernel.org>,
Vasily Gorbik <gor@...ux.ibm.com>,
Richard Weinberger <richard@....at>,
linux-x86 <x86@...nel.org>, Russell King <linux@...linux.org.uk>,
Ingo Molnar <mingo@...hat.com>,
Catalin Marinas <catalin.marinas@....com>,
Andrey Ryabinin <aryabinin@...tuozzo.com>,
Heiko Carstens <hca@...ux.ibm.com>,
Arnd Bergmann <arnd@...db.de>, Jeff Dike <jdike@...toit.com>,
linux-um <linux-um@...ts.infradead.org>,
Borislav Petkov <bp@...en8.de>,
Andy Lutomirski <luto@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
linux-arm <linux-arm-kernel@...ts.infradead.org>,
linux-power <linuxppc-dev@...ts.ozlabs.org>,
LKML <linux-kernel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Mike Rapoport <rppt@...nel.org>
Subject: Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table
folding
Le 08/09/2020 à 14:09, Christian Borntraeger a écrit :
>
>
> On 08.09.20 07:06, Christophe Leroy wrote:
>>
>>
>> Le 07/09/2020 à 20:00, Gerald Schaefer a écrit :
>>> From: Alexander Gordeev <agordeev@...ux.ibm.com>
>>>
>>> Commit 1a42010cdc26 ("s390/mm: convert to the generic get_user_pages_fast
>>> code") introduced a subtle but severe bug on s390 with gup_fast, due to
>>> dynamic page table folding.
>>>
>>> The question "What would it require for the generic code to work for s390"
>>> has already been discussed here
>>> https://lkml.kernel.org/r/20190418100218.0a4afd51@mschwideX1
>>> and ended with a promising approach here
>>> https://lkml.kernel.org/r/20190419153307.4f2911b5@mschwideX1
>>> which in the end unfortunately didn't quite work completely.
>>>
>>> We tried to mimic static level folding by changing pgd_offset to always
>>> calculate top level page table offset, and do nothing in folded pXd_offset.
>>> What has been overlooked is that PxD_SIZE/MASK and thus pXd_addr_end do
>>> not reflect this dynamic behaviour, and still act like static 5-level
>>> page tables.
>>>
>>
>> [...]
>>
>>>
>>> Fix this by introducing new pXd_addr_end_folded helpers, which take an
>>> additional pXd entry value parameter, that can be used on s390
>>> to determine the correct page table level and return corresponding
>>> end / boundary. With that, the pointer iteration will always
>>> happen in gup_pgd_range for s390. No change for other architectures
>>> introduced.
>>
>> Not sure pXd_addr_end_folded() is the best understandable name, allthough I don't have any alternative suggestion at the moment.
>> Maybe could be something like pXd_addr_end_fixup() as it will disappear in the next patch, or pXd_addr_end_gup() ?
>>
>> Also, if it happens to be acceptable to get patch 2 in stable, I think you should switch patch 1 and patch 2 to avoid the step through pXd_addr_end_folded()
>
> given that this fixes a data corruption issue, wouldnt it be the best to go forward
> with this patch ASAP and then handle the other patches on top with all the time that
> we need?
I have no strong opinion on this, but I feel rather tricky to have to
change generic part of GUP to use a new fonction then revert that change
in the following patch, just because you want the first patch in stable
and not the second one.
Regardless, I was wondering, why do we need a reference to the pXd at
all when calling pXd_addr_end() ?
Couldn't S390 retrieve the pXd by using the pXd_offset() dance with the
passed addr ?
Christophe
Powered by blists - more mailing lists