Re: [PATCH v7 11/16] mm/highmem: introduce clear_user_highpages()

[David Hildenbrand]

On 23.09.25 22:34, Ankur Arora wrote:
> David Hildenbrand <david@xxxxxxxxxx> writes:
>
> > On 17.09.25 17:24, Ankur Arora wrote:
> > > Define clear_user_highpages() which clears pages sequentially using
> > > the single page variant.
> > > With !CONFIG_HIGHMEM, pages are contiguous so use the range clearing
> > > primitive clear_user_pages().
> > > Signed-off-by: Ankur Arora <ankur.a.arora@xxxxxxxxxx>
> > > ---
> > >    include/linux/highmem.h | 18 ++++++++++++++++++
> > >    1 file changed, 18 insertions(+)
> > > diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> > > index 6234f316468c..ed609987e24d 100644
> > > --- a/include/linux/highmem.h
> > > +++ b/include/linux/highmem.h
> > > @@ -207,6 +207,24 @@ static inline void clear_user_highpage(struct page *page, unsigned long vaddr)
> > >    }
> > >    #endif
> > >    +#ifndef clear_user_highpages
> >
> > Maybe we can add a simple kernel doc that points at the doc of clear_user_pages,
> > but makes it clear that this for pages that might reside in highmem.
>
> Didn't add one because clear_user_highpage() didn't have one. Will add
> for both.

Doesn't have to be excessive. But even I have to keep reminding myself 
when to use clear_page(), clear_user_page(), clear_user_highpage() ...

> > > +static inline void clear_user_highpages(struct page *page, unsigned long vaddr,
> > > +					unsigned int npages)
> > > +{
> > > +	if (!IS_ENABLED(CONFIG_HIGHMEM)) {
> > > +		clear_user_pages(base, vaddr, page, npages);
> >
> > Single line should work
> >
> > 		clear_user_pages(page_address(page), vaddr, page, npages);
>
> Unfortunately not. The problem is that I'm defining the fallback version
> of clear_user_pages() as a macro in the previous patch.

Yet another sign that we have to fix that instead. :)

--
Cheers

David / dhildenb