Re: [PATCH RFC 1/6] mm: update _mapcount and page_type documentation

[Mike Rapoport]

On Wed, May 22, 2024 at 11:03:36PM +0200, David Hildenbrand wrote:
> Let's make it clearer that _mapcount must no longer be used for own
> purposes, and how _mapcount and page_type behaves nowadays (also in the
> context of hugetlb folios, which are typed folios that will be mapped
> to user space).
> 
> Move the documentation regarding "-1" over from page_mapcount_reset(),
> which we will remove next.
> 
> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
> ---
>  include/linux/mm.h       |  5 -----
>  include/linux/mm_types.h | 24 +++++++++++++++---------
>  2 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 9849dfda44d4..018e7c0265ca 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1206,11 +1206,6 @@ static inline int folio_entire_mapcount(const struct folio *folio)
>  	return atomic_read(&folio->_entire_mapcount) + 1;
>  }
>  
> -/*
> - * The atomic page->_mapcount, starts from -1: so that transitions
> - * both from it and to it can be tracked, using atomic_inc_and_test
> - * and atomic_add_negative(-1).
> - */
>  static inline void page_mapcount_reset(struct page *page)
>  {
>  	atomic_set(&(page)->_mapcount, -1);
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 24323c7d0bd4..532a3030405d 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -46,9 +46,7 @@ struct mem_cgroup;
>   * which is guaranteed to be aligned.  If you use the same storage as
>   * page->mapping, you must restore it to NULL before freeing the page.
>   *
> - * If your page will not be mapped to userspace, you can also use the four
> - * bytes in the mapcount union, but you must call page_mapcount_reset()
> - * before freeing it.
> + * The mapcount field must not be used for own purposes.
>   *
>   * If you want to use the refcount field, it must be used in such a way
>   * that other CPUs temporarily incrementing and then decrementing the
> @@ -152,16 +150,24 @@ struct page {
>  
>  	union {		/* This union is 4 bytes in size. */
>  		/*
> -		 * If the page can be mapped to userspace, encodes the number
> -		 * of times this page is referenced by a page table.
> +		 * For pages part of non-typed folios for which mappings are

Maybe

For pages that are part ...

> +		 * tracked via the RMAP, encodes the number of times this page
> +		 * is directly referenced by a page table.
> +		 *
> +		 * Note that the mapcount is always initialized to -1, so that
> +		 * transitions both from it and to it can be tracked, using
> +		 * atomic_inc_and_test() and atomic_add_negative(-1).
>  		 */
>  		atomic_t _mapcount;
>  
>  		/*
> -		 * If the page is neither PageSlab nor mappable to userspace,
> -		 * the value stored here may help determine what this page
> -		 * is used for.  See page-flags.h for a list of page types
> -		 * which are currently stored here.
> +		 * For head pages of typed folios, the value stored here
> +		 * allows for determining what this page is used for. The
> +		 * tail pages of typed folios will not store a type
> +		 * (mapcount == -1).
> +		 *
> +		 * See page-flags.h for a list of page types which are currently
> +		 * stored here.
>  		 */
>  		unsigned int page_type;

and maybe move page_type before _mapcount, so that it will be a bit clearer
what are "typed" pages and folios are.

>  	};
> -- 
> 2.45.0
> 

-- 
Sincerely yours,
Mike.