+ mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch added to mm-unstable branch

+ mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch added to mm-unstable branch

[Andrew Morton]

The patch titled
     Subject: mm-constify-highmem-related-functions-for-improved-const-correctness-fix
has been added to the -mm mm-unstable branch.  Its filename is
     mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch

This patch will shortly appear at
     https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch

This patch will later appear in the mm-unstable branch at
    git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***

The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days

------------------------------------------------------
From: Andrew Morton <akpm@linux-foundation.org>
Subject: mm-constify-highmem-related-functions-for-improved-const-correctness-fix
Date: Wed Sep  3 07:51:59 PM PDT 2025

nastily "fix" folio_page()

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/linux/page-flags.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/include/linux/page-flags.h~mm-constify-highmem-related-functions-for-improved-const-correctness-fix
+++ a/include/linux/page-flags.h
@@ -316,9 +316,9 @@ static __always_inline unsigned long _co
  * check that the page number lies within @folio; the caller is presumed
  * to have a reference to the page.
  */
-static inline struct page *folio_page(struct folio *folio, unsigned long n)
+static inline struct page *folio_page(const struct folio *folio, unsigned long n)
 {
-	return &folio->page + n;
+	return (struct page *)(&folio->page + n);
 }
 
 static __always_inline int PageTail(const struct page *page)
_

Patches currently in -mm which might be from akpm@linux-foundation.org are

mm-replace-20-page_shift-with-common-macros-for-pages-mb-conversion-fix.patch
mm-replace-20-page_shift-with-common-macros-for-pages-mb-conversion-fix-fix.patch
mm-damon-core-skip-needless-update-of-damon_attrs-in-damon_commit_ctx-fix.patch
mm-convert-core-mm-to-mm_flags_-accessors-fix.patch
memcg-optimize-exit-to-user-space-fix.patch
memfd-move-mfd_all_flags-definition-to-memfdh.patch
mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch
mm-filemap-align-last_index-to-folio-size-fix-fix.patch

Re: + mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch added to mm-unstable branch

[David Hildenbrand]

On 04.09.25 04:59, Andrew Morton wrote:
> 
> The patch titled
>       Subject: mm-constify-highmem-related-functions-for-improved-const-correctness-fix
> has been added to the -mm mm-unstable branch.  Its filename is
>       mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch
> 
> This patch will shortly appear at
>       https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch
> 
> This patch will later appear in the mm-unstable branch at
>      git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> 
> Before you just go and hit "reply", please:
>     a) Consider who else should be cc'ed
>     b) Prefer to cc a suitable mailing list as well
>     c) Ideally: find the original patch on the mailing list and do a
>        reply-to-all to that, adding suitable additional cc's
> 
> *** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
> 
> The -mm tree is included into linux-next via the mm-everything
> branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> and is updated there every 2-3 working days
> 
> ------------------------------------------------------
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: mm-constify-highmem-related-functions-for-improved-const-correctness-fix
> Date: Wed Sep  3 07:51:59 PM PDT 2025
> 
> nastily "fix" folio_page()
> 
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>   include/linux/page-flags.h |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/include/linux/page-flags.h~mm-constify-highmem-related-functions-for-improved-const-correctness-fix
> +++ a/include/linux/page-flags.h
> @@ -316,9 +316,9 @@ static __always_inline unsigned long _co
>    * check that the page number lies within @folio; the caller is presumed
>    * to have a reference to the page.
>    */
> -static inline struct page *folio_page(struct folio *folio, unsigned long n)
> +static inline struct page *folio_page(const struct folio *folio, unsigned long n)
>   {
> -	return &folio->page + n;
> +	return (struct page *)(&folio->page + n);
>   }
>   
>   static __always_inline int PageTail(const struct page *page)
> _
> 


I think we should do the following. And I'm wondering if we should squash that
into the relevant nth_page patch instead:

 From 6ada1d36d18969db587ab647757a9538ee1d09a6 Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@redhat.com>
Date: Thu, 4 Sep 2025 08:35:24 +0200
Subject: [PATCH] fixup folio_page() const correctness

