* Re: [PATCH 2/2] powerpc/vdso32: link vdso64 with linker
From: Christophe Leroy @ 2020-09-02 15:43 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Christophe Leroy, Joe Lawrence, Kees Cook, Fangrui Song,
Nick Desaulniers, linux-kernel, Nicholas Piggin,
clang-built-linux, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200902141431.GV28786@gate.crashing.org>
Hi,
Le 02/09/2020 à 16:14, Segher Boessenkool a écrit :
> Hi!
>
> On Wed, Sep 02, 2020 at 06:46:45AM +0000, Christophe Leroy wrote:
>> ld crashes:
>>
>> LD arch/powerpc/kernel/vdso32/vdso32.so.dbg
>> /bin/sh: line 1: 23780 Segmentation fault (core dumped)
>> ppc-linux-ld -EB -m elf32ppc -shared -soname linux-vdso32.so.1
>> --eh-frame-hdr --orphan-handling=warn -T
>> arch/powerpc/kernel/vdso32/vdso32.lds
>> arch/powerpc/kernel/vdso32/sigtramp.o
>> arch/powerpc/kernel/vdso32/gettimeofday.o
>> arch/powerpc/kernel/vdso32/datapage.o
>> arch/powerpc/kernel/vdso32/cacheflush.o
>> arch/powerpc/kernel/vdso32/note.o arch/powerpc/kernel/vdso32/getcpu.o -o
>> arch/powerpc/kernel/vdso32/vdso32.so.dbg
>> make[4]: *** [arch/powerpc/kernel/vdso32/vdso32.so.dbg] Error 139
>>
>>
>> [root@localhost linux-powerpc]# ppc-linux-ld --version
>> GNU ld (GNU Binutils) 2.26.20160125
>
> [ Don't build as root :-P ]
>
> Try with a newer ld? If it still happens with current versions, please
> open a bug report? https://sourceware.org/bugzilla
Yes it works with 2.30 and 2.34
But minimum for building kernel is supposed to be 2.23
Christophe
^ permalink raw reply
* Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: Christophe Leroy @ 2020-09-02 15:17 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, Kees Cook, x86, linuxppc-dev, linux-kernel, Al Viro,
linux-fsdevel, Linus Torvalds
In-Reply-To: <20200902123646.GA31184@lst.de>
Le 02/09/2020 à 14:36, Christoph Hellwig a écrit :
> On Wed, Sep 02, 2020 at 08:15:12AM +0200, Christophe Leroy wrote:
>>> - return 0;
>>> - return (size == 0 || size - 1 <= seg.seg - addr);
>>> + if (addr >= TASK_SIZE_MAX)
>>> + return false;
>>> + if (size == 0)
>>> + return false;
>>
>> __access_ok() was returning true when size == 0 up to now. Any reason to
>> return false now ?
>
> No, this is accidental and broken. Can you re-run your benchmark with
> this fixed?
>
With this fix, I get
root@vgoippro:~# time dd if=/dev/zero of=/dev/null count=1M
1048576+0 records in
1048576+0 records out
536870912 bytes (512.0MB) copied, 6.776327 seconds, 75.6MB/s
real 0m 6.78s
user 0m 1.64s
sys 0m 5.13s
That's still far from the 91.7MB/s I get with 5.9-rc2, but better than
the 65.8MB/s I got yesterday with your series. Still some way to go thought.
Christophe
^ permalink raw reply
* RE: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: David Laight @ 2020-09-02 15:02 UTC (permalink / raw)
To: 'Christophe Leroy', 'Christoph Hellwig'
Cc: linux-arch@vger.kernel.org, Kees Cook, x86@kernel.org,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
Al Viro, linux-fsdevel@vger.kernel.org, Linus Torvalds
In-Reply-To: <6e88048a-8b30-400e-11c6-8d91ba77cbb0@csgroup.eu>
From: Christophe Leroy
> Sent: 02 September 2020 15:13
>
>
> Le 02/09/2020 à 15:51, David Laight a écrit :
> > From: Christophe Leroy
> >> Sent: 02 September 2020 14:25
> >> Le 02/09/2020 à 15:13, David Laight a écrit :
> >>> From: Christoph Hellwig
> >>>> Sent: 02 September 2020 13:37
> >>>>
> >>>> On Wed, Sep 02, 2020 at 08:15:12AM +0200, Christophe Leroy wrote:
> >>>>>> - return 0;
> >>>>>> - return (size == 0 || size - 1 <= seg.seg - addr);
> >>>>>> + if (addr >= TASK_SIZE_MAX)
> >>>>>> + return false;
> >>>>>> + if (size == 0)
> >>>>>> + return false;
> >>>>>
> >>>>> __access_ok() was returning true when size == 0 up to now. Any reason to
> >>>>> return false now ?
> >>>>
> >>>> No, this is accidental and broken. Can you re-run your benchmark with
> >>>> this fixed?
> >>>
> >>> Is TASK_SIZE_MASK defined such that you can do:
> >>>
> >>> return (addr | size) < TASK_SIZE_MAX) || !size;
> >>
> >> TASK_SIZE_MAX will usually be 0xc0000000
> >>
> >> With:
> >> addr = 0x80000000;
> >> size = 0x80000000;
> >>
> >> I expect it to fail ....
> >>
> >> With the formula you propose it will succeed, won't it ?
> >
> > Hmmm... Was i getting confused about some comments for 64bit
> > about there being such a big hole between valid user and kernel
> > addresses that it was enough to check that 'size < TASK_SIZE_MAX'.
> >
> > That would be true for 64bit x86 (and probably ppc (& arm??))
> > if TASK_SIZE_MAX were 0x4 << 60.
> > IIUC the highest user address is (much) less than 0x0 << 60
> > and the lowest kernel address (much) greater than 0xf << 60
> > on all these 64bit platforms.
> >
> > Actually if doing access_ok() inside get_user() you don't
> > need to check the size at all.
>
> You mean on 64 bit or on any platform ?
64bit and 32bit
> What about a word write to 0xbffffffe, won't it overwrite 0xc0000000 ?
>
> > You don't even need to in copy_to/from_user() provided
> > it always does a forwards copy.
>
> Do you mean due to the gap ?
> Is it garantied to be a gap ? Even on a 32 bits having TASK_SIZE set to
> 0xc0000000 and PAGE_OFFSET set to the same ?
I read somewhere (I won't find it again) that the last 4k page
(below 0xc0000000) must not be allocated on i386 because some
cpu (both intel and amd) do 'horrid things' if they try to
(IIRC) do instruction prefetches across the boundary.
So the accesses to 0xbffffffe will fault and the one to 0xc0000000
won't happen (in any useful way at least).
I'd suspect that not allocating the 3G-4k page would be a safe
bet on all architectures - even 68k.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* [Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
From: bugzilla-daemon @ 2020-09-02 14:45 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206203-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206203
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #288187|0 |1
is obsolete| |
--- Comment #20 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 292289
--> https://bugzilla.kernel.org/attachment.cgi?id=292289&action=edit
kernel .config (kernel 5.9-rc3, Talos II)
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
From: bugzilla-daemon @ 2020-09-02 14:39 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206203-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206203
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #288189|0 |1
is obsolete| |
--- Comment #19 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 292287
--> https://bugzilla.kernel.org/attachment.cgi?id=292287&action=edit
kmemleak output (kernel 5.9-rc3, Talos II)
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
From: bugzilla-daemon @ 2020-09-02 14:38 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206203-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206203
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #288185|0 |1
is obsolete| |
--- Comment #18 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 292285
--> https://bugzilla.kernel.org/attachment.cgi?id=292285&action=edit
dmesg (kernel 5.9-rc3, Talos II)
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/vdso32: link vdso64 with linker
From: Segher Boessenkool @ 2020-09-02 14:14 UTC (permalink / raw)
To: Christophe Leroy
Cc: Christophe Leroy, Joe Lawrence, Kees Cook, Fangrui Song,
Nick Desaulniers, linux-kernel, Nicholas Piggin,
clang-built-linux, Paul Mackerras, linuxppc-dev
In-Reply-To: <b2066ccd-2b81-6032-08e3-41105b400f75@csgroup.eu>
Hi!
On Wed, Sep 02, 2020 at 06:46:45AM +0000, Christophe Leroy wrote:
> ld crashes:
>
> LD arch/powerpc/kernel/vdso32/vdso32.so.dbg
> /bin/sh: line 1: 23780 Segmentation fault (core dumped)
> ppc-linux-ld -EB -m elf32ppc -shared -soname linux-vdso32.so.1
> --eh-frame-hdr --orphan-handling=warn -T
> arch/powerpc/kernel/vdso32/vdso32.lds
> arch/powerpc/kernel/vdso32/sigtramp.o
> arch/powerpc/kernel/vdso32/gettimeofday.o
> arch/powerpc/kernel/vdso32/datapage.o
> arch/powerpc/kernel/vdso32/cacheflush.o
> arch/powerpc/kernel/vdso32/note.o arch/powerpc/kernel/vdso32/getcpu.o -o
> arch/powerpc/kernel/vdso32/vdso32.so.dbg
> make[4]: *** [arch/powerpc/kernel/vdso32/vdso32.so.dbg] Error 139
>
>
> [root@localhost linux-powerpc]# ppc-linux-ld --version
> GNU ld (GNU Binutils) 2.26.20160125
[ Don't build as root :-P ]
Try with a newer ld? If it still happens with current versions, please
open a bug report? https://sourceware.org/bugzilla
Segher
^ permalink raw reply
* Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: Christophe Leroy @ 2020-09-02 14:12 UTC (permalink / raw)
To: David Laight, 'Christoph Hellwig'
Cc: linux-arch@vger.kernel.org, Kees Cook, x86@kernel.org,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
Al Viro, linux-fsdevel@vger.kernel.org, Linus Torvalds
In-Reply-To: <0c298e0d972a48bd9ee178225e404b12@AcuMS.aculab.com>
Le 02/09/2020 à 15:51, David Laight a écrit :
> From: Christophe Leroy
>> Sent: 02 September 2020 14:25
>> Le 02/09/2020 à 15:13, David Laight a écrit :
>>> From: Christoph Hellwig
>>>> Sent: 02 September 2020 13:37
>>>>
>>>> On Wed, Sep 02, 2020 at 08:15:12AM +0200, Christophe Leroy wrote:
>>>>>> - return 0;
>>>>>> - return (size == 0 || size - 1 <= seg.seg - addr);
>>>>>> + if (addr >= TASK_SIZE_MAX)
>>>>>> + return false;
>>>>>> + if (size == 0)
>>>>>> + return false;
>>>>>
>>>>> __access_ok() was returning true when size == 0 up to now. Any reason to
>>>>> return false now ?
>>>>
>>>> No, this is accidental and broken. Can you re-run your benchmark with
>>>> this fixed?
>>>
>>> Is TASK_SIZE_MASK defined such that you can do:
>>>
>>> return (addr | size) < TASK_SIZE_MAX) || !size;
>>
>> TASK_SIZE_MAX will usually be 0xc0000000
>>
>> With:
>> addr = 0x80000000;
>> size = 0x80000000;
>>
>> I expect it to fail ....
>>
>> With the formula you propose it will succeed, won't it ?
>
> Hmmm... Was i getting confused about some comments for 64bit
> about there being such a big hole between valid user and kernel
> addresses that it was enough to check that 'size < TASK_SIZE_MAX'.
>
> That would be true for 64bit x86 (and probably ppc (& arm??))
> if TASK_SIZE_MAX were 0x4 << 60.
> IIUC the highest user address is (much) less than 0x0 << 60
> and the lowest kernel address (much) greater than 0xf << 60
> on all these 64bit platforms.
>
> Actually if doing access_ok() inside get_user() you don't
> need to check the size at all.
You mean on 64 bit or on any platform ?
What about a word write to 0xbffffffe, won't it overwrite 0xc0000000 ?
> You don't even need to in copy_to/from_user() provided
> it always does a forwards copy.
Do you mean due to the gap ?
Is it garantied to be a gap ? Even on a 32 bits having TASK_SIZE set to
0xc0000000 and PAGE_OFFSET set to the same ?
Christophe
^ permalink raw reply
* RE: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: David Laight @ 2020-09-02 13:51 UTC (permalink / raw)
To: 'Christophe Leroy', 'Christoph Hellwig'
Cc: linux-arch@vger.kernel.org, Kees Cook, x86@kernel.org,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
Al Viro, linux-fsdevel@vger.kernel.org, Linus Torvalds
In-Reply-To: <8de54fe0-4be9-5624-dd1d-d95d792e933d@csgroup.eu>
From: Christophe Leroy
> Sent: 02 September 2020 14:25
> Le 02/09/2020 à 15:13, David Laight a écrit :
> > From: Christoph Hellwig
> >> Sent: 02 September 2020 13:37
> >>
> >> On Wed, Sep 02, 2020 at 08:15:12AM +0200, Christophe Leroy wrote:
> >>>> - return 0;
> >>>> - return (size == 0 || size - 1 <= seg.seg - addr);
> >>>> + if (addr >= TASK_SIZE_MAX)
> >>>> + return false;
> >>>> + if (size == 0)
> >>>> + return false;
> >>>
> >>> __access_ok() was returning true when size == 0 up to now. Any reason to
> >>> return false now ?
> >>
> >> No, this is accidental and broken. Can you re-run your benchmark with
> >> this fixed?
> >
> > Is TASK_SIZE_MASK defined such that you can do:
> >
> > return (addr | size) < TASK_SIZE_MAX) || !size;
>
> TASK_SIZE_MAX will usually be 0xc0000000
>
> With:
> addr = 0x80000000;
> size = 0x80000000;
>
> I expect it to fail ....
>
> With the formula you propose it will succeed, won't it ?
Hmmm... Was i getting confused about some comments for 64bit
about there being such a big hole between valid user and kernel
addresses that it was enough to check that 'size < TASK_SIZE_MAX'.
That would be true for 64bit x86 (and probably ppc (& arm??))
if TASK_SIZE_MAX were 0x4 << 60.
IIUC the highest user address is (much) less than 0x0 << 60
and the lowest kernel address (much) greater than 0xf << 60
on all these 64bit platforms.
Actually if doing access_ok() inside get_user() you don't
need to check the size at all.
You don't even need to in copy_to/from_user() provided
it always does a forwards copy.
(Rather that copying the last word first for misaligned lengths.)
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: Christophe Leroy @ 2020-09-02 13:24 UTC (permalink / raw)
To: David Laight, 'Christoph Hellwig'
Cc: linux-arch@vger.kernel.org, Kees Cook, x86@kernel.org,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
Al Viro, linux-fsdevel@vger.kernel.org, Linus Torvalds
In-Reply-To: <61b9a880a6424a34b841cf3dddb463ad@AcuMS.aculab.com>
Le 02/09/2020 à 15:13, David Laight a écrit :
> From: Christoph Hellwig
>> Sent: 02 September 2020 13:37
>>
>> On Wed, Sep 02, 2020 at 08:15:12AM +0200, Christophe Leroy wrote:
>>>> - return 0;
>>>> - return (size == 0 || size - 1 <= seg.seg - addr);
>>>> + if (addr >= TASK_SIZE_MAX)
>>>> + return false;
>>>> + if (size == 0)
>>>> + return false;
>>>
>>> __access_ok() was returning true when size == 0 up to now. Any reason to
>>> return false now ?
>>
>> No, this is accidental and broken. Can you re-run your benchmark with
>> this fixed?
>
> Is TASK_SIZE_MASK defined such that you can do:
>
> return (addr | size) < TASK_SIZE_MAX) || !size;
TASK_SIZE_MAX will usually be 0xc0000000
With:
addr = 0x80000000;
size = 0x80000000;
I expect it to fail ....
With the formula you propose it will succeed, won't it ?
Christophe
^ permalink raw reply
* Re: [PATCH v3 12/13] mm/debug_vm_pgtable/hugetlb: Disable hugetlb test on ppc64
From: Aneesh Kumar K.V @ 2020-09-02 13:20 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm, akpm
Cc: linux-arch, linux-s390, Christophe Leroy, x86, Mike Rapoport,
Qian Cai, Gerald Schaefer, Vineet Gupta, linux-snps-arc,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <a76a180b-650c-c868-7a52-593afe97eab3@arm.com>
Anshuman Khandual <anshuman.khandual@arm.com> writes:
> On 09/01/2020 12:00 PM, Aneesh Kumar K.V wrote:
>> On 9/1/20 9:33 AM, Anshuman Khandual wrote:
>>>
>>>
>>> On 08/27/2020 01:34 PM, Aneesh Kumar K.V wrote:
>>>> The seems to be missing quite a lot of details w.r.t allocating
>>>> the correct pgtable_t page (huge_pte_alloc()), holding the right
>>>> lock (huge_pte_lock()) etc. The vma used is also not a hugetlb VMA.
>>>>
>>>> ppc64 do have runtime checks within CONFIG_DEBUG_VM for most of these.
>>>> Hence disable the test on ppc64.
>>>
>>> Would really like this to get resolved in an uniform and better way
>>> instead, i.e a modified hugetlb_advanced_tests() which works on all
>>> platforms including ppc64.
>>>
>>> In absence of a modified version, I do realize the situation here,
>>> where DEBUG_VM_PGTABLE test either runs on ppc64 or just completely
>>> remove hugetlb_advanced_tests() from other platforms as well.
>>>
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>>> ---
>>>> mm/debug_vm_pgtable.c | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>>>> index a188b6e4e37e..21329c7d672f 100644
>>>> --- a/mm/debug_vm_pgtable.c
>>>> +++ b/mm/debug_vm_pgtable.c
>>>> @@ -813,6 +813,7 @@ static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot)
>>>> #endif /* CONFIG_ARCH_WANT_GENERAL_HUGETLB */
>>>> }
>>>> +#ifndef CONFIG_PPC_BOOK3S_64
>>>> static void __init hugetlb_advanced_tests(struct mm_struct *mm,
>>>> struct vm_area_struct *vma,
>>>> pte_t *ptep, unsigned long pfn,
>>>> @@ -855,6 +856,7 @@ static void __init hugetlb_advanced_tests(struct mm_struct *mm,
>>>> pte = huge_ptep_get(ptep);
>>>> WARN_ON(!(huge_pte_write(pte) && huge_pte_dirty(pte)));
>>>> }
>>>> +#endif
>>>
>>> In the worst case if we could not get a new hugetlb_advanced_tests() test
>>> that works on all platforms, this might be the last fallback option. In
>>> which case, it will require a proper comment section with a "FIXME: ",
>>> explaining the current situation (and that #ifdef is temporary in nature)
>>> and a hugetlb_advanced_tests() stub when CONFIG_PPC_BOOK3S_64 is enabled.
>>>
>>>> #else /* !CONFIG_HUGETLB_PAGE */
>>>> static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot) { }
>>>> static void __init hugetlb_advanced_tests(struct mm_struct *mm,
>>>> @@ -1065,7 +1067,9 @@ static int __init debug_vm_pgtable(void)
>>>> pud_populate_tests(mm, pudp, saved_pmdp);
>>>> spin_unlock(ptl);
>>>> +#ifndef CONFIG_PPC_BOOK3S_64
>>>> hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
>>>> +#endif
>>>
>>
>> I actually wanted to add #ifdef BROKEN. That test is completely broken. Infact I would suggest to remove that test completely.
>>
>>
>>
>>> #ifdef will not be required here as there would be a stub definition
>>> for hugetlb_advanced_tests() when CONFIG_PPC_BOOK3S_64 is enabled.
>>>
>>>> spin_lock(&mm->page_table_lock);
>>>> p4d_clear_tests(mm, p4dp);
>>>>
>>>
>>> But again, we should really try and avoid taking this path.
>>>
>>
>> To be frank i am kind of frustrated with how this patch series is being looked at. We pushed a completely broken test to upstream and right now we have a code in upstream that crash when booted on ppc64. My attempt has been to make progress here and you definitely seems to be not in agreement to that.
>>
>
> I am afraid, this does not accurately represent the situation.
>
> - The second set patch series got merged in it's V5 after accommodating almost
> all reviews and objections during previous discussion cycles. For a complete
> development log, please refer https://patchwork.kernel.org/cover/11658627/.
>
> - The series has been repeatedly tested on arm64 and x86 platforms for multiple
> configurations but build tested on all other enabled platforms. I have always
> been dependent on voluntary help from folks on the list to get this tested on
> other enabled platforms as I dont have access to such systems. Always assumed
> that is the way to go for anything which runs on multiple platforms. So, am I
> expected to test on platforms that I dont have access to ? But I am ready to
> be corrected here, if the community protocol is not what I have always assumed
> it to be.
>
> - Each and every version of the series had appropriately copied all the enabled
> platform's mailing list. Also, I had explicitly asked for volunteers to test
> this out on platforms apart from x86 and arm64. We had positive response from
> all platforms i.e arc, s390, ppc32 but except for ppc64.
>
> https://patchwork.kernel.org/cover/11644771/
> https://patchwork.kernel.org/cover/11603713/
>
> - The development cycle provided sufficient time window for any detailed review
> and test. I have always been willing to address almost all the issues brought
> forward during these discussions. From past experience on this test, there is
> an inherent need to understand platform specific details while trying to come
> up with something generic enough that works on all platforms. It necessitates
> participation from relevant folks to enable this test on a given platform. We
> were able to enable this on arm64, x86, arc, s390, powerpc following a similar
> model.
>
> - I have to disagree here that the concerned test i.e hugetlb_advanced_tests()
> is completely broken. As mentioned before, the idea here has always been to
> emulate enough MM objects, so that a given page table helper could be tested.
> hugetlb_advanced_tests() seems to be insufficient on ppc64 platform causing it
> to crash, which is not the case on other platforms. But it is not perfect and
> can be improved upon. Given the constraints i.e limited emulation of objects,
> the test tries to do the right thing. Calling it broken is not an appropriate
> description.
>
None of the fixes done here are specific to ppc64. I am not sure why you
keep suggesting ppc64 specific issues. One should not do page table
updates without holding locks. A hugetlb pte updates expect a vma marked
hugetlb.
As explained in the patch, I see very little value in a bunch of tests
like this and the only reason I started to fix this up is because of
multiple crash reports on ppc64.
Considering the hugetlb tests require much larger change and as it is
currently written is broken, I wanted to remove that test and let you
come up with a proper test. But since you had it "working", I disabled
this only on ppc64.
But you keep suggesting that the hugetlb test need to be fixed as part
of the patch series review. I don't have enough motivation to fix that,
because I don't see much value in a bunch of tests like these. As shown
already these tests already reported success till now without even
following any page table update rules.
-aneesh
^ permalink raw reply
* RE: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: David Laight @ 2020-09-02 13:13 UTC (permalink / raw)
To: 'Christoph Hellwig', Christophe Leroy
Cc: linux-arch@vger.kernel.org, Kees Cook, x86@kernel.org,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
Al Viro, linux-fsdevel@vger.kernel.org, Linus Torvalds
In-Reply-To: <20200902123646.GA31184@lst.de>
From: Christoph Hellwig
> Sent: 02 September 2020 13:37
>
> On Wed, Sep 02, 2020 at 08:15:12AM +0200, Christophe Leroy wrote:
> >> - return 0;
> >> - return (size == 0 || size - 1 <= seg.seg - addr);
> >> + if (addr >= TASK_SIZE_MAX)
> >> + return false;
> >> + if (size == 0)
> >> + return false;
> >
> > __access_ok() was returning true when size == 0 up to now. Any reason to
> > return false now ?
>
> No, this is accidental and broken. Can you re-run your benchmark with
> this fixed?
Is TASK_SIZE_MASK defined such that you can do:
return (addr | size) < TASK_SIZE_MAX) || !size;
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH v3 12/13] mm/debug_vm_pgtable/hugetlb: Disable hugetlb test on ppc64
From: Anshuman Khandual @ 2020-09-02 13:05 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm
Cc: linux-arch, linux-s390, Christophe Leroy, x86, Mike Rapoport,
Qian Cai, Gerald Schaefer, Vineet Gupta, linux-snps-arc,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <68f90b44-b830-58be-3c21-424fee05da37@linux.ibm.com>
On 09/01/2020 12:00 PM, Aneesh Kumar K.V wrote:
> On 9/1/20 9:33 AM, Anshuman Khandual wrote:
>>
>>
>> On 08/27/2020 01:34 PM, Aneesh Kumar K.V wrote:
>>> The seems to be missing quite a lot of details w.r.t allocating
>>> the correct pgtable_t page (huge_pte_alloc()), holding the right
>>> lock (huge_pte_lock()) etc. The vma used is also not a hugetlb VMA.
>>>
>>> ppc64 do have runtime checks within CONFIG_DEBUG_VM for most of these.
>>> Hence disable the test on ppc64.
>>
>> Would really like this to get resolved in an uniform and better way
>> instead, i.e a modified hugetlb_advanced_tests() which works on all
>> platforms including ppc64.
>>
>> In absence of a modified version, I do realize the situation here,
>> where DEBUG_VM_PGTABLE test either runs on ppc64 or just completely
>> remove hugetlb_advanced_tests() from other platforms as well.
>>
>>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>> ---
>>> mm/debug_vm_pgtable.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>>> index a188b6e4e37e..21329c7d672f 100644
>>> --- a/mm/debug_vm_pgtable.c
>>> +++ b/mm/debug_vm_pgtable.c
>>> @@ -813,6 +813,7 @@ static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot)
>>> #endif /* CONFIG_ARCH_WANT_GENERAL_HUGETLB */
>>> }
>>> +#ifndef CONFIG_PPC_BOOK3S_64
>>> static void __init hugetlb_advanced_tests(struct mm_struct *mm,
>>> struct vm_area_struct *vma,
>>> pte_t *ptep, unsigned long pfn,
>>> @@ -855,6 +856,7 @@ static void __init hugetlb_advanced_tests(struct mm_struct *mm,
>>> pte = huge_ptep_get(ptep);
>>> WARN_ON(!(huge_pte_write(pte) && huge_pte_dirty(pte)));
>>> }
>>> +#endif
>>
>> In the worst case if we could not get a new hugetlb_advanced_tests() test
>> that works on all platforms, this might be the last fallback option. In
>> which case, it will require a proper comment section with a "FIXME: ",
>> explaining the current situation (and that #ifdef is temporary in nature)
>> and a hugetlb_advanced_tests() stub when CONFIG_PPC_BOOK3S_64 is enabled.
>>
>>> #else /* !CONFIG_HUGETLB_PAGE */
>>> static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot) { }
>>> static void __init hugetlb_advanced_tests(struct mm_struct *mm,
>>> @@ -1065,7 +1067,9 @@ static int __init debug_vm_pgtable(void)
>>> pud_populate_tests(mm, pudp, saved_pmdp);
>>> spin_unlock(ptl);
>>> +#ifndef CONFIG_PPC_BOOK3S_64
>>> hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
>>> +#endif
>>
>
> I actually wanted to add #ifdef BROKEN. That test is completely broken. Infact I would suggest to remove that test completely.
>
>
>
>> #ifdef will not be required here as there would be a stub definition
>> for hugetlb_advanced_tests() when CONFIG_PPC_BOOK3S_64 is enabled.
>>
>>> spin_lock(&mm->page_table_lock);
>>> p4d_clear_tests(mm, p4dp);
>>>
>>
>> But again, we should really try and avoid taking this path.
>>
>
> To be frank i am kind of frustrated with how this patch series is being looked at. We pushed a completely broken test to upstream and right now we have a code in upstream that crash when booted on ppc64. My attempt has been to make progress here and you definitely seems to be not in agreement to that.
>
I am afraid, this does not accurately represent the situation.
- The second set patch series got merged in it's V5 after accommodating almost
all reviews and objections during previous discussion cycles. For a complete
development log, please refer https://patchwork.kernel.org/cover/11658627/.
- The series has been repeatedly tested on arm64 and x86 platforms for multiple
configurations but build tested on all other enabled platforms. I have always
been dependent on voluntary help from folks on the list to get this tested on
other enabled platforms as I dont have access to such systems. Always assumed
that is the way to go for anything which runs on multiple platforms. So, am I
expected to test on platforms that I dont have access to ? But I am ready to
be corrected here, if the community protocol is not what I have always assumed
it to be.
- Each and every version of the series had appropriately copied all the enabled
platform's mailing list. Also, I had explicitly asked for volunteers to test
this out on platforms apart from x86 and arm64. We had positive response from
all platforms i.e arc, s390, ppc32 but except for ppc64.
https://patchwork.kernel.org/cover/11644771/
https://patchwork.kernel.org/cover/11603713/
- The development cycle provided sufficient time window for any detailed review
and test. I have always been willing to address almost all the issues brought
forward during these discussions. From past experience on this test, there is
an inherent need to understand platform specific details while trying to come
up with something generic enough that works on all platforms. It necessitates
participation from relevant folks to enable this test on a given platform. We
were able to enable this on arm64, x86, arc, s390, powerpc following a similar
model.
- I have to disagree here that the concerned test i.e hugetlb_advanced_tests()
is completely broken. As mentioned before, the idea here has always been to
emulate enough MM objects, so that a given page table helper could be tested.
hugetlb_advanced_tests() seems to be insufficient on ppc64 platform causing it
to crash, which is not the case on other platforms. But it is not perfect and
can be improved upon. Given the constraints i.e limited emulation of objects,
the test tries to do the right thing. Calling it broken is not an appropriate
description.
> At this point I am tempted to suggest we remove the DEBUG_VM_PGTABLE support on ppc64 because AFAIU it doesn't add any value.
Now that this has been proposed [1], along with it any possible urgency factor
out of the way, we could review the proposal with a more long term view making
sure that it improves existing tests and benefits all enabled platforms. Will
look forward to it.
[1] https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20200901094423.100149-1-aneesh.kumar@linux.ibm.com/
- Anshuman
^ permalink raw reply
* Re: [PATCH v4 04/13] mm/debug_vm_pgtables/hugevmap: Use the arch helper to identify huge vmap support.
From: Aneesh Kumar K.V @ 2020-09-02 12:53 UTC (permalink / raw)
To: Christophe Leroy, linux-mm, akpm; +Cc: linuxppc-dev, Anshuman Khandual
In-Reply-To: <7e88e090-e3af-b26c-ca65-3d98e8f81eb3@csgroup.eu>
On 9/2/20 6:10 PM, Christophe Leroy wrote:
>
>
> Le 02/09/2020 à 13:42, Aneesh Kumar K.V a écrit :
>> ppc64 supports huge vmap only with radix translation. Hence use arch
>> helper
>> to determine the huge vmap support.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> mm/debug_vm_pgtable.c | 14 ++++++++++++--
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>> index 00649b47f6e0..4c73e63b4ceb 100644
>> --- a/mm/debug_vm_pgtable.c
>> +++ b/mm/debug_vm_pgtable.c
>> @@ -28,6 +28,7 @@
>> #include <linux/swapops.h>
>> #include <linux/start_kernel.h>
>> #include <linux/sched/mm.h>
>> +#include <linux/io.h>
>> #include <asm/pgalloc.h>
>> #include <asm/tlbflush.h>
>> @@ -206,11 +207,12 @@ static void __init pmd_leaf_tests(unsigned long
>> pfn, pgprot_t prot)
>> WARN_ON(!pmd_leaf(pmd));
>> }
>> +#ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
>> static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn,
>> pgprot_t prot)
>> {
>> pmd_t pmd;
>> - if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMAP))
>> + if (!arch_ioremap_pmd_supported())
>
> What about moving ioremap_pmd_enabled() from mm/ioremap.c to some .h,
> and using it ?
> As ioremap_pmd_enabled() is defined at all time, no need of #ifdef
>
yes. This was discussed earlier too. IMHO we should do that outside this
series. I guess figuring out ioremap_pmd/pud support can definitely be
simplified. With a generic version like
#ifndef arch_ioremap_pmd_supported
static inline bool arch_ioremap_pmd_supported(void)
{
return false;
}
#endif
>> return;
>> pr_debug("Validating PMD huge\n");
>> @@ -224,6 +226,9 @@ static void __init pmd_huge_tests(pmd_t *pmdp,
>> unsigned long pfn, pgprot_t prot)
>> pmd = READ_ONCE(*pmdp);
>> WARN_ON(!pmd_none(pmd));
>> }
>> +#else /* CONFIG_HAVE_ARCH_HUGE_VMAP */
>> +static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn,
>> pgprot_t prot) { }
>> +#endif /* CONFIG_HAVE_ARCH_HUGE_VMAP */
>> static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t
>> prot)
>> {
>> @@ -320,11 +325,12 @@ static void __init pud_leaf_tests(unsigned long
>> pfn, pgprot_t prot)
>> WARN_ON(!pud_leaf(pud));
>> }
>> +#ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
>> static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn,
>> pgprot_t prot)
>> {
>> pud_t pud;
>> - if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMAP))
>> + if (!arch_ioremap_pud_supported())
>
> What about moving ioremap_pud_enabled() from mm/ioremap.c to some .h,
> and using it ?
> As ioremap_pud_enabled() is defined at all time, no need of #ifdef
>
>> return;
>> pr_debug("Validating PUD huge\n");
>> @@ -338,6 +344,10 @@ static void __init pud_huge_tests(pud_t *pudp,
>> unsigned long pfn, pgprot_t prot)
>> pud = READ_ONCE(*pudp);
>> WARN_ON(!pud_none(pud));
>> }
>> +#else /* !CONFIG_HAVE_ARCH_HUGE_VMAP */
>> +static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn,
>> pgprot_t prot) { }
>> +#endif /* !CONFIG_HAVE_ARCH_HUGE_VMAP */
>> +
>> #else /* !CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */
>> static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot)
>> { }
>> static void __init pud_advanced_tests(struct mm_struct *mm,
>>
>
> Christophe
^ permalink raw reply
* Re: [PATCH v4 04/13] mm/debug_vm_pgtables/hugevmap: Use the arch helper to identify huge vmap support.
From: Christophe Leroy @ 2020-09-02 12:40 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev, Anshuman Khandual
In-Reply-To: <20200902114222.181353-5-aneesh.kumar@linux.ibm.com>
Le 02/09/2020 à 13:42, Aneesh Kumar K.V a écrit :
> ppc64 supports huge vmap only with radix translation. Hence use arch helper
> to determine the huge vmap support.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/debug_vm_pgtable.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 00649b47f6e0..4c73e63b4ceb 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -28,6 +28,7 @@
> #include <linux/swapops.h>
> #include <linux/start_kernel.h>
> #include <linux/sched/mm.h>
> +#include <linux/io.h>
> #include <asm/pgalloc.h>
> #include <asm/tlbflush.h>
>
> @@ -206,11 +207,12 @@ static void __init pmd_leaf_tests(unsigned long pfn, pgprot_t prot)
> WARN_ON(!pmd_leaf(pmd));
> }
>
> +#ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
> static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
> {
> pmd_t pmd;
>
> - if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMAP))
> + if (!arch_ioremap_pmd_supported())
What about moving ioremap_pmd_enabled() from mm/ioremap.c to some .h,
and using it ?
As ioremap_pmd_enabled() is defined at all time, no need of #ifdef
> return;
>
> pr_debug("Validating PMD huge\n");
> @@ -224,6 +226,9 @@ static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
> pmd = READ_ONCE(*pmdp);
> WARN_ON(!pmd_none(pmd));
> }
> +#else /* CONFIG_HAVE_ARCH_HUGE_VMAP */
> +static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) { }
> +#endif /* CONFIG_HAVE_ARCH_HUGE_VMAP */
>
> static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot)
> {
> @@ -320,11 +325,12 @@ static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot)
> WARN_ON(!pud_leaf(pud));
> }
>
> +#ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
> static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
> {
> pud_t pud;
>
> - if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMAP))
> + if (!arch_ioremap_pud_supported())
What about moving ioremap_pud_enabled() from mm/ioremap.c to some .h,
and using it ?
As ioremap_pud_enabled() is defined at all time, no need of #ifdef
> return;
>
> pr_debug("Validating PUD huge\n");
> @@ -338,6 +344,10 @@ static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
> pud = READ_ONCE(*pudp);
> WARN_ON(!pud_none(pud));
> }
> +#else /* !CONFIG_HAVE_ARCH_HUGE_VMAP */
> +static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) { }
> +#endif /* !CONFIG_HAVE_ARCH_HUGE_VMAP */
> +
> #else /* !CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */
> static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
> static void __init pud_advanced_tests(struct mm_struct *mm,
>
Christophe
^ permalink raw reply
* Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: Christoph Hellwig @ 2020-09-02 12:36 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, Kees Cook, x86, linuxppc-dev, linux-kernel, Al Viro,
linux-fsdevel, Linus Torvalds, Christoph Hellwig
In-Reply-To: <8974838a-a0b1-1806-4a3a-e983deda67ca@csgroup.eu>
On Wed, Sep 02, 2020 at 08:15:12AM +0200, Christophe Leroy wrote:
>> - return 0;
>> - return (size == 0 || size - 1 <= seg.seg - addr);
>> + if (addr >= TASK_SIZE_MAX)
>> + return false;
>> + if (size == 0)
>> + return false;
>
> __access_ok() was returning true when size == 0 up to now. Any reason to
> return false now ?
No, this is accidental and broken. Can you re-run your benchmark with
this fixed?
^ permalink raw reply
* Re: [PATCH v4 02/13] powerpc/mm: Move setting pte specific flags to pfn_pte
From: Christophe Leroy @ 2020-09-02 12:30 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev, Anshuman Khandual
In-Reply-To: <20200902114222.181353-3-aneesh.kumar@linux.ibm.com>
Le 02/09/2020 à 13:42, Aneesh Kumar K.V a écrit :
> powerpc used to set the pte specific flags in set_pte_at(). This is
> different from other architectures. To be consistent with other
> architecture update pfn_pte to set _PAGE_PTE on ppc64. Also, drop now
> unused pte_mkpte.
>
> We add a VM_WARN_ON() to catch the usage of calling set_pte_at()
> without setting _PAGE_PTE bit. We will remove that after a few releases.
>
> With respect to huge pmd entries, pmd_mkhuge() takes care of adding the
> _PAGE_PTE bit.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Small nit below.
> ---
> arch/powerpc/include/asm/book3s/64/pgtable.h | 15 +++++++++------
> arch/powerpc/include/asm/nohash/pgtable.h | 5 -----
> arch/powerpc/mm/pgtable.c | 5 -----
> 3 files changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index 079211968987..2382fd516f6b 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -823,6 +818,14 @@ static inline int pte_none(pte_t pte)
> static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
> pte_t *ptep, pte_t pte, int percpu)
> {
> +
> + VM_WARN_ON(!(pte_raw(pte) & cpu_to_be64(_PAGE_PTE)));
> + /*
> + * Keep the _PAGE_PTE added till we are sure we handle _PAGE_PTE
> + * in all the callers.
> + */
> + pte = __pte_raw(pte_raw(pte) | cpu_to_be64(_PAGE_PTE));
Wrong alignment, there is a leading space.
> +
> if (radix_enabled())
> return radix__set_pte_at(mm, addr, ptep, pte, percpu);
> return hash__set_pte_at(mm, addr, ptep, pte, percpu);
Christophe
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker
From: Michael Ellerman @ 2020-09-02 12:14 UTC (permalink / raw)
To: Nick Desaulniers, Nicholas Piggin
Cc: Christophe Leroy, Joe Lawrence, Kees Cook, Fangrui Song,
Nick Desaulniers, linux-kernel, clang-built-linux, Paul Mackerras,
linuxppc-dev
In-Reply-To: <20200901222523.1941988-2-ndesaulniers@google.com>
Nick Desaulniers <ndesaulniers@google.com> writes:
> Rather than invoke the compiler as the driver, use the linker. That way
> we can check --orphan-handling=warn support correctly, as cc-ldoption
> was removed in
> commit 055efab3120b ("kbuild: drop support for cc-ldoption").
Ouch.
Seems make is quite happy to $(call deadbeef, ...) and not print a
warning, which I guess is probably a feature.
> Painstakingly compared the output between `objdump -a` before and after
> this change. Now function symbols have the correct type of FUNC rather
> than NONE, and the entry is slightly different (which doesn't matter for
> the vdso). Binary size is the same.
>
> Fixes: commit f2af201002a8 ("powerpc/build: vdso linker warning for orphan sections")
I think I'll just revert that for v5.9 ?
cheers
> Link: https://lore.kernel.org/lkml/CAKwvOdnn3wxYdJomvnveyD_njwRku3fABWT_bS92duihhywLJQ@mail.gmail.com/
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> arch/powerpc/include/asm/vdso.h | 17 ++---------------
> arch/powerpc/kernel/vdso64/Makefile | 8 ++++++--
> arch/powerpc/kernel/vdso64/vdso64.lds.S | 1 -
> 3 files changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/vdso.h b/arch/powerpc/include/asm/vdso.h
> index 2ff884853f97..11b2ecf49f79 100644
> --- a/arch/powerpc/include/asm/vdso.h
> +++ b/arch/powerpc/include/asm/vdso.h
> @@ -24,19 +24,7 @@ int vdso_getcpu_init(void);
>
> #else /* __ASSEMBLY__ */
>
> -#ifdef __VDSO64__
> -#define V_FUNCTION_BEGIN(name) \
> - .globl name; \
> - name: \
> -
> -#define V_FUNCTION_END(name) \
> - .size name,.-name;
> -
> -#define V_LOCAL_FUNC(name) (name)
> -#endif /* __VDSO64__ */
> -
> -#ifdef __VDSO32__
> -
> +#if defined(__VDSO32__) || defined (__VDSO64__)
> #define V_FUNCTION_BEGIN(name) \
> .globl name; \
> .type name,@function; \
> @@ -46,8 +34,7 @@ int vdso_getcpu_init(void);
> .size name,.-name;
>
> #define V_LOCAL_FUNC(name) (name)
> -
> -#endif /* __VDSO32__ */
> +#endif /* __VDSO{32|64}__ */
>
> #endif /* __ASSEMBLY__ */
>
> diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
> index 38c317f25141..7ea3ce537d0a 100644
> --- a/arch/powerpc/kernel/vdso64/Makefile
> +++ b/arch/powerpc/kernel/vdso64/Makefile
> @@ -32,9 +32,13 @@ $(obj)/%.so: OBJCOPYFLAGS := -S
> $(obj)/%.so: $(obj)/%.so.dbg FORCE
> $(call if_changed,objcopy)
>
> +ldflags-y := -shared -soname linux-vdso64.so.1 \
> + $(call ld-option, --eh-frame-hdr) \
> + $(call ld-option, --orphan-handling=warn) -T
> +
> # actual build commands
> -quiet_cmd_vdso64ld = VDSO64L $@
> - cmd_vdso64ld = $(CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) $(call cc-ldoption, -Wl$(comma)--orphan-handling=warn)
> +quiet_cmd_vdso64ld = LD $@
> + cmd_vdso64ld = $(cmd_ld)
>
> # install commands for the unstripped file
> quiet_cmd_vdso_install = INSTALL $@
> diff --git a/arch/powerpc/kernel/vdso64/vdso64.lds.S b/arch/powerpc/kernel/vdso64/vdso64.lds.S
> index 4e3a8d4ee614..58c33b704b6a 100644
> --- a/arch/powerpc/kernel/vdso64/vdso64.lds.S
> +++ b/arch/powerpc/kernel/vdso64/vdso64.lds.S
> @@ -11,7 +11,6 @@ OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle")
> OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc")
> #endif
> OUTPUT_ARCH(powerpc:common64)
> -ENTRY(_start)
>
> SECTIONS
> {
> --
> 2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply
* [PATCH v4 13/13] mm/debug_vm_pgtable: Avoid none pte in pte_clear_test
From: Aneesh Kumar K.V @ 2020-09-02 11:46 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
pte_clear_tests operate on an existing pte entry. Make sure that
is not a none pte entry.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 9afa1354326b..c36530c69e33 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -542,9 +542,10 @@ static void __init pgd_populate_tests(struct mm_struct *mm, pgd_t *pgdp,
#endif /* PAGETABLE_P4D_FOLDED */
static void __init pte_clear_tests(struct mm_struct *mm, pte_t *ptep,
- unsigned long vaddr)
+ unsigned long pfn, unsigned long vaddr,
+ pgprot_t prot)
{
- pte_t pte = ptep_get(ptep);
+ pte_t pte = pfn_pte(pfn, prot);
pr_debug("Validating PTE clear\n");
pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
@@ -1049,7 +1050,7 @@ static int __init debug_vm_pgtable(void)
ptl = pte_lockptr(mm, pmdp);
spin_lock(ptl);
- pte_clear_tests(mm, ptep, vaddr);
+ pte_clear_tests(mm, ptep, pte_aligned, vaddr, prot);
pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
pte_unmap_unlock(ptep, ptl);
--
2.26.2
^ permalink raw reply related
* [PATCH v4 13/13] mm/debug_vm_pgtable: Avoid none pte in pte_clear_test
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
pte_clear_tests operate on an existing pte entry. Make sure that
is not a none pte entry.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 9afa1354326b..c36530c69e33 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -542,9 +542,10 @@ static void __init pgd_populate_tests(struct mm_struct *mm, pgd_t *pgdp,
#endif /* PAGETABLE_P4D_FOLDED */
static void __init pte_clear_tests(struct mm_struct *mm, pte_t *ptep,
- unsigned long vaddr)
+ unsigned long pfn, unsigned long vaddr,
+ pgprot_t prot)
{
- pte_t pte = ptep_get(ptep);
+ pte_t pte = pfn_pte(pfn, prot);
pr_debug("Validating PTE clear\n");
pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
@@ -1049,7 +1050,7 @@ static int __init debug_vm_pgtable(void)
ptl = pte_lockptr(mm, pmdp);
spin_lock(ptl);
- pte_clear_tests(mm, ptep, vaddr);
+ pte_clear_tests(mm, ptep, pte_aligned, vaddr, prot);
pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
pte_unmap_unlock(ptep, ptl);
--
2.26.2
^ permalink raw reply related
* [PATCH v4 12/13] mm/debug_vm_pgtable/hugetlb: Disable hugetlb test on ppc64
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
The seems to be missing quite a lot of details w.r.t allocating
the correct pgtable_t page (huge_pte_alloc()), holding the right
lock (huge_pte_lock()) etc. The vma used is also not a hugetlb VMA.
ppc64 do have runtime checks within CONFIG_DEBUG_VM for most of these.
Hence disable the test on ppc64.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index b53903fdee85..9afa1354326b 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -811,6 +811,7 @@ static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot)
#endif /* CONFIG_ARCH_WANT_GENERAL_HUGETLB */
}
+#ifndef CONFIG_PPC_BOOK3S_64
static void __init hugetlb_advanced_tests(struct mm_struct *mm,
struct vm_area_struct *vma,
pte_t *ptep, unsigned long pfn,
@@ -853,6 +854,7 @@ static void __init hugetlb_advanced_tests(struct mm_struct *mm,
pte = huge_ptep_get(ptep);
WARN_ON(!(huge_pte_write(pte) && huge_pte_dirty(pte)));
}
+#endif
#else /* !CONFIG_HUGETLB_PAGE */
static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot) { }
static void __init hugetlb_advanced_tests(struct mm_struct *mm,
@@ -1065,7 +1067,9 @@ static int __init debug_vm_pgtable(void)
pud_populate_tests(mm, pudp, saved_pmdp);
spin_unlock(ptl);
+#ifndef CONFIG_PPC_BOOK3S_64
hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
+#endif
spin_lock(&mm->page_table_lock);
p4d_clear_tests(mm, p4dp);
--
2.26.2
^ permalink raw reply related
* [PATCH v4 11/13] mm/debug_vm_pgtable/pmd_clear: Don't use pmd/pud_clear on pte entries
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
pmd_clear() should not be used to clear pmd level pte entries.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 26023d990bd0..b53903fdee85 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -196,6 +196,8 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
pmd = READ_ONCE(*pmdp);
WARN_ON(pmd_young(pmd));
+ /* Clear the pte entries */
+ pmdp_huge_get_and_clear(mm, vaddr, pmdp);
pgtable = pgtable_trans_huge_withdraw(mm, pmdp);
}
@@ -319,6 +321,8 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
pudp_test_and_clear_young(vma, vaddr, pudp);
pud = READ_ONCE(*pudp);
WARN_ON(pud_young(pud));
+
+ pudp_huge_get_and_clear(mm, vaddr, pudp);
}
static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot)
@@ -442,8 +446,6 @@ static void __init pud_populate_tests(struct mm_struct *mm, pud_t *pudp,
* This entry points to next level page table page.
* Hence this must not qualify as pud_bad().
*/
- pmd_clear(pmdp);
- pud_clear(pudp);
pud_populate(mm, pudp, pmdp);
pud = READ_ONCE(*pudp);
WARN_ON(pud_bad(pud));
@@ -575,7 +577,6 @@ static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp,
* This entry points to next level page table page.
* Hence this must not qualify as pmd_bad().
*/
- pmd_clear(pmdp);
pmd_populate(mm, pmdp, pgtable);
pmd = READ_ONCE(*pmdp);
WARN_ON(pmd_bad(pmd));
--
2.26.2
^ permalink raw reply related
* [PATCH v4 10/13] mm/debug_vm_pgtable/thp: Use page table depost/withdraw with THP
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
Architectures like ppc64 use deposited page table while updating the
huge pte entries.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 2bc1952e5f83..26023d990bd0 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -154,7 +154,7 @@ static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
static void __init pmd_advanced_tests(struct mm_struct *mm,
struct vm_area_struct *vma, pmd_t *pmdp,
unsigned long pfn, unsigned long vaddr,
- pgprot_t prot)
+ pgprot_t prot, pgtable_t pgtable)
{
pmd_t pmd;
@@ -165,6 +165,8 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
/* Align the address wrt HPAGE_PMD_SIZE */
vaddr = (vaddr & HPAGE_PMD_MASK) + HPAGE_PMD_SIZE;
+ pgtable_trans_huge_deposit(mm, pmdp, pgtable);
+
pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
set_pmd_at(mm, vaddr, pmdp, pmd);
pmdp_set_wrprotect(mm, vaddr, pmdp);
@@ -193,6 +195,8 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
pmdp_test_and_clear_young(vma, vaddr, pmdp);
pmd = READ_ONCE(*pmdp);
WARN_ON(pmd_young(pmd));
+
+ pgtable = pgtable_trans_huge_withdraw(mm, pmdp);
}
static void __init pmd_leaf_tests(unsigned long pfn, pgprot_t prot)
@@ -371,7 +375,7 @@ static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
static void __init pmd_advanced_tests(struct mm_struct *mm,
struct vm_area_struct *vma, pmd_t *pmdp,
unsigned long pfn, unsigned long vaddr,
- pgprot_t prot)
+ pgprot_t prot, pgtable_t pgtable)
{
}
static void __init pud_advanced_tests(struct mm_struct *mm,
@@ -1048,7 +1052,7 @@ static int __init debug_vm_pgtable(void)
ptl = pmd_lock(mm, pmdp);
pmd_clear_tests(mm, pmdp);
- pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
+ pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot, saved_ptep);
pmd_huge_tests(pmdp, pmd_aligned, prot);
pmd_populate_tests(mm, pmdp, saved_ptep);
spin_unlock(ptl);
--
2.26.2
^ permalink raw reply related
* [PATCH v4 09/13] mm/debug_vm_pgtable/locks: Take correct page table lock
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
Make sure we call pte accessors with correct lock held.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index f59cf6a9b05e..2bc1952e5f83 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -1035,30 +1035,39 @@ static int __init debug_vm_pgtable(void)
hugetlb_basic_tests(pte_aligned, prot);
- pte_clear_tests(mm, ptep, vaddr);
- pmd_clear_tests(mm, pmdp);
- pud_clear_tests(mm, pudp);
- p4d_clear_tests(mm, p4dp);
- pgd_clear_tests(mm, pgdp);
+ /*
+ * Page table modifying tests. They need to hold
+ * proper page table lock.
+ */
ptl = pte_lockptr(mm, pmdp);
spin_lock(ptl);
-
+ pte_clear_tests(mm, ptep, vaddr);
pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
- pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
- pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
- hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
-
+ pte_unmap_unlock(ptep, ptl);
+ ptl = pmd_lock(mm, pmdp);
+ pmd_clear_tests(mm, pmdp);
+ pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
pmd_huge_tests(pmdp, pmd_aligned, prot);
+ pmd_populate_tests(mm, pmdp, saved_ptep);
+ spin_unlock(ptl);
+
+ ptl = pud_lock(mm, pudp);
+ pud_clear_tests(mm, pudp);
+ pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
pud_huge_tests(pudp, pud_aligned, prot);
+ pud_populate_tests(mm, pudp, saved_pmdp);
+ spin_unlock(ptl);
- pte_unmap_unlock(ptep, ptl);
+ hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
- pmd_populate_tests(mm, pmdp, saved_ptep);
- pud_populate_tests(mm, pudp, saved_pmdp);
+ spin_lock(&mm->page_table_lock);
+ p4d_clear_tests(mm, p4dp);
+ pgd_clear_tests(mm, pgdp);
p4d_populate_tests(mm, p4dp, saved_pudp);
pgd_populate_tests(mm, pgdp, saved_p4dp);
+ spin_unlock(&mm->page_table_lock);
p4d_free(mm, saved_p4dp);
pud_free(mm, saved_pudp);
--
2.26.2
^ permalink raw reply related
* [PATCH v4 08/13] mm/debug_vm_pgtable/locks: Move non page table modifying test together
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
This will help in adding proper locks in a later patch
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 51 ++++++++++++++++++++++++-------------------
1 file changed, 28 insertions(+), 23 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index de333871f407..f59cf6a9b05e 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -986,7 +986,7 @@ static int __init debug_vm_pgtable(void)
p4dp = p4d_alloc(mm, pgdp, vaddr);
pudp = pud_alloc(mm, p4dp, vaddr);
pmdp = pmd_alloc(mm, pudp, vaddr);
- ptep = pte_alloc_map_lock(mm, pmdp, vaddr, &ptl);
+ ptep = pte_alloc_map(mm, pmdp, vaddr);
/*
* Save all the page table page addresses as the page table
@@ -1006,33 +1006,12 @@ static int __init debug_vm_pgtable(void)
p4d_basic_tests(p4d_aligned, prot);
pgd_basic_tests(pgd_aligned, prot);
- pte_clear_tests(mm, ptep, vaddr);
- pmd_clear_tests(mm, pmdp);
- pud_clear_tests(mm, pudp);
- p4d_clear_tests(mm, p4dp);
- pgd_clear_tests(mm, pgdp);
-
- pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
- pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
- pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
- hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
-
pmd_leaf_tests(pmd_aligned, prot);
pud_leaf_tests(pud_aligned, prot);
- pmd_huge_tests(pmdp, pmd_aligned, prot);
- pud_huge_tests(pudp, pud_aligned, prot);
-
pte_savedwrite_tests(pte_aligned, protnone);
pmd_savedwrite_tests(pmd_aligned, protnone);
- pte_unmap_unlock(ptep, ptl);
-
- pmd_populate_tests(mm, pmdp, saved_ptep);
- pud_populate_tests(mm, pudp, saved_pmdp);
- p4d_populate_tests(mm, p4dp, saved_pudp);
- pgd_populate_tests(mm, pgdp, saved_p4dp);
-
pte_special_tests(pte_aligned, prot);
pte_protnone_tests(pte_aligned, protnone);
pmd_protnone_tests(pmd_aligned, protnone);
@@ -1050,11 +1029,37 @@ static int __init debug_vm_pgtable(void)
pmd_swap_tests(pmd_aligned, prot);
swap_migration_tests();
- hugetlb_basic_tests(pte_aligned, prot);
pmd_thp_tests(pmd_aligned, prot);
pud_thp_tests(pud_aligned, prot);
+ hugetlb_basic_tests(pte_aligned, prot);
+
+ pte_clear_tests(mm, ptep, vaddr);
+ pmd_clear_tests(mm, pmdp);
+ pud_clear_tests(mm, pudp);
+ p4d_clear_tests(mm, p4dp);
+ pgd_clear_tests(mm, pgdp);
+
+ ptl = pte_lockptr(mm, pmdp);
+ spin_lock(ptl);
+
+ pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
+ pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
+ pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
+ hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
+
+
+ pmd_huge_tests(pmdp, pmd_aligned, prot);
+ pud_huge_tests(pudp, pud_aligned, prot);
+
+ pte_unmap_unlock(ptep, ptl);
+
+ pmd_populate_tests(mm, pmdp, saved_ptep);
+ pud_populate_tests(mm, pudp, saved_pmdp);
+ p4d_populate_tests(mm, p4dp, saved_pudp);
+ pgd_populate_tests(mm, pgdp, saved_p4dp);
+
p4d_free(mm, saved_p4dp);
pud_free(mm, saved_pudp);
pmd_free(mm, saved_pmdp);
--
2.26.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox