Re: [PATCH v12 01/16] set_memory: set_direct_map_* to take address

[Mike Rapoport]

On Fri, Jun 26, 2026 at 03:04:48PM +0000, Brendan Jackman wrote:
> On Fri Jun 26, 2026 at 2:38 PM UTC, Brendan Jackman wrote:
> > On Tue Apr 21, 2026 at 2:43 PM UTC, Lorenzo Stoakes wrote:
> >> On Fri, Apr 10, 2026 at 03:17:58PM +0000, Kalyazin, Nikita wrote:
> >>> From: Nikita Kalyazin <nikita.kalyazin@xxxxxxxxx>
> >>>
> >>> Let's convert set_direct_map_*() to take an address instead of a page to
> >>> prepare for adding helpers that operate on folios; it will be more
> >>> efficient to convert from a folio directly to an address without going
> >>> through a page first.
> >
> > Why is this more efficient? Isn't it a purely compile-time conversion?
> >
> > Indeed in the current implementation folio_address() is
> > page_address(&folio->page) so it still goes through a page anyway, no?
> >
> > I might be missing context here about how this will look in the
> > memdesc future.
> 
> Sorry I failed to do the history search here, this was suggested by
> Willy during the review here:
> 
> https://lore.kernel.org/kvm/aWkN4yzwPtotaTeq@xxxxxxxxxxxxxxxxxxxx/
> 
> Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> > The implementation isn't the greatest.  None of the implementations
> > of set_direct_map_valid_noflush() actually do anything with the struct
> > page; they all call page_address() or page_to_virt() (fundamentally the
> > same thing).  
> 
> But IMO the struct page isn't there to carry runtime information, it's a
> type hint that this API operates on physical pages. Yes this is already
> implied by the name but it also gives us a certain amount of safety
> since it avoids unaligned or non-physmap addresses at compile time.

I think it's more historical thingy, the initial user was
vmalloc::set_area_direct_map() that passed each page from vm->pages[] to
set_direct_map functions.
 
And yes, having a page there let's the implementation to go directly from
the page to it's address in the direct map, even for highmem ...

> > So converting folio->page->address is a bit inefficient.
> 
> ... but yeah now I see this comes from Willy I definitely suspect I'm
> missing memdesc insight here. 
> 
> > It feels like we should change set_direct_map_valid_noflush() to take a
> > const void * and pass either page_address() or folio_address(), depending
> > whether the caller has a page or a folio.  What do you think?

... while with void * it's not necessarily one-to-one because void * can be
a vmalloc address. 

-- 
Sincerely yours,
Mike.