Signed-off-by: David Hildenbrand <david@redhat.com>
---
  include/linux/page-flags.h | 13 +++++++++----
  1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index faf17ca211b4f..3a9e73abea3e3 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -307,6 +307,12 @@ static __always_inline unsigned long _compound_head(const struct page *page)
  	const struct page *:	(const struct folio *)_compound_head(p), \
  	struct page *:		(struct folio *)_compound_head(p)))
  
+static inline const struct page *_folio_page(const struct folio *folio,
+		unsigned long n)
+{
+	return &folio->page + n;
+}
+
  /**
   * folio_page - Return a page from a folio.
   * @folio: The folio.
@@ -316,10 +322,9 @@ static __always_inline unsigned long _compound_head(const struct page *page)
   * check that the page number lies within @folio; the caller is presumed
   * to have a reference to the page.
   */
-static inline struct page *folio_page(struct folio *folio, unsigned long n)
-{
-	return &folio->page + n;
-}
+#define folio_page(f, n)	(_Generic((f),				\
+	const struct folio *:	(const struct page *)_folio_page(f, n), \
+	struct folio *:		(struct page *)_folio_page(f, n)))
  
  static __always_inline int PageTail(const struct page *page)
  {
-- 
2.50.1


-- 
Cheers

David / dhildenb

Re: + mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch added to mm-unstable branch

[Vlastimil Babka]

On 9/4/25 08:37, David Hildenbrand wrote:
> On 04.09.25 04:59, Andrew Morton wrote:
>> 
>> The patch titled
>>       Subject: mm-constify-highmem-related-functions-for-improved-const-correctness-fix
>> has been added to the -mm mm-unstable branch.  Its filename is
>>       mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch
>> 
>> This patch will shortly appear at
>>       https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch
>> 
>> This patch will later appear in the mm-unstable branch at
>>      git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>> 
>> Before you just go and hit "reply", please:
>>     a) Consider who else should be cc'ed
>>     b) Prefer to cc a suitable mailing list as well
>>     c) Ideally: find the original patch on the mailing list and do a
>>        reply-to-all to that, adding suitable additional cc's
>> 
>> *** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
>> 
>> The -mm tree is included into linux-next via the mm-everything
>> branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>> and is updated there every 2-3 working days
>> 
>> ------------------------------------------------------
>> From: Andrew Morton <akpm@linux-foundation.org>
>> Subject: mm-constify-highmem-related-functions-for-improved-const-correctness-fix
>> Date: Wed Sep  3 07:51:59 PM PDT 2025
>> 
>> nastily "fix" folio_page()
>> 
>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> Cc: Max Filippov <jcmvbkbc@gmail.com>
>> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>> ---
>> 
>>   include/linux/page-flags.h |    4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> --- a/include/linux/page-flags.h~mm-constify-highmem-related-functions-for-improved-const-correctness-fix
>> +++ a/include/linux/page-flags.h
>> @@ -316,9 +316,9 @@ static __always_inline unsigned long _co
>>    * check that the page number lies within @folio; the caller is presumed
>>    * to have a reference to the page.
>>    */
>> -static inline struct page *folio_page(struct folio *folio, unsigned long n)
>> +static inline struct page *folio_page(const struct folio *folio, unsigned long n)
>>   {
>> -	return &folio->page + n;
>> +	return (struct page *)(&folio->page + n);
>>   }
>>   
>>   static __always_inline int PageTail(const struct page *page)
>> _
>> 
> 
> 
> I think we should do the following. And I'm wondering if we should squash that
> into the relevant nth_page patch instead:

Sounds good, seems to be working for page_folio().

