* [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
@ 2025-09-30 2:52 Anshuman Khandual
2025-09-30 4:37 ` Dev Jain
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Anshuman Khandual @ 2025-09-30 2:52 UTC (permalink / raw)
To: linux-mm
Cc: Anshuman Khandual, Andrew Morton, David Hildenbrand, linux-kernel
Replace READ_ONCE() with standard page table accessors i.e pxdp_get() which
anyways default into READ_ONCE() in cases where platform does not override.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
mm/ptdump.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/mm/ptdump.c b/mm/ptdump.c
index b600c7f864b8..18861501b533 100644
--- a/mm/ptdump.c
+++ b/mm/ptdump.c
@@ -31,7 +31,7 @@ static int ptdump_pgd_entry(pgd_t *pgd, unsigned long addr,
unsigned long next, struct mm_walk *walk)
{
struct ptdump_state *st = walk->private;
- pgd_t val = READ_ONCE(*pgd);
+ pgd_t val = pgdp_get(pgd);
#if CONFIG_PGTABLE_LEVELS > 4 && \
(defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
@@ -54,7 +54,7 @@ static int ptdump_p4d_entry(p4d_t *p4d, unsigned long addr,
unsigned long next, struct mm_walk *walk)
{
struct ptdump_state *st = walk->private;
- p4d_t val = READ_ONCE(*p4d);
+ p4d_t val = p4dp_get(p4d);
#if CONFIG_PGTABLE_LEVELS > 3 && \
(defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
@@ -77,7 +77,7 @@ static int ptdump_pud_entry(pud_t *pud, unsigned long addr,
unsigned long next, struct mm_walk *walk)
{
struct ptdump_state *st = walk->private;
- pud_t val = READ_ONCE(*pud);
+ pud_t val = pudp_get(pud);
#if CONFIG_PGTABLE_LEVELS > 2 && \
(defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
@@ -100,7 +100,7 @@ static int ptdump_pmd_entry(pmd_t *pmd, unsigned long addr,
unsigned long next, struct mm_walk *walk)
{
struct ptdump_state *st = walk->private;
- pmd_t val = READ_ONCE(*pmd);
+ pmd_t val = pmdp_get(pmd);
#if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
if (pmd_page(val) == virt_to_page(lm_alias(kasan_early_shadow_pte)))
--
2.30.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
2025-09-30 2:52 [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors Anshuman Khandual
@ 2025-09-30 4:37 ` Dev Jain
2025-09-30 6:41 ` David Hildenbrand
2025-09-30 14:30 ` Lance Yang
2025-09-30 18:21 ` SeongJae Park
2 siblings, 1 reply; 8+ messages in thread
From: Dev Jain @ 2025-09-30 4:37 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm
Cc: Andrew Morton, David Hildenbrand, linux-kernel
On 30/09/25 8:22 am, Anshuman Khandual wrote:
> Replace READ_ONCE() with standard page table accessors i.e pxdp_get() which
> anyways default into READ_ONCE() in cases where platform does not override.
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> mm/ptdump.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/mm/ptdump.c b/mm/ptdump.c
> index b600c7f864b8..18861501b533 100644
> --- a/mm/ptdump.c
> +++ b/mm/ptdump.c
> @@ -31,7 +31,7 @@ static int ptdump_pgd_entry(pgd_t *pgd, unsigned long addr,
> unsigned long next, struct mm_walk *walk)
> {
> struct ptdump_state *st = walk->private;
> - pgd_t val = READ_ONCE(*pgd);
> + pgd_t val = pgdp_get(pgd);
>
> #if CONFIG_PGTABLE_LEVELS > 4 && \
> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
> @@ -54,7 +54,7 @@ static int ptdump_p4d_entry(p4d_t *p4d, unsigned long addr,
> unsigned long next, struct mm_walk *walk)
> {
> struct ptdump_state *st = walk->private;
> - p4d_t val = READ_ONCE(*p4d);
> + p4d_t val = p4dp_get(p4d);
>
> #if CONFIG_PGTABLE_LEVELS > 3 && \
> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
> @@ -77,7 +77,7 @@ static int ptdump_pud_entry(pud_t *pud, unsigned long addr,
> unsigned long next, struct mm_walk *walk)
> {
> struct ptdump_state *st = walk->private;
> - pud_t val = READ_ONCE(*pud);
> + pud_t val = pudp_get(pud);
>
> #if CONFIG_PGTABLE_LEVELS > 2 && \
> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
> @@ -100,7 +100,7 @@ static int ptdump_pmd_entry(pmd_t *pmd, unsigned long addr,
> unsigned long next, struct mm_walk *walk)
> {
> struct ptdump_state *st = walk->private;
> - pmd_t val = READ_ONCE(*pmd);
> + pmd_t val = pmdp_get(pmd);
I believe this should go through pmdp_get_lockless(). I can see in pgtable.h that
some magic is required on some arches to decode the pmd correctly in case walking
without locks.
Reviewed-by: Dev Jain <dev.jain@arm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
2025-09-30 4:37 ` Dev Jain
@ 2025-09-30 6:41 ` David Hildenbrand
2025-09-30 7:00 ` Anshuman Khandual
0 siblings, 1 reply; 8+ messages in thread
From: David Hildenbrand @ 2025-09-30 6:41 UTC (permalink / raw)
To: Dev Jain, Anshuman Khandual, linux-mm; +Cc: Andrew Morton, linux-kernel
On 30.09.25 06:37, Dev Jain wrote:
>
> On 30/09/25 8:22 am, Anshuman Khandual wrote:
>> Replace READ_ONCE() with standard page table accessors i.e pxdp_get() which
>> anyways default into READ_ONCE() in cases where platform does not override.
>>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: linux-mm@kvack.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> mm/ptdump.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/mm/ptdump.c b/mm/ptdump.c
>> index b600c7f864b8..18861501b533 100644
>> --- a/mm/ptdump.c
>> +++ b/mm/ptdump.c
>> @@ -31,7 +31,7 @@ static int ptdump_pgd_entry(pgd_t *pgd, unsigned long addr,
>> unsigned long next, struct mm_walk *walk)
>> {
>> struct ptdump_state *st = walk->private;
>> - pgd_t val = READ_ONCE(*pgd);
>> + pgd_t val = pgdp_get(pgd);
>>
>> #if CONFIG_PGTABLE_LEVELS > 4 && \
>> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
>> @@ -54,7 +54,7 @@ static int ptdump_p4d_entry(p4d_t *p4d, unsigned long addr,
>> unsigned long next, struct mm_walk *walk)
>> {
>> struct ptdump_state *st = walk->private;
>> - p4d_t val = READ_ONCE(*p4d);
>> + p4d_t val = p4dp_get(p4d);
>>
>> #if CONFIG_PGTABLE_LEVELS > 3 && \
>> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
>> @@ -77,7 +77,7 @@ static int ptdump_pud_entry(pud_t *pud, unsigned long addr,
>> unsigned long next, struct mm_walk *walk)
>> {
>> struct ptdump_state *st = walk->private;
>> - pud_t val = READ_ONCE(*pud);
>> + pud_t val = pudp_get(pud);
>>
>> #if CONFIG_PGTABLE_LEVELS > 2 && \
>> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
>> @@ -100,7 +100,7 @@ static int ptdump_pmd_entry(pmd_t *pmd, unsigned long addr,
>> unsigned long next, struct mm_walk *walk)
>> {
>> struct ptdump_state *st = walk->private;
>> - pmd_t val = READ_ONCE(*pmd);
>> + pmd_t val = pmdp_get(pmd);
>
> I believe this should go through pmdp_get_lockless(). I can see in pgtable.h that
> some magic is required on some arches to decode the pmd correctly in case walking
> without locks.
pmdp_get_lockless() is a nasty thingy to handle selected 32bit
architectures.
But given that we're using ptep_get_lockless() in ptdump_pmd_entry() it
probably wouldn't hurt to use pmdp_get_lockless() here.
Staring at ARCH_HAS_PTDUMP, I don't think any 32bit arch would actually
end up compiling ptdump.c.
E.g., on x86 only X86_64 ends up selecting ARCH_HAS_PTDUMP.
--
Cheers
David / dhildenb
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
2025-09-30 6:41 ` David Hildenbrand
@ 2025-09-30 7:00 ` Anshuman Khandual
2025-09-30 14:43 ` David Hildenbrand
0 siblings, 1 reply; 8+ messages in thread
From: Anshuman Khandual @ 2025-09-30 7:00 UTC (permalink / raw)
To: David Hildenbrand, Dev Jain, linux-mm; +Cc: Andrew Morton, linux-kernel
On 30/09/25 12:11 PM, David Hildenbrand wrote:
> On 30.09.25 06:37, Dev Jain wrote:
>>
>> On 30/09/25 8:22 am, Anshuman Khandual wrote:
>>> Replace READ_ONCE() with standard page table accessors i.e pxdp_get() which
>>> anyways default into READ_ONCE() in cases where platform does not override.
>>>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: linux-mm@kvack.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>> mm/ptdump.c | 8 ++++----
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/mm/ptdump.c b/mm/ptdump.c
>>> index b600c7f864b8..18861501b533 100644
>>> --- a/mm/ptdump.c
>>> +++ b/mm/ptdump.c
>>> @@ -31,7 +31,7 @@ static int ptdump_pgd_entry(pgd_t *pgd, unsigned long addr,
>>> unsigned long next, struct mm_walk *walk)
>>> {
>>> struct ptdump_state *st = walk->private;
>>> - pgd_t val = READ_ONCE(*pgd);
>>> + pgd_t val = pgdp_get(pgd);
>>> #if CONFIG_PGTABLE_LEVELS > 4 && \
>>> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
>>> @@ -54,7 +54,7 @@ static int ptdump_p4d_entry(p4d_t *p4d, unsigned long addr,
>>> unsigned long next, struct mm_walk *walk)
>>> {
>>> struct ptdump_state *st = walk->private;
>>> - p4d_t val = READ_ONCE(*p4d);
>>> + p4d_t val = p4dp_get(p4d);
>>> #if CONFIG_PGTABLE_LEVELS > 3 && \
>>> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
>>> @@ -77,7 +77,7 @@ static int ptdump_pud_entry(pud_t *pud, unsigned long addr,
>>> unsigned long next, struct mm_walk *walk)
>>> {
>>> struct ptdump_state *st = walk->private;
>>> - pud_t val = READ_ONCE(*pud);
>>> + pud_t val = pudp_get(pud);
>>> #if CONFIG_PGTABLE_LEVELS > 2 && \
>>> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
>>> @@ -100,7 +100,7 @@ static int ptdump_pmd_entry(pmd_t *pmd, unsigned long addr,
>>> unsigned long next, struct mm_walk *walk)
>>> {
>>> struct ptdump_state *st = walk->private;
>>> - pmd_t val = READ_ONCE(*pmd);
>>> + pmd_t val = pmdp_get(pmd);
>>
>> I believe this should go through pmdp_get_lockless(). I can see in pgtable.h that
>> some magic is required on some arches to decode the pmd correctly in case walking
>> without locks.
>
> pmdp_get_lockless() is a nasty thingy to handle selected 32bit architectures.
>
> But given that we're using ptep_get_lockless() in ptdump_pmd_entry() it probably wouldn't hurt to use pmdp_get_lockless() here.
>
> Staring at ARCH_HAS_PTDUMP, I don't think any 32bit arch would actually end up compiling ptdump.c.
>
> E.g., on x86 only X86_64 ends up selecting ARCH_HAS_PTDUMP.
>
pxdp_get_lockless() not really required here, let's stick with pxdp_get() instead.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
2025-09-30 2:52 [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors Anshuman Khandual
2025-09-30 4:37 ` Dev Jain
@ 2025-09-30 14:30 ` Lance Yang
2025-09-30 18:21 ` SeongJae Park
2 siblings, 0 replies; 8+ messages in thread
From: Lance Yang @ 2025-09-30 14:30 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-mm, Andrew Morton, David Hildenbrand, linux-kernel
On Tue, Sep 30, 2025 at 10:56 AM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
> Replace READ_ONCE() with standard page table accessors i.e pxdp_get() which
> anyways default into READ_ONCE() in cases where platform does not override.
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
Nice cleanup! I assume it's a no-op change on all architectures for now ;)
Acked-by: Lance Yang <lance.yang@linux.dev>
> mm/ptdump.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/mm/ptdump.c b/mm/ptdump.c
> index b600c7f864b8..18861501b533 100644
> --- a/mm/ptdump.c
> +++ b/mm/ptdump.c
> @@ -31,7 +31,7 @@ static int ptdump_pgd_entry(pgd_t *pgd, unsigned long addr,
> unsigned long next, struct mm_walk *walk)
> {
> struct ptdump_state *st = walk->private;
> - pgd_t val = READ_ONCE(*pgd);
> + pgd_t val = pgdp_get(pgd);
>
> #if CONFIG_PGTABLE_LEVELS > 4 && \
> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
> @@ -54,7 +54,7 @@ static int ptdump_p4d_entry(p4d_t *p4d, unsigned long addr,
> unsigned long next, struct mm_walk *walk)
> {
> struct ptdump_state *st = walk->private;
> - p4d_t val = READ_ONCE(*p4d);
> + p4d_t val = p4dp_get(p4d);
>
> #if CONFIG_PGTABLE_LEVELS > 3 && \
> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
> @@ -77,7 +77,7 @@ static int ptdump_pud_entry(pud_t *pud, unsigned long addr,
> unsigned long next, struct mm_walk *walk)
> {
> struct ptdump_state *st = walk->private;
> - pud_t val = READ_ONCE(*pud);
> + pud_t val = pudp_get(pud);
>
> #if CONFIG_PGTABLE_LEVELS > 2 && \
> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
> @@ -100,7 +100,7 @@ static int ptdump_pmd_entry(pmd_t *pmd, unsigned long addr,
> unsigned long next, struct mm_walk *walk)
> {
> struct ptdump_state *st = walk->private;
> - pmd_t val = READ_ONCE(*pmd);
> + pmd_t val = pmdp_get(pmd);
>
> #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
> if (pmd_page(val) == virt_to_page(lm_alias(kasan_early_shadow_pte)))
> --
> 2.30.2
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
2025-09-30 7:00 ` Anshuman Khandual
@ 2025-09-30 14:43 ` David Hildenbrand
2025-09-30 16:08 ` Dev Jain
0 siblings, 1 reply; 8+ messages in thread
From: David Hildenbrand @ 2025-09-30 14:43 UTC (permalink / raw)
To: Anshuman Khandual, Dev Jain, linux-mm; +Cc: Andrew Morton, linux-kernel
On 30.09.25 09:00, Anshuman Khandual wrote:
>
>
> On 30/09/25 12:11 PM, David Hildenbrand wrote:
>> On 30.09.25 06:37, Dev Jain wrote:
>>>
>>> On 30/09/25 8:22 am, Anshuman Khandual wrote:
>>>> Replace READ_ONCE() with standard page table accessors i.e pxdp_get() which
>>>> anyways default into READ_ONCE() in cases where platform does not override.
>>>>
>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>>> Cc: David Hildenbrand <david@redhat.com>
>>>> Cc: linux-mm@kvack.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>> ---
>>>> mm/ptdump.c | 8 ++++----
>>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/mm/ptdump.c b/mm/ptdump.c
>>>> index b600c7f864b8..18861501b533 100644
>>>> --- a/mm/ptdump.c
>>>> +++ b/mm/ptdump.c
>>>> @@ -31,7 +31,7 @@ static int ptdump_pgd_entry(pgd_t *pgd, unsigned long addr,
>>>> unsigned long next, struct mm_walk *walk)
>>>> {
>>>> struct ptdump_state *st = walk->private;
>>>> - pgd_t val = READ_ONCE(*pgd);
>>>> + pgd_t val = pgdp_get(pgd);
>>>> #if CONFIG_PGTABLE_LEVELS > 4 && \
>>>> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
>>>> @@ -54,7 +54,7 @@ static int ptdump_p4d_entry(p4d_t *p4d, unsigned long addr,
>>>> unsigned long next, struct mm_walk *walk)
>>>> {
>>>> struct ptdump_state *st = walk->private;
>>>> - p4d_t val = READ_ONCE(*p4d);
>>>> + p4d_t val = p4dp_get(p4d);
>>>> #if CONFIG_PGTABLE_LEVELS > 3 && \
>>>> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
>>>> @@ -77,7 +77,7 @@ static int ptdump_pud_entry(pud_t *pud, unsigned long addr,
>>>> unsigned long next, struct mm_walk *walk)
>>>> {
>>>> struct ptdump_state *st = walk->private;
>>>> - pud_t val = READ_ONCE(*pud);
>>>> + pud_t val = pudp_get(pud);
>>>> #if CONFIG_PGTABLE_LEVELS > 2 && \
>>>> (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS))
>>>> @@ -100,7 +100,7 @@ static int ptdump_pmd_entry(pmd_t *pmd, unsigned long addr,
>>>> unsigned long next, struct mm_walk *walk)
>>>> {
>>>> struct ptdump_state *st = walk->private;
>>>> - pmd_t val = READ_ONCE(*pmd);
>>>> + pmd_t val = pmdp_get(pmd);
>>>
>>> I believe this should go through pmdp_get_lockless(). I can see in pgtable.h that
>>> some magic is required on some arches to decode the pmd correctly in case walking
>>> without locks.
>>
>> pmdp_get_lockless() is a nasty thingy to handle selected 32bit architectures.
>>
>> But given that we're using ptep_get_lockless() in ptdump_pmd_entry() it probably wouldn't hurt to use pmdp_get_lockless() here.
>>
>> Staring at ARCH_HAS_PTDUMP, I don't think any 32bit arch would actually end up compiling ptdump.c.
>>
>> E.g., on x86 only X86_64 ends up selecting ARCH_HAS_PTDUMP.
>>
>
> pxdp_get_lockless() not really required here, let's stick with pxdp_get() instead.
I'd suggest that we keep it consistent. That is, also removing the
ptep_get_lockless() if not really required.
--
Cheers
David / dhildenb
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
2025-09-30 14:43 ` David Hildenbrand
@ 2025-09-30 16:08 ` Dev Jain
0 siblings, 0 replies; 8+ messages in thread
From: Dev Jain @ 2025-09-30 16:08 UTC (permalink / raw)
To: David Hildenbrand, Anshuman Khandual, linux-mm
Cc: Andrew Morton, linux-kernel
On 30/09/25 8:13 pm, David Hildenbrand wrote:
> On 30.09.25 09:00, Anshuman Khandual wrote:
>>
>>
>> On 30/09/25 12:11 PM, David Hildenbrand wrote:
>>> On 30.09.25 06:37, Dev Jain wrote:
>>>>
>>>> On 30/09/25 8:22 am, Anshuman Khandual wrote:
>>>>> Replace READ_ONCE() with standard page table accessors i.e
>>>>> pxdp_get() which
>>>>> anyways default into READ_ONCE() in cases where platform does not
>>>>> override.
>>>>>
>>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>>>> Cc: David Hildenbrand <david@redhat.com>
>>>>> Cc: linux-mm@kvack.org
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>>> ---
>>>>> mm/ptdump.c | 8 ++++----
>>>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/mm/ptdump.c b/mm/ptdump.c
>>>>> index b600c7f864b8..18861501b533 100644
>>>>> --- a/mm/ptdump.c
>>>>> +++ b/mm/ptdump.c
>>>>> @@ -31,7 +31,7 @@ static int ptdump_pgd_entry(pgd_t *pgd, unsigned
>>>>> long addr,
>>>>> unsigned long next, struct mm_walk *walk)
>>>>> {
>>>>> struct ptdump_state *st = walk->private;
>>>>> - pgd_t val = READ_ONCE(*pgd);
>>>>> + pgd_t val = pgdp_get(pgd);
>>>>> #if CONFIG_PGTABLE_LEVELS > 4 && \
>>>>> (defined(CONFIG_KASAN_GENERIC) ||
>>>>> defined(CONFIG_KASAN_SW_TAGS))
>>>>> @@ -54,7 +54,7 @@ static int ptdump_p4d_entry(p4d_t *p4d, unsigned
>>>>> long addr,
>>>>> unsigned long next, struct mm_walk *walk)
>>>>> {
>>>>> struct ptdump_state *st = walk->private;
>>>>> - p4d_t val = READ_ONCE(*p4d);
>>>>> + p4d_t val = p4dp_get(p4d);
>>>>> #if CONFIG_PGTABLE_LEVELS > 3 && \
>>>>> (defined(CONFIG_KASAN_GENERIC) ||
>>>>> defined(CONFIG_KASAN_SW_TAGS))
>>>>> @@ -77,7 +77,7 @@ static int ptdump_pud_entry(pud_t *pud, unsigned
>>>>> long addr,
>>>>> unsigned long next, struct mm_walk *walk)
>>>>> {
>>>>> struct ptdump_state *st = walk->private;
>>>>> - pud_t val = READ_ONCE(*pud);
>>>>> + pud_t val = pudp_get(pud);
>>>>> #if CONFIG_PGTABLE_LEVELS > 2 && \
>>>>> (defined(CONFIG_KASAN_GENERIC) ||
>>>>> defined(CONFIG_KASAN_SW_TAGS))
>>>>> @@ -100,7 +100,7 @@ static int ptdump_pmd_entry(pmd_t *pmd,
>>>>> unsigned long addr,
>>>>> unsigned long next, struct mm_walk *walk)
>>>>> {
>>>>> struct ptdump_state *st = walk->private;
>>>>> - pmd_t val = READ_ONCE(*pmd);
>>>>> + pmd_t val = pmdp_get(pmd);
>>>>
>>>> I believe this should go through pmdp_get_lockless(). I can see in
>>>> pgtable.h that
>>>> some magic is required on some arches to decode the pmd correctly
>>>> in case walking
>>>> without locks.
>>>
>>> pmdp_get_lockless() is a nasty thingy to handle selected 32bit
>>> architectures.
>>>
>>> But given that we're using ptep_get_lockless() in ptdump_pmd_entry()
>>> it probably wouldn't hurt to use pmdp_get_lockless() here.
>>>
>>> Staring at ARCH_HAS_PTDUMP, I don't think any 32bit arch would
>>> actually end up compiling ptdump.c.
>>>
>>> E.g., on x86 only X86_64 ends up selecting ARCH_HAS_PTDUMP.
>>>
>>
>> pxdp_get_lockless() not really required here, let's stick with
>> pxdp_get() instead.
>
> I'd suggest that we keep it consistent. That is, also removing the
> ptep_get_lockless() if not really required.
So in theory, the _lockless variant is not required, but surely
consistency would mean that since we are
dereferencing the pte without the PTL, we should use the _lockless variant?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
2025-09-30 2:52 [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors Anshuman Khandual
2025-09-30 4:37 ` Dev Jain
2025-09-30 14:30 ` Lance Yang
@ 2025-09-30 18:21 ` SeongJae Park
2 siblings, 0 replies; 8+ messages in thread
From: SeongJae Park @ 2025-09-30 18:21 UTC (permalink / raw)
To: Anshuman Khandual
Cc: SeongJae Park, linux-mm, Andrew Morton, David Hildenbrand, linux-kernel
On Tue, 30 Sep 2025 03:52:46 +0100 Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> Replace READ_ONCE() with standard page table accessors i.e pxdp_get() which
> anyways default into READ_ONCE() in cases where platform does not override.
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
Acked-by: SeongJae Park <sj@kernel.org>
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-09-30 18:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-30 2:52 [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors Anshuman Khandual
2025-09-30 4:37 ` Dev Jain
2025-09-30 6:41 ` David Hildenbrand
2025-09-30 7:00 ` Anshuman Khandual
2025-09-30 14:43 ` David Hildenbrand
2025-09-30 16:08 ` Dev Jain
2025-09-30 14:30 ` Lance Yang
2025-09-30 18:21 ` SeongJae Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox