Re: [PATCH v2 mm-new] mm: check if folio has valid mapcount before folio_test_{anon,ksm}() when necessary

[Harry Yoo <harry.yoo@oracle.com>]

On Tue, Jul 08, 2025 at 10:07:26AM +0200, David Hildenbrand wrote:
> >   	if (folio_test_anon(folio)) {
> > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> > index 970600d79dac..00ba2c8b221e 100644
> > --- a/include/linux/page-flags.h
> > +++ b/include/linux/page-flags.h
> > @@ -706,12 +706,15 @@ PAGEFLAG_FALSE(VmemmapSelfHosted, vmemmap_self_hosted)
> >    * address_space which maps the folio from disk; whereas "folio_mapped"
> >    * refers to user virtual address space into which the folio is mapped.
> >    *
> > - * For slab pages, since slab reuses the bits in struct page to store its
> > - * internal states, the folio->mapping does not exist as such, nor do
> > - * these flags below.  So in order to avoid testing non-existent bits,
> > - * please make sure that folio_test_slab(folio) actually evaluates to
> > - * false before calling the following functions (e.g., folio_test_anon).
> > - * See mm/slab.h.
> > + * For certain typed pages like slabs, since they reuse bits in struct page
> > + * to store internal states, folio->mapping does not point to a valid
> > + * mapping, nor do these flags exist. To avoid testing non-existent bits,
> > + * make sure folio_has_mapcount() actually evaluates to true before calling
> > + * the following functions (e.g., folio_test_anon).
> > + *
> > + * The folio_has_mapcount() check can be skipped if the folio is mapped
> > + * to userspace, since a folio with !folio_has_mapcount() cannot be mapped
> > + * to userspace at all.
> >    */
> >   #define FOLIO_MAPPING_ANON	0x1
> >   #define FOLIO_MAPPING_ANON_KSM	0x2
> > @@ -1092,6 +1095,11 @@ static inline bool PageHuge(const struct page *page)
> >   	return folio_test_hugetlb(page_folio(page));
> >   }
> > +static inline bool folio_has_mapcount(const struct folio *folio)
> > +{
> > +	return !page_has_type(&folio->page) || folio_test_hugetlb(folio);
> > +}
> > +
> 
> I don't like the naming.

I don't like it either, but I couldn't come up with a better name :(

> The thing is, in the future only folios will have a mapcount.

Agreed.

> Asking whether a folio has a mapcount is asking the wrong question.

Agreed.

> It's rather, that we want to reject *pages* early, and not cast them to the
> folio in the first place.

Totally agreed.

But that's the future, and we're not there yet.
What should we do now?

-- 
Cheers,
Harry / Hyeonggon