>  From 6ada1d36d18969db587ab647757a9538ee1d09a6 Mon Sep 17 00:00:00 2001
> From: David Hildenbrand <david@redhat.com>
> Date: Thu, 4 Sep 2025 08:35:24 +0200
> Subject: [PATCH] fixup folio_page() const correctness
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   include/linux/page-flags.h | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index faf17ca211b4f..3a9e73abea3e3 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -307,6 +307,12 @@ static __always_inline unsigned long _compound_head(const struct page *page)
>   	const struct page *:	(const struct folio *)_compound_head(p), \
>   	struct page *:		(struct folio *)_compound_head(p)))
>   
> +static inline const struct page *_folio_page(const struct folio *folio,
> +		unsigned long n)
> +{
> +	return &folio->page + n;

Maybe you can just copy/paste "&(f)->page + (n)" twice directly and not need
this wrapper? Maybe not even the casts as they would follow from the type of
f automatically?

> +}
> +
>   /**
>    * folio_page - Return a page from a folio.
>    * @folio: The folio.
> @@ -316,10 +322,9 @@ static __always_inline unsigned long _compound_head(const struct page *page)
>    * check that the page number lies within @folio; the caller is presumed
>    * to have a reference to the page.
>    */
> -static inline struct page *folio_page(struct folio *folio, unsigned long n)
> -{
> -	return &folio->page + n;
> -}
> +#define folio_page(f, n)	(_Generic((f),				\
> +	const struct folio *:	(const struct page *)_folio_page(f, n), \
> +	struct folio *:		(struct page *)_folio_page(f, n)))
>   
>   static __always_inline int PageTail(const struct page *page)
>   {

Re: + mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch added to mm-unstable branch

[David Hildenbrand]

On 04.09.25 09:57, Vlastimil Babka wrote:
> On 9/4/25 08:37, David Hildenbrand wrote:
>> On 04.09.25 04:59, Andrew Morton wrote:
>>>
>>> The patch titled
>>>        Subject: mm-constify-highmem-related-functions-for-improved-const-correctness-fix
>>> has been added to the -mm mm-unstable branch.  Its filename is
>>>        mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch
>>>
>>> This patch will shortly appear at
>>>        https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch
>>>
>>> This patch will later appear in the mm-unstable branch at
>>>       git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>
>>> Before you just go and hit "reply", please:
>>>      a) Consider who else should be cc'ed
>>>      b) Prefer to cc a suitable mailing list as well
>>>      c) Ideally: find the original patch on the mailing list and do a
>>>         reply-to-all to that, adding suitable additional cc's
>>>
>>> *** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
>>>
>>> The -mm tree is included into linux-next via the mm-everything
>>> branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>> and is updated there every 2-3 working days
>>>
>>> ------------------------------------------------------
>>> From: Andrew Morton <akpm@linux-foundation.org>
>>> Subject: mm-constify-highmem-related-functions-for-improved-const-correctness-fix
>>> Date: Wed Sep  3 07:51:59 PM PDT 2025
>>>
>>> nastily "fix" folio_page()
>>>
>>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>>> Cc: Max Filippov <jcmvbkbc@gmail.com>
>>> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>> Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
>>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>> ---
>>>
>>>    include/linux/page-flags.h |    4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> --- a/include/linux/page-flags.h~mm-constify-highmem-related-functions-for-improved-const-correctness-fix
>>> +++ a/include/linux/page-flags.h
>>> @@ -316,9 +316,9 @@ static __always_inline unsigned long _co
>>>     * check that the page number lies within @folio; the caller is presumed
>>>     * to have a reference to the page.
>>>     */
>>> -static inline struct page *folio_page(struct folio *folio, unsigned long n)
>>> +static inline struct page *folio_page(const struct folio *folio, unsigned long n)
>>>    {
>>> -	return &folio->page + n;
>>> +	return (struct page *)(&folio->page + n);
>>>    }
>>>    
>>>    static __always_inline int PageTail(const struct page *page)
>>> _
>>>
>>
>>
>> I think we should do the following. And I'm wondering if we should squash that
>> into the relevant nth_page patch instead:
> 
> Sounds good, seems to be working for page_folio().
> 
>>   From 6ada1d36d18969db587ab647757a9538ee1d09a6 Mon Sep 17 00:00:00 2001
>> From: David Hildenbrand <david@redhat.com>
>> Date: Thu, 4 Sep 2025 08:35:24 +0200
>> Subject: [PATCH] fixup folio_page() const correctness
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>    include/linux/page-flags.h | 13 +++++++++----
>>    1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>> index faf17ca211b4f..3a9e73abea3e3 100644
>> --- a/include/linux/page-flags.h
>> +++ b/include/linux/page-flags.h
>> @@ -307,6 +307,12 @@ static __always_inline unsigned long _compound_head(const struct page *page)
>>    	const struct page *:	(const struct folio *)_compound_head(p), \
>>    	struct page *:		(struct folio *)_compound_head(p)))
>>    
>> +static inline const struct page *_folio_page(const struct folio *folio,
>> +		unsigned long n)
>> +{
>> +	return &folio->page + n;
> 
> Maybe you can just copy/paste "&(f)->page + (n)" twice directly and not need
> this wrapper? Maybe not even the casts as they would follow from the type of
> f automatically?

Well, then the inline function is gone (which I enjoyed having), and the 
there would not be much sense in having the generic at all?

I mean, sure, the compiler would bail out on any other passed types and 
probably spit out less weird compiler errors compared to a simple macro.

-- 
Cheers

David / dhildenb

Re: + mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch added to mm-unstable branch

[Vlastimil Babka]

On 9/4/25 11:16, David Hildenbrand wrote:
> On 04.09.25 09:57, Vlastimil Babka wrote:
>> On 9/4/25 08:37, David Hildenbrand wrote:
>>> On 04.09.25 04:59, Andrew Morton wrote:
>>>>
>>>> The patch titled
>>>>        Subject: mm-constify-highmem-related-functions-for-improved-const-correctness-fix
>>>> has been added to the -mm mm-unstable branch.  Its filename is
>>>>        mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch
>>>>
>>>> This patch will shortly appear at
>>>>        https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch
>>>>
>>>> This patch will later appear in the mm-unstable branch at
>>>>       git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>>
>>>> Before you just go and hit "reply", please:
>>>>      a) Consider who else should be cc'ed
>>>>      b) Prefer to cc a suitable mailing list as well
>>>>      c) Ideally: find the original patch on the mailing list and do a
>>>>         reply-to-all to that, adding suitable additional cc's
>>>>
>>>> *** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
>>>>
>>>> The -mm tree is included into linux-next via the mm-everything
>>>> branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>> and is updated there every 2-3 working days
>>>>
>>>> ------------------------------------------------------
>>>> From: Andrew Morton <akpm@linux-foundation.org>
>>>> Subject: mm-constify-highmem-related-functions-for-improved-const-correctness-fix
>>>> Date: Wed Sep  3 07:51:59 PM PDT 2025
>>>>
>>>> nastily "fix" folio_page()
>>>>
>>>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>>>> Cc: Max Filippov <jcmvbkbc@gmail.com>
>>>> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>> Cc: David Hildenbrand <david@redhat.com>
>>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>>> Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
>>>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>>> ---
>>>>
>>>>    include/linux/page-flags.h |    4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> --- a/include/linux/page-flags.h~mm-constify-highmem-related-functions-for-improved-const-correctness-fix
>>>> +++ a/include/linux/page-flags.h
>>>> @@ -316,9 +316,9 @@ static __always_inline unsigned long _co
>>>>     * check that the page number lies within @folio; the caller is presumed
>>>>     * to have a reference to the page.
>>>>     */
>>>> -static inline struct page *folio_page(struct folio *folio, unsigned long n)
>>>> +static inline struct page *folio_page(const struct folio *folio, unsigned long n)
>>>>    {
>>>> -	return &folio->page + n;
>>>> +	return (struct page *)(&folio->page + n);
>>>>    }
>>>>    
>>>>    static __always_inline int PageTail(const struct page *page)
>>>> _
>>>>
>>>
>>>
>>> I think we should do the following. And I'm wondering if we should squash that
>>> into the relevant nth_page patch instead:
>> 
>> Sounds good, seems to be working for page_folio().
>> 
>>>   From 6ada1d36d18969db587ab647757a9538ee1d09a6 Mon Sep 17 00:00:00 2001
>>> From: David Hildenbrand <david@redhat.com>
>>> Date: Thu, 4 Sep 2025 08:35:24 +0200
>>> Subject: [PATCH] fixup folio_page() const correctness
>>>
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>> ---
>>>    include/linux/page-flags.h | 13 +++++++++----
>>>    1 file changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>>> index faf17ca211b4f..3a9e73abea3e3 100644
>>> --- a/include/linux/page-flags.h
>>> +++ b/include/linux/page-flags.h
>>> @@ -307,6 +307,12 @@ static __always_inline unsigned long _compound_head(const struct page *page)
>>>    	const struct page *:	(const struct folio *)_compound_head(p), \
>>>    	struct page *:		(struct folio *)_compound_head(p)))
>>>    
>>> +static inline const struct page *_folio_page(const struct folio *folio,
>>> +		unsigned long n)
>>> +{
>>> +	return &folio->page + n;
>> 
>> Maybe you can just copy/paste "&(f)->page + (n)" twice directly and not need
>> this wrapper? Maybe not even the casts as they would follow from the type of
>> f automatically?
> 
> Well, then the inline function is gone (which I enjoyed having), and the 
> there would not be much sense in having the generic at all?

I thought the benefit of the generic is if you pass const folio you get
const page, and if you pass non-const you get non-stop, and it doesn't
involve any cast of something already assumed const to non-const which I
assume is bad (TM). But I don't know how much bad in practice (can it lead
to optimizations screwing with your code?).

