Received-SPF: pass (google.com: domain of linux-kernel+bounces-188498-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Fri, 24 May 2024 11:58:39 +0300
From: Mike Rapoport <rppt@kernel.org>
To: David Hildenbrand <david@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Andrew Morton <akpm@linux-foundation.org>,
	"Matthew Wilcox (Oracle)" <willy@infradead.org>,
	Minchan Kim <minchan@kernel.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Hyeonggon Yoo <42.hyeyoo@gmail.com>
Subject: Re: [PATCH RFC 1/6] mm: update _mapcount and page_type documentation
Message-ID: <ZlBWv3nrN_U_5QJQ@kernel.org>
References: <20240522210341.1030552-1-david@redhat.com>
 <20240522210341.1030552-2-david@redhat.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240522210341.1030552-2-david@redhat.com>

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@redhat.com>
> ---
>  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.