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

[Helge Deller]

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 <pfalcato@xxxxxxx>
> > > > > > ---
> > > > > >   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....

Helge