With the inline function in place there's such a cast const -> non-const on
the page pointer it returns, but maybe it's fine because it only happens
when the original folio was non-const? I don't know. It just doesn't seem to
add benefits when only used via the generic?

> I mean, sure, the compiler would bail out on any other passed types and 
> probably spit out less weird compiler errors compared to a simple macro.
> 

Re: + mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch added to mm-unstable branch

[David Hildenbrand]

On 04.09.25 11:28, Vlastimil Babka wrote:
> On 9/4/25 11:16, David Hildenbrand wrote:
>> On 04.09.25 09:57, Vlastimil Babka wrote:
>>> On 9/4/25 08:37, David Hildenbrand wrote:
>>>> On 04.09.25 04:59, Andrew Morton wrote:
>>>>>
>>>>> The patch titled
>>>>>         Subject: mm-constify-highmem-related-functions-for-improved-const-correctness-fix
>>>>> has been added to the -mm mm-unstable branch.  Its filename is
>>>>>         mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch
>>>>>
>>>>> This patch will shortly appear at
>>>>>         https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch
>>>>>
>>>>> This patch will later appear in the mm-unstable branch at
>>>>>        git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>>>
>>>>> Before you just go and hit "reply", please:
>>>>>       a) Consider who else should be cc'ed
>>>>>       b) Prefer to cc a suitable mailing list as well
>>>>>       c) Ideally: find the original patch on the mailing list and do a
>>>>>          reply-to-all to that, adding suitable additional cc's
>>>>>
>>>>> *** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
>>>>>
>>>>> The -mm tree is included into linux-next via the mm-everything
>>>>> branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>>> and is updated there every 2-3 working days
>>>>>
>>>>> ------------------------------------------------------
>>>>> From: Andrew Morton <akpm@linux-foundation.org>
>>>>> Subject: mm-constify-highmem-related-functions-for-improved-const-correctness-fix
>>>>> Date: Wed Sep  3 07:51:59 PM PDT 2025
>>>>>
>>>>> nastily "fix" folio_page()
>>>>>
>>>>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>>>>> Cc: Max Filippov <jcmvbkbc@gmail.com>
>>>>> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>>> Cc: David Hildenbrand <david@redhat.com>
>>>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>>>> Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
>>>>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>>>> ---
>>>>>
>>>>>     include/linux/page-flags.h |    4 ++--
>>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> --- a/include/linux/page-flags.h~mm-constify-highmem-related-functions-for-improved-const-correctness-fix
>>>>> +++ a/include/linux/page-flags.h
>>>>> @@ -316,9 +316,9 @@ static __always_inline unsigned long _co
>>>>>      * check that the page number lies within @folio; the caller is presumed
>>>>>      * to have a reference to the page.
>>>>>      */
>>>>> -static inline struct page *folio_page(struct folio *folio, unsigned long n)
>>>>> +static inline struct page *folio_page(const struct folio *folio, unsigned long n)
>>>>>     {
>>>>> -	return &folio->page + n;
>>>>> +	return (struct page *)(&folio->page + n);
>>>>>     }
>>>>>     
>>>>>     static __always_inline int PageTail(const struct page *page)
>>>>> _
>>>>>
>>>>
>>>>
>>>> I think we should do the following. And I'm wondering if we should squash that
>>>> into the relevant nth_page patch instead:
>>>
>>> Sounds good, seems to be working for page_folio().
>>>
>>>>    From 6ada1d36d18969db587ab647757a9538ee1d09a6 Mon Sep 17 00:00:00 2001
>>>> From: David Hildenbrand <david@redhat.com>
>>>> Date: Thu, 4 Sep 2025 08:35:24 +0200
>>>> Subject: [PATCH] fixup folio_page() const correctness
>>>>
>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>> ---
>>>>     include/linux/page-flags.h | 13 +++++++++----
>>>>     1 file changed, 9 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>>>> index faf17ca211b4f..3a9e73abea3e3 100644
>>>> --- a/include/linux/page-flags.h
>>>> +++ b/include/linux/page-flags.h
>>>> @@ -307,6 +307,12 @@ static __always_inline unsigned long _compound_head(const struct page *page)
>>>>     	const struct page *:	(const struct folio *)_compound_head(p), \
>>>>     	struct page *:		(struct folio *)_compound_head(p)))
>>>>     
>>>> +static inline const struct page *_folio_page(const struct folio *folio,
>>>> +		unsigned long n)
>>>> +{
>>>> +	return &folio->page + n;
>>>
>>> Maybe you can just copy/paste "&(f)->page + (n)" twice directly and not need
>>> this wrapper? Maybe not even the casts as they would follow from the type of
>>> f automatically?
>>
>> Well, then the inline function is gone (which I enjoyed having), and the
>> there would not be much sense in having the generic at all?
> 
> I thought the benefit of the generic is if you pass const folio you get
> const page, and if you pass non-const you get non-stop, and it doesn't
> involve any cast of something already assumed const to non-const which I
> assume is bad (TM). But I don't know how much bad in practice (can it lead
> to optimizations screwing with your code?).
> 
> With the inline function in place there's such a cast const -> non-const on
> the page pointer it returns, but maybe it's fine because it only happens
> when the original folio was non-const? I don't know. It just doesn't seem to
> add benefits when only used via the generic?

