Re: [PATCH v10 01/15] set_memory: set_direct_map_* to take address

[Nikita Kalyazin]

On 05/03/2026 17:23, David Hildenbrand (Arm) wrote:
> On 1/26/26 17:46, Kalyazin, Nikita wrote:
> > From: Nikita Kalyazin <kalyazin@xxxxxxxxxx>
> >
> > This is to avoid excessive conversions folio->page->address when adding
> > helpers on top of set_direct_map_valid_noflush() in the next patch.
> >
> > Signed-off-by: Nikita Kalyazin <kalyazin@xxxxxxxxxx>
> > ---
> >   arch/arm64/include/asm/set_memory.h     |  7 ++++---
> >   arch/arm64/mm/pageattr.c                | 19 +++++++++----------
> >   arch/loongarch/include/asm/set_memory.h |  7 ++++---
> >   arch/loongarch/mm/pageattr.c            | 25 ++++++++++++-------------
> >   arch/riscv/include/asm/set_memory.h     |  7 ++++---
> >   arch/riscv/mm/pageattr.c                | 17 +++++++++--------
> >   arch/s390/include/asm/set_memory.h      |  7 ++++---
> >   arch/s390/mm/pageattr.c                 | 13 +++++++------
> >   arch/x86/include/asm/set_memory.h       |  7 ++++---
> >   arch/x86/mm/pat/set_memory.c            | 23 ++++++++++++-----------
> >   include/linux/set_memory.h              |  9 +++++----
> >   kernel/power/snapshot.c                 |  4 ++--
> >   mm/execmem.c                            |  6 ++++--
> >   mm/secretmem.c                          |  6 +++---
> >   mm/vmalloc.c                            | 11 +++++++----
> >   15 files changed, 90 insertions(+), 78 deletions(-)
>
> [...]
>
> > --- a/arch/loongarch/mm/pageattr.c
> > +++ b/arch/loongarch/mm/pageattr.c
> > @@ -198,32 +198,31 @@ bool kernel_page_present(struct page *page)
> >        return pte_present(ptep_get(pte));
> >   }
> >
> > -int set_direct_map_default_noflush(struct page *page)
> > +int set_direct_map_default_noflush(const void *addr)
> >   {
> > -     unsigned long addr = (unsigned long)page_address(page);
> > -
> > -     if (addr < vm_map_base)
> > +     if ((unsigned long)addr < vm_map_base)
> >                return 0;
> >
> > -     return __set_memory(addr, 1, PAGE_KERNEL, __pgprot(0));
> > +     return __set_memory((unsigned long)addr, 1, PAGE_KERNEL, __pgprot(0));
> >   }
> >
> > -int set_direct_map_invalid_noflush(struct page *page)
> > +int set_direct_map_invalid_noflush(const void *addr)
> >   {
> > -     unsigned long addr = (unsigned long)page_address(page);
> > +     unsigned long addr = (unsigned long)addr;
>
> Are you sure you want a local variable with the exact same name

You're right.  Thanks for spotting that.

> ...
>
> > -     if (addr < vm_map_base)
> > +     if ((unsigned long)addr < vm_map_base)
> >                return 0;
> >
> > -     return __set_memory(addr, 1, __pgprot(0), __pgprot(_PAGE_PRESENT | _PAGE_VALID));
> > +     return __set_memory((unsigned long)addr, 1, __pgprot(0),
> > +                         __pgprot(_PAGE_PRESENT | _PAGE_VALID));
>
> And cast it to (unsigned long) even though not required two times? :)
>
> I assume you wanted to get rid of the local varable.

Yes, that's what I meant.

> >   }
> >
> > -int set_direct_map_valid_noflush(struct page *page, unsigned nr, bool valid)
> > +int set_direct_map_valid_noflush(const void *addr, unsigned long numpages,
> > +                              bool valid)
>
>
>
> Nothing else jumped at me.
>
> Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

Thanks!

> It would be good to get some ACK from some arch people that are CCed :)
>
> --
> Cheers,
>
> David