All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Price <steven.price@arm.com>
To: Christophe Leroy <christophe.leroy@csgroup.eu>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	akpm@linux-foundation.org
Cc: linux-arch@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-riscv@lists.infradead.org, x86@kernel.org,
	linux-mm@kvack.org
Subject: Re: [PATCH v1 3/5] mm: ptdump: Provide page size to notepage()
Date: Mon, 19 Apr 2021 15:00:14 +0100	[thread overview]
Message-ID: <4a76fbda-aa9d-867b-e2eb-a1951780aeec@arm.com> (raw)
In-Reply-To: <627ee414-2f78-94e3-b77b-1013f52e77e3@csgroup.eu>

On 19/04/2021 14:14, Christophe Leroy wrote:
> 
> 
> Le 16/04/2021 à 12:51, Steven Price a écrit :
>> On 16/04/2021 11:38, Christophe Leroy wrote:
>>>
>>>
>>> Le 16/04/2021 à 11:28, Steven Price a écrit :
>>>> On 15/04/2021 18:18, Christophe Leroy wrote:
>>>>
>>>> To be honest I don't fully understand why powerpc requires the 
>>>> page_size - it appears to be using it purely to find "holes" in the 
>>>> calls to note_page(), but I haven't worked out why such holes would 
>>>> occur.
>>>
>>> I was indeed introduced for KASAN. We have a first commit 
>>> https://github.com/torvalds/linux/commit/cabe8138 which uses page 
>>> size to detect whether it is a KASAN like stuff.
>>>
>>> Then came https://github.com/torvalds/linux/commit/b00ff6d8c as a 
>>> fix. I can't remember what the problem was exactly, something around 
>>> the use of hugepages for kernel memory, came as part of the series 
>>> https://patchwork.ozlabs.org/project/linuxppc-dev/cover/cover.1589866984.git.christophe.leroy@csgroup.eu/ 
>>
>>
>>
>> Ah, that's useful context. So it looks like powerpc took a different 
>> route to reducing the KASAN output to x86.
>>
>> Given the generic ptdump code has handling for KASAN already it should 
>> be possible to drop that from the powerpc arch code, which I think 
>> means we don't actually need to provide page size to notepage(). 
>> Hopefully that means more code to delete ;)
>>
> 
> Looking at how the generic ptdump code handles KASAN, I'm a bit sceptic.
> 
> IIUC, it is checking that kasan_early_shadow_pte is in the same page as 
> the pgtable referred by the PMD entry. But what happens if that PMD 
> entry is referring another pgtable which is inside the same page as 
> kasan_early_shadow_pte ?
> 
> Shouldn't the test be
> 
>      if (pmd_page_vaddr(val) == lm_alias(kasan_early_shadow_pte))
>          return note_kasan_page_table(walk, addr);

Now I come to look at this code again, I think you're right. On arm64 
this doesn't cause a problem - page tables are page sized and page 
aligned, so there couldn't be any non-KASAN pgtables sharing the page. 
Obviously that's not necessarily true of other architectures.

Feel free to add a patch to your series ;)

Steve

WARNING: multiple messages have this Message-ID (diff)
From: Steven Price <steven.price@arm.com>
To: Christophe Leroy <christophe.leroy@csgroup.eu>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	akpm@linux-foundation.org
Cc: linux-arch@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-riscv@lists.infradead.org, x86@kernel.org,
	linux-mm@kvack.org
Subject: Re: [PATCH v1 3/5] mm: ptdump: Provide page size to notepage()
Date: Mon, 19 Apr 2021 15:00:14 +0100	[thread overview]
Message-ID: <4a76fbda-aa9d-867b-e2eb-a1951780aeec@arm.com> (raw)
In-Reply-To: <627ee414-2f78-94e3-b77b-1013f52e77e3@csgroup.eu>