Well, take a look at page_folio(), I really just mimic what they do.

I think the point is that with generics you can control that where you 
cast the const of is sane.

In Andrew's patch you just cast the const away unconditionally, which is 
rather nasty.

With generic you could write two functions _folio_page_non_const() + 
_folio_page_const(), but that's just stupid.

A plain

	#define folio_page(f, n) (&(f)->page + (n))	

Would also make the compiler respect the const'ness of f, because 
"->page" is not a pointer.

Now, of course that would change once ->page becomes a pointer (memdescs 
...)

-- 
Cheers

David / dhildenb

Re: + mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch added to mm-unstable branch

[Vlastimil Babka]

On 9/4/25 11:40, David Hildenbrand wrote:
> On 04.09.25 11:28, Vlastimil Babka wrote:
>> On 9/4/25 11:16, David Hildenbrand wrote:
>>> On 04.09.25 09:57, Vlastimil Babka wrote:
>>>> On 9/4/25 08:37, David Hildenbrand wrote:
>>>>> On 04.09.25 04:59, Andrew Morton wrote:
>> I thought the benefit of the generic is if you pass const folio you get
>> const page, and if you pass non-const you get non-stop, and it doesn't
>> involve any cast of something already assumed const to non-const which I
>> assume is bad (TM). But I don't know how much bad in practice (can it lead
>> to optimizations screwing with your code?).
>> 
>> With the inline function in place there's such a cast const -> non-const on
>> the page pointer it returns, but maybe it's fine because it only happens
>> when the original folio was non-const? I don't know. It just doesn't seem to
>> add benefits when only used via the generic?
> 
> Well, take a look at page_folio(), I really just mimic what they do.

