Re: [PATCH 3/5] mm/parisc: constify ptep_get() argument

Pedro Falcato <[email protected]> Mon, 27 Jul 2026 10:14:54 +0100
Newsgroups org.kernel.vger.linux-parisc,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kvack.linux-mm,org.ozlabs.lists.linuxppc-dev
Message-ID <amchbBH5zF7Rq5LT@pedro-suse>
On Sat, Jul 25, 2026 at 07:38:23PM +0200, Helge Deller wrote:
> * John David Anglin <[email protected]>:
> > On 2026-07-25 12:27 p.m., Helge Deller wrote:
> > > On 7/24/26 17:48, Usama Arif wrote:
> > >>
> > >>
> > >> On 24/07/2026 16:32, Pedro Falcato wrote:
> > >>> On Fri, Jul 24, 2026 at 04:03:44PM +0100, Usama Arif wrote:
> > >>>>
> > >>>>
> > >>>> On 24/07/2026 15:14, Matthew Wilcox wrote:
> > >>>>> On Fri, Jul 24, 2026 at 02:36:59PM +0100, Usama Arif wrote:
> > >>>>>>
> > >>>>>>
> > >>>>>> On 24/07/2026 11:20, Pedro Falcato wrote:
> > >>>>>>> ptep_get() does not need write access to the PTE.
> > >>>>>>>
> > >>>>>>> Signed-off-by: Pedro Falcato <[email protected]>
> > >>>>>>> ---
> > >>>>>>>   arch/parisc/include/asm/pgtable.h | 2 +-
> > >>>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>>>>>
> > >>>>>> hmm I think you might break build bisectibility if
> > >>>>>> you separate out the patches, you should squash patch 3 and 4,
> > >>>>>> with patch 1.
> > >>>>>
> > >>>>> I don't see how this breaks bisectability.  Can you elaborate on what
> > >>>>> you think would break?
> > >>>>
> > >>>> Patch 1 does:
> > >>>>
> > >>>> -static inline pte_t ptep_get_lockless(pte_t *ptep)
> > >>>> +static inline pte_t ptep_get_lockless(const pte_t *ptep)
> > >>>>   {
> > >>>>       return ptep_get(ptep);
> > >>>>   }
> > >>>>
> > >>>>
> > >>>> ptep_get() takes a non-const arg till patch 3 for example for parsic.
> > >>>> I don't think you can pass a const variable to non const function arg?
> > >>>>
> > >>>> So parsic won't compile in patch 1 and 2, powerprc wont compile
> > >>>> for patches 1, 2 and 3..
> > >>>
> > >>> Aha, yes, nice catch! I'll have to rethink that. Maybe squashing the patches
> > >>> would be the cleanest way forward.
> > >>>
> > >>
> > >> Yes, squashing is the simplest way forward.
> > >>
> > >>> FWIW, the vast majority of architectures aren't doing funny things on
> > >>> ptep_get(). E.g PA-RISC, loongarch only define these so they can use it in
> > >>> their own asm/pgtable.h. I'd really like to delete these but I can't tell
> > >>> if they are *actually* required.
> > >>>
> > >>
> > >> Ah maybe there is something in git history on why it was needed?
> > >>
> > >> If not, hopefully someone from the arch lists could clarify..
> > > I tried to remove ptep_get() and ptep_test_and_clear_young() from
> > > arch/parisc/include/asm/pgtable.h, but this then triggers compile errors:
> > > 
> > >  CC      arch/parisc/kernel/asm-offsets.s
> > > In file included from /home/cvs/parisc/git-kernel/linus-linux-2.6/include/linux/mm.h:31,
> > >                  from /home/cvs/parisc/git-kernel/linus-linux-2.6/include/linux/pid_namespace.h:7,
> > >                  from /home/cvs/parisc/git-kernel/linus-linux-2.6/include/linux/ptrace.h:10,
> > >                  from /home/cvs/parisc/git-kernel/linus-linux-2.6/arch/parisc/kernel/asm-offsets.c:21:
> > > /home/cvs/parisc/git-kernel/linus-linux-2.6/include/linux/pgtable.h: In function ‘clear_young_dirty_ptes’:
> > > /home/cvs/parisc/git-kernel/linus-linux-2.6/include/linux/pgtable.h:683:25: error: implicit declaration of function ‘ptep_test_and_clear_young’;
> > > did you mean ‘pmdp_test_and_clear_young’? [-Wimplicit-function-declaration]
> > >   683 |                         ptep_test_and_clear_young(vma, addr, ptep);
> > >       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~
> > >       |                         pmdp_test_and_clear_young
> > > 
> > > So, I think the answer is, that the local copy of ptep_get() is used in the parisc implementation of
> > > ptep_test_and_clear_young() function.
> > > 
> > > Switching to the default include/linux/pgtable.h seems not easy doable....
> > 
> > Did you remove define for __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG in parisc version of pgtable.h?
> 
> Indeed, I missed to remove that.
> 
> Usama, if you want you can add the patch below to your patch set, thus
> simplifying your series?

It's actually my series but thanks Helge I'll pick it up!

> 
> Helge
> 
> ------
> 
> From: Helge Deller <[email protected]>
> Date: Sat, 25 Jul 2026 19:19:55 +0200
> Subject: [PATCH] parisc: Drop own implementations for ptep_get() and
>  ptep_test_and_clear_young()
> 
> Switch to the generic implementations, which are identical.
> 
> Suggested-by: Usama Arif <[email protected]>
> Suggested-by: John David Anglin <[email protected]>
> Signed-off-by: Helge Deller <[email protected]>
> 
> diff --git a/arch/parisc/include/asm/pgtable.h b/arch/parisc/include/asm/pgtable.h
> index 467b8547ac8b..d92dee9fef3f 100644
> --- a/arch/parisc/include/asm/pgtable.h
> +++ b/arch/parisc/include/asm/pgtable.h
> @@ -432,25 +432,6 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
>  	return pte;
>  }
>  
> -static inline pte_t ptep_get(pte_t *ptep)
> -{
> -	return READ_ONCE(*ptep);
> -}
> -#define ptep_get ptep_get
> -
> -static inline bool ptep_test_and_clear_young(struct vm_area_struct *vma,
> -		unsigned long addr, pte_t *ptep)
> -{
> -	pte_t pte;
> -
> -	pte = ptep_get(ptep);
> -	if (!pte_young(pte)) {
> -		return false;
> -	}
> -	set_pte_at(vma->vm_mm, addr, ptep, pte_mkold(pte));
> -	return true;
> -}
> -
>  bool ptep_clear_flush_young(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep);
>  pte_t ptep_clear_flush(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep);
>  
> @@ -491,7 +472,6 @@ static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long addr,
>  #define HAVE_ARCH_UNMAPPED_AREA
>  #define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
>  
> -#define __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG
>  #define __HAVE_ARCH_PTEP_CLEAR_YOUNG_FLUSH
>  #define __HAVE_ARCH_PTEP_CLEAR_FLUSH
>  #define __HAVE_ARCH_PTEP_SET_WRPROTECT

-- 
Pedro