On 19/04/2021 14:14, Christophe Leroy wrote:
> 
> 
> Le 16/04/2021 à 12:51, Steven Price a écrit :
>> On 16/04/2021 11:38, Christophe Leroy wrote:
>>>
>>>
>>> Le 16/04/2021 à 11:28, Steven Price a écrit :
>>>> On 15/04/2021 18:18, Christophe Leroy wrote:
>>>>
>>>> To be honest I don't fully understand why powerpc requires the 
>>>> page_size - it appears to be using it purely to find "holes" in the 
>>>> calls to note_page(), but I haven't worked out why such holes would 
>>>> occur.
>>>
>>> I was indeed introduced for KASAN. We have a first commit 
>>> https://github.com/torvalds/linux/commit/cabe8138 which uses page 
>>> size to detect whether it is a KASAN like stuff.
>>>
>>> Then came https://github.com/torvalds/linux/commit/b00ff6d8c as a 
>>> fix. I can't remember what the problem was exactly, something around 
>>> the use of hugepages for kernel memory, came as part of the series 
>>> https://patchwork.ozlabs.org/project/linuxppc-dev/cover/cover.1589866984.git.christophe.leroy@csgroup.eu/ 
>>
>>
>>
>> Ah, that's useful context. So it looks like powerpc took a different 
>> route to reducing the KASAN output to x86.
>>
>> Given the generic ptdump code has handling for KASAN already it should 
>> be possible to drop that from the powerpc arch code, which I think 
>> means we don't actually need to provide page size to notepage(). 
>> Hopefully that means more code to delete ;)
>>
> 
> Looking at how the generic ptdump code handles KASAN, I'm a bit sceptic.
> 
> IIUC, it is checking that kasan_early_shadow_pte is in the same page as 
> the pgtable referred by the PMD entry. But what happens if that PMD 
> entry is referring another pgtable which is inside the same page as 
> kasan_early_shadow_pte ?
> 
> Shouldn't the test be
> 
>      if (pmd_page_vaddr(val) == lm_alias(kasan_early_shadow_pte))
>          return note_kasan_page_table(walk, addr);

Now I come to look at this code again, I think you're right. On arm64 
this doesn't cause a problem - page tables are page sized and page 
aligned, so there couldn't be any non-KASAN pgtables sharing the page. 
Obviously that's not necessarily true of other architectures.

Feel free to add a patch to your series ;)

Steve

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

WARNING: multiple messages have this Message-ID (diff)
From: Steven Price <steven.price@arm.com>
To: Christophe Leroy <christophe.leroy@csgroup.eu>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	akpm@linux-foundation.org
Cc: linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
	x86@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-riscv@lists.infradead.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v1 3/5] mm: ptdump: Provide page size to notepage()
Date: Mon, 19 Apr 2021 15:00:14 +0100	[thread overview]
Message-ID: <4a76fbda-aa9d-867b-e2eb-a1951780aeec@arm.com> (raw)
In-Reply-To: <627ee414-2f78-94e3-b77b-1013f52e77e3@csgroup.eu>

On 19/04/2021 14:14, Christophe Leroy wrote:
> 
> 
> Le 16/04/2021 à 12:51, Steven Price a écrit :
>> On 16/04/2021 11:38, Christophe Leroy wrote:
>>>
>>>
>>> Le 16/04/2021 à 11:28, Steven Price a écrit :
>>>> On 15/04/2021 18:18, Christophe Leroy wrote:
>>>>
>>>> To be honest I don't fully understand why powerpc requires the 
>>>> page_size - it appears to be using it purely to find "holes" in the 
>>>> calls to note_page(), but I haven't worked out why such holes would 
>>>> occur.
>>>
>>> I was indeed introduced for KASAN. We have a first commit 
>>> https://github.com/torvalds/linux/commit/cabe8138 which uses page 
>>> size to detect whether it is a KASAN like stuff.
>>>
>>> Then came https://github.com/torvalds/linux/commit/b00ff6d8c as a 
>>> fix. I can't remember what the problem was exactly, something around 
>>> the use of hugepages for kernel memory, came as part of the series 
>>> https://patchwork.ozlabs.org/project/linuxppc-dev/cover/cover.1589866984.git.christophe.leroy@csgroup.eu/ 
>>
>>
>>
>> Ah, that's useful context. So it looks like powerpc took a different 
>> route to reducing the KASAN output to x86.
>>
>> Given the generic ptdump code has handling for KASAN already it should 
>> be possible to drop that from the powerpc arch code, which I think 
>> means we don't actually need to provide page size to notepage(). 
>> Hopefully that means more code to delete ;)
>>
> 
> Looking at how the generic ptdump code handles KASAN, I'm a bit sceptic.
> 
> IIUC, it is checking that kasan_early_shadow_pte is in the same page as 
> the pgtable referred by the PMD entry. But what happens if that PMD 
> entry is referring another pgtable which is inside the same page as 
> kasan_early_shadow_pte ?
> 
> Shouldn't the test be
> 
>      if (pmd_page_vaddr(val) == lm_alias(kasan_early_shadow_pte))
>          return note_kasan_page_table(walk, addr);