Yeah, but that one is different due to _compound_head() returning unsigned long.

> I think the point is that with generics you can control that where you 
> cast the const of is sane.
> 
> In Andrew's patch you just cast the const away unconditionally, which is 
> rather nasty.

Yup I'm in favor of your approach.

> With generic you could write two functions _folio_page_non_const() + 
> _folio_page_const(), but that's just stupid.
> 
> A plain
> 
> 	#define folio_page(f, n) (&(f)->page + (n))	
> 
> Would also make the compiler respect the const'ness of f, because 
> "->page" is not a pointer.

Hm that's true. The generic then only adds some type restrictions.

> Now, of course that would change once ->page becomes a pointer (memdescs 
> ...)

Right. I'm not opposed to your patch, just thought it could be smaller, but
not insisting. Do you want anyone to use _folio_page() directly? If not
maybe add a comment that it's internal and only folio_page() should be used,
to help avoid accidental misuse?

I've also Cc'd willy on my previosus reply in case he has opinions.

Re: + mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch added to mm-unstable branch

[David Hildenbrand]

On 04.09.25 11:48, Vlastimil Babka wrote:
> On 9/4/25 11:40, David Hildenbrand wrote:
>> On 04.09.25 11:28, Vlastimil Babka wrote:
>>> On 9/4/25 11:16, David Hildenbrand wrote:
>>>> On 04.09.25 09:57, Vlastimil Babka wrote:
>>>>> On 9/4/25 08:37, David Hildenbrand wrote:
>>>>>> On 04.09.25 04:59, Andrew Morton wrote:
>>> I thought the benefit of the generic is if you pass const folio you get
>>> const page, and if you pass non-const you get non-stop, and it doesn't
>>> involve any cast of something already assumed const to non-const which I
>>> assume is bad (TM). But I don't know how much bad in practice (can it lead
>>> to optimizations screwing with your code?).
>>>
>>> With the inline function in place there's such a cast const -> non-const on
>>> the page pointer it returns, but maybe it's fine because it only happens
>>> when the original folio was non-const? I don't know. It just doesn't seem to
>>> add benefits when only used via the generic?
>>
>> Well, take a look at page_folio(), I really just mimic what they do.
> 
> Yeah, but that one is different due to _compound_head() returning unsigned long.
> 
>> I think the point is that with generics you can control that where you
>> cast the const of is sane.
>>
>> In Andrew's patch you just cast the const away unconditionally, which is
>> rather nasty.
> 
> Yup I'm in favor of your approach.
> 
>> With generic you could write two functions _folio_page_non_const() +
>> _folio_page_const(), but that's just stupid.
>>
>> A plain
>>
>> 	#define folio_page(f, n) (&(f)->page + (n))	
>>
>> Would also make the compiler respect the const'ness of f, because
>> "->page" is not a pointer.
> 
> Hm that's true. The generic then only adds some type restrictions.
> 
>> Now, of course that would change once ->page becomes a pointer (memdescs
>> ...)
> 
> Right. I'm not opposed to your patch, just thought it could be smaller, but
> not insisting. Do you want anyone to use _folio_page() directly?

