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

Pedro Falcato <[email protected]>
Newsgroups org.ozlabs.lists.linuxppc-dev,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-parisc,org.kvack.linux-mm
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.