Now I come to look at this code again, I think you're right. On arm64 
this doesn't cause a problem - page tables are page sized and page 
aligned, so there couldn't be any non-KASAN pgtables sharing the page. 
Obviously that's not necessarily true of other architectures.

Feel free to add a patch to your series ;)

Steve

WARNING: multiple messages have this Message-ID (diff)
From: Steven Price <steven.price@arm.com>
To: Christophe Leroy <christophe.leroy@csgroup.eu>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	akpm@linux-foundation.org
Cc: linux-arch@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-riscv@lists.infradead.org, x86@kernel.org,
	linux-mm@kvack.org
Subject: Re: [PATCH v1 3/5] mm: ptdump: Provide page size to notepage()
Date: Mon, 19 Apr 2021 15:00:14 +0100	[thread overview]
Message-ID: <4a76fbda-aa9d-867b-e2eb-a1951780aeec@arm.com> (raw)
In-Reply-To: <627ee414-2f78-94e3-b77b-1013f52e77e3@csgroup.eu>

On 19/04/2021 14:14, Christophe Leroy wrote:
> 
> 
> Le 16/04/2021 à 12:51, Steven Price a écrit :
>> On 16/04/2021 11:38, Christophe Leroy wrote:
>>>
>>>
>>> Le 16/04/2021 à 11:28, Steven Price a écrit :
>>>> On 15/04/2021 18:18, Christophe Leroy wrote:
>>>>
>>>> To be honest I don't fully understand why powerpc requires the 
>>>> page_size - it appears to be using it purely to find "holes" in the 
>>>> calls to note_page(), but I haven't worked out why such holes would 
>>>> occur.
>>>
>>> I was indeed introduced for KASAN. We have a first commit 
>>> https://github.com/torvalds/linux/commit/cabe8138 which uses page 
>>> size to detect whether it is a KASAN like stuff.
>>>
>>> Then came https://github.com/torvalds/linux/commit/b00ff6d8c as a 
>>> fix. I can't remember what the problem was exactly, something around 
>>> the use of hugepages for kernel memory, came as part of the series 
>>> https://patchwork.ozlabs.org/project/linuxppc-dev/cover/cover.1589866984.git.christophe.leroy@csgroup.eu/ 
>>
>>
>>
>> Ah, that's useful context. So it looks like powerpc took a different 
>> route to reducing the KASAN output to x86.
>>
>> Given the generic ptdump code has handling for KASAN already it should 
>> be possible to drop that from the powerpc arch code, which I think 
>> means we don't actually need to provide page size to notepage(). 
>> Hopefully that means more code to delete ;)
>>
> 
> Looking at how the generic ptdump code handles KASAN, I'm a bit sceptic.
> 
> IIUC, it is checking that kasan_early_shadow_pte is in the same page as 
> the pgtable referred by the PMD entry. But what happens if that PMD 
> entry is referring another pgtable which is inside the same page as 
> kasan_early_shadow_pte ?
> 
> Shouldn't the test be
> 
>      if (pmd_page_vaddr(val) == lm_alias(kasan_early_shadow_pte))
>          return note_kasan_page_table(walk, addr);

Now I come to look at this code again, I think you're right. On arm64 
this doesn't cause a problem - page tables are page sized and page 
aligned, so there couldn't be any non-KASAN pgtables sharing the page. 
Obviously that's not necessarily true of other architectures.

Feel free to add a patch to your series ;)

Steve

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-04-19 14:00 UTC|newest]