I think just like _compound_head it's rather implied that this is only 
for people that know what they are doing, but sure we could add a comment.

> If not
> maybe add a comment that it's internal and only folio_page() should be used,
> to help avoid accidental misuse?

Just to clarify: I'm not married to any of that, but I enjoyed having an 
inline function. So I'm mostly wondering "how could we do better than 
just going back to a plain macro".

I'll note that _compound_head() is rather special as in it essentially 
consumes a "const pointer" but then just looks up another pointer where 
we lose the const-ness.

-- 
Cheers

David / dhildenb

Re: + mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch added to mm-unstable branch

[Matthew Wilcox]

On Thu, Sep 04, 2025 at 11:40:34AM +0200, David Hildenbrand wrote:
> A plain
> 
> 	#define folio_page(f, n) (&(f)->page + (n))	
> 
> Would also make the compiler respect the const'ness of f, because "->page"
> is not a pointer.
> 
> Now, of course that would change once ->page becomes a pointer (memdescs
> ...)

Right, once folio is a separate data structure from page, the constness
of one does not naturally follow the other.  I'm not actually planning
on having a page pointer in folio but rather pfn.  It's my feeling that
we want the pfn of the folio more often than we want the page of the
folio (eg so we can put the pfn in the page table).  Sometimes we want
the kernel virtual address for the page (read() / write()) but we can't
store folio->addr because highmem.  And I think pfn -> addr is more
efficient than page -> addr.

