1. Patchew
  2. linux
  3. View series

[PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors

Anshuman Khandual posted 1 patch 1 day, 20 hours ago
There is a newer version of this series
mm/ptdump.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
Posted by Anshuman Khandual 1 day, 20 hours ago 
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
Re: [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
Posted by SeongJae Park 1 day, 5 hours ago 
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

[...]
Re: [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
Posted by Lance Yang 1 day, 9 hours ago 
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
>
>
Re: [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
Posted by Dev Jain 1 day, 18 hours ago 
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>
Re: [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
Posted by David Hildenbrand 1 day, 16 hours ago 
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
Re: [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
Posted by Anshuman Khandual 1 day, 16 hours ago 

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.
Re: [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
Posted by David Hildenbrand 1 day, 8 hours ago 
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
Re: [PATCH] mm/ptdump: Replace READ_ONCE() with standard page table accessors
Posted by Dev Jain 1 day, 7 hours ago 
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?

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.