Thread overview: 91+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-15 17:18 [PATCH v1 0/5] Convert powerpc to GENERIC_PTDUMP Christophe Leroy
2021-04-15 17:18 ` Christophe Leroy
2021-04-15 17:18 ` Christophe Leroy
2021-04-15 17:18 ` Christophe Leroy
2021-04-15 17:18 ` [PATCH v1 1/5] mm: pagewalk: Fix walk for hugepage tables Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy
2021-04-15 22:43   ` Daniel Axtens
2021-04-15 22:43     ` Daniel Axtens
2021-04-15 22:43     ` Daniel Axtens
2021-04-15 22:43     ` Daniel Axtens
2021-04-16  5:48     ` Christophe Leroy
2021-04-16  5:48       ` Christophe Leroy
2021-04-16  5:48       ` Christophe Leroy
2021-04-16  5:48       ` Christophe Leroy
2021-04-15 17:18 ` [PATCH v1 2/5] mm: ptdump: Fix build failure Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy
2021-04-15 17:18 ` [PATCH v1 3/5] mm: ptdump: Provide page size to notepage() Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy
2021-04-15 23:12   ` Daniel Axtens
2021-04-15 23:12     ` Daniel Axtens
2021-04-15 23:12     ` Daniel Axtens
2021-04-15 23:12     ` Daniel Axtens
2021-04-16  5:19     ` Christophe Leroy
2021-04-16  5:19       ` Christophe Leroy
2021-04-16  5:19       ` Christophe Leroy
2021-04-16  5:19       ` Christophe Leroy
2021-04-16  9:28   ` Steven Price
2021-04-16  9:28     ` Steven Price
2021-04-16  9:28     ` Steven Price
2021-04-16  9:28     ` Steven Price
2021-04-16 10:38     ` Christophe Leroy
2021-04-16 10:38       ` Christophe Leroy
2021-04-16 10:38       ` Christophe Leroy
2021-04-16 10:38       ` Christophe Leroy
2021-04-16 10:51       ` Steven Price
2021-04-16 10:51         ` Steven Price
2021-04-16 10:51         ` Steven Price
2021-04-16 10:51         ` Steven Price
2021-04-16 11:08         ` Christophe Leroy
2021-04-16 11:08           ` Christophe Leroy
2021-04-16 11:08           ` Christophe Leroy
2021-04-16 11:08           ` Christophe Leroy
2021-04-16 13:00           ` Steven Price
2021-04-16 13:00             ` Steven Price
2021-04-16 13:00             ` Steven Price
2021-04-16 13:00             ` Steven Price
2021-04-16 14:40             ` Christophe Leroy
2021-04-16 14:40               ` Christophe Leroy
2021-04-16 14:40               ` Christophe Leroy
2021-04-16 14:40               ` Christophe Leroy
2021-04-16 15:04               ` Christophe Leroy
2021-04-16 15:04                 ` Christophe Leroy
2021-04-16 15:04                 ` Christophe Leroy
2021-04-16 15:15                 ` Christophe Leroy
2021-04-16 15:15                   ` Christophe Leroy
2021-04-16 15:15                   ` Christophe Leroy
2021-04-16 16:00                   ` Steven Price
2021-04-16 16:00                     ` Steven Price
2021-04-16 16:00                     ` Steven Price
2021-04-19 13:14         ` Christophe Leroy
2021-04-19 13:14           ` Christophe Leroy
2021-04-19 13:14           ` Christophe Leroy
2021-04-19 13:14           ` Christophe Leroy
2021-04-19 14:00           ` Steven Price [this message]
2021-04-19 14:00             ` Steven Price
2021-04-19 14:00             ` Steven Price
2021-04-19 14:00             ` Steven Price
2021-04-19 16:41             ` Christophe Leroy
2021-04-19 16:41               ` Christophe Leroy
2021-04-19 16:41               ` Christophe Leroy
2021-04-19 16:41               ` Christophe Leroy
2021-04-15 17:18 ` [PATCH v1 4/5] mm: ptdump: Support hugepd table entries Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy
2021-04-15 23:29   ` Daniel Axtens
2021-04-15 23:29     ` Daniel Axtens
2021-04-15 23:29     ` Daniel Axtens
2021-04-16  5:25     ` Christophe Leroy
2021-04-16  5:25       ` Christophe Leroy
2021-04-16  5:25       ` Christophe Leroy
2021-04-15 17:18 ` [PATCH v1 5/5] powerpc/mm: Convert powerpc to GENERIC_PTDUMP Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy
2021-04-15 17:18   ` Christophe Leroy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4a76fbda-aa9d-867b-e2eb-a1951780aeec@arm.com \
    --to=steven.price@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.