ie I expect folio_page() to be implemented as:

struct page *folio_page(const struct folio *folio, unsigned long n)
{
	return pfn_to_page(folio->pfn + n);
}

Re: + mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch added to mm-unstable branch

[David Hildenbrand]

On 04.09.25 15:06, Matthew Wilcox wrote:
> On Thu, Sep 04, 2025 at 11:40:34AM +0200, David Hildenbrand wrote:
>> A plain
>>
>> 	#define folio_page(f, n) (&(f)->page + (n))	
>>
>> Would also make the compiler respect the const'ness of f, because "->page"
>> is not a pointer.
>>
>> Now, of course that would change once ->page becomes a pointer (memdescs
>> ...)
> 
> Right, once folio is a separate data structure from page, the constness
> of one does not naturally follow the other.  I'm not actually planning
> on having a page pointer in folio but rather pfn.  It's my feeling that
> we want the pfn of the folio more often than we want the page of the
> folio (eg so we can put the pfn in the page table).  Sometimes we want
> the kernel virtual address for the page (read() / write()) but we can't
> store folio->addr because highmem.  And I think pfn -> addr is more
> efficient than page -> addr.
> 
> ie I expect folio_page() to be implemented as:
> 
> struct page *folio_page(const struct folio *folio, unsigned long n)
> {
> 	return pfn_to_page(folio->pfn + n);
> }

Okay, thanks, so we could just exchange that implementation in the 
current _folio_page() proposal.

Thanks!

-- 
Cheers

David / dhildenb

Re: + mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch added to mm-unstable branch

[Andrew Morton]

On Thu, 4 Sep 2025 08:37:11 +0200 David Hildenbrand <david@redhat.com> wrote:

> > --- a/include/linux/page-flags.h~mm-constify-highmem-related-functions-for-improved-const-correctness-fix
> > +++ a/include/linux/page-flags.h
> > @@ -316,9 +316,9 @@ static __always_inline unsigned long _co
> >    * check that the page number lies within @folio; the caller is presumed
> >    * to have a reference to the page.
> >    */
> > -static inline struct page *folio_page(struct folio *folio, unsigned long n)
> > +static inline struct page *folio_page(const struct folio *folio, unsigned long n)
> >   {
> > -	return &folio->page + n;
> > +	return (struct page *)(&folio->page + n);
> >   }
> >   
> >   static __always_inline int PageTail(const struct page *page)
> > _
> > 
> 
> 
> I think we should do the following. And I'm wondering if we should squash that
> into the relevant nth_page patch instead:

Well I didn't merge anything new as a result of this patch and it's
time to get this series into mm-stable.  So I'll use my nasty hack for
now - could someone please prepare a followup to denastify it?

Re: + mm-constify-highmem-related-functions-for-improved-const-correctness-fix.patch added to mm-unstable branch

[David Hildenbrand]

On 21.09.25 22:05, Andrew Morton wrote:
> On Thu, 4 Sep 2025 08:37:11 +0200 David Hildenbrand <david@redhat.com> wrote:
> 
>>> --- a/include/linux/page-flags.h~mm-constify-highmem-related-functions-for-improved-const-correctness-fix
>>> +++ a/include/linux/page-flags.h
>>> @@ -316,9 +316,9 @@ static __always_inline unsigned long _co
>>>     * check that the page number lies within @folio; the caller is presumed
>>>     * to have a reference to the page.
>>>     */
>>> -static inline struct page *folio_page(struct folio *folio, unsigned long n)
>>> +static inline struct page *folio_page(const struct folio *folio, unsigned long n)
>>>    {
>>> -	return &folio->page + n;
>>> +	return (struct page *)(&folio->page + n);
>>>    }
>>>    
>>>    static __always_inline int PageTail(const struct page *page)
>>> _
>>>
>>
>>
>> I think we should do the following. And I'm wondering if we should squash that
>> into the relevant nth_page patch instead:
> 
> Well I didn't merge anything new as a result of this patch and it's
> time to get this series into mm-stable.  So I'll use my nasty hack for
> now - could someone please prepare a followup to denastify it?

I can send a patch.

-- 
Cheers

David / dhildenb