[PATCH 0/3] mm/rmap: small cleanup for __folio_remove_rmap()

[PATCH 0/3] mm/rmap: small cleanup for __folio_remove_rmap()

[Wei Yang]

During code reading, I found some small places where we can improve based on
the knowledge we already have.

For example, since base folio is handled in another branch, we case use
folio_large_nr_pages() to get the number directly.

Hope I don't mess it up.

Wei Yang (3):
  mm/rmap: not necessary to mask off FOLIO_PAGES_MAPPED
  mm/rmap: could be partially_mapped only after no entire map
  mm/rmap: use folio_large_nr_pages() when we are sure it is a large
    folio

 mm/rmap.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH 1/3] mm/rmap: not necessary to mask off FOLIO_PAGES_MAPPED

[Wei Yang]

At this point, we are sure (nr < ENTIRELY_MAPPED). This means the upper
bits are already cleared.

Not necessary to mask off it.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Rik van Riel <riel@surriel.com>
Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Harry Yoo <harry.yoo@oracle.com>
---
 mm/rmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/rmap.c b/mm/rmap.c
index 1c5988dbd1e7..a927437a56c2 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1749,7 +1749,7 @@ static __always_inline void __folio_remove_rmap(struct folio *folio,
 				nr_pages = folio_large_nr_pages(folio);
 				if (level == PGTABLE_LEVEL_PMD)
 					nr_pmdmapped = nr_pages;
-				nr = nr_pages - (nr & FOLIO_PAGES_MAPPED);
+				nr = nr_pages - nr;
 				/* Raced ahead of another remove and an add? */
 				if (unlikely(nr < 0))
 					nr = 0;
-- 
2.34.1

[PATCH 2/3] mm/rmap: could be partially_mapped only after no entire map

[Wei Yang]

If it is not the last entire map, we are sure the folio is not partially
mapped.

Move the check when there is no entire map.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Rik van Riel <riel@surriel.com>
Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Harry Yoo <harry.yoo@oracle.com>
---
 mm/rmap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/rmap.c b/mm/rmap.c
index a927437a56c2..645d924bfc7d 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1757,9 +1757,9 @@ static __always_inline void __folio_remove_rmap(struct folio *folio,
 				/* An add of ENTIRELY_MAPPED raced ahead */
 				nr = 0;
 			}
-		}
 
-		partially_mapped = nr && nr < nr_pmdmapped;
+			partially_mapped = nr && nr < nr_pmdmapped;
+		}
 		break;
 	default:
 		BUILD_BUG();
-- 
2.34.1

[PATCH 3/3] mm/rmap: use folio_large_nr_pages() when we are sure it is a large folio

[Wei Yang]

Non-large folio is handled at the beginning, so it is a large folio for
sure.

Use folio_large_nr_pages() here like elsewhere.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Rik van Riel <riel@surriel.com>
Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Harry Yoo <harry.yoo@oracle.com>
---
 mm/rmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/rmap.c b/mm/rmap.c
index 645d924bfc7d..1b7011afc663 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1703,7 +1703,7 @@ static __always_inline void __folio_remove_rmap(struct folio *folio,
 			nr = folio_sub_return_large_mapcount(folio, nr_pages, vma);
 			if (!nr) {
 				/* Now completely unmapped. */
-				nr = folio_nr_pages(folio);
+				nr = folio_large_nr_pages(folio);
 			} else {
 				partially_mapped = nr < folio_large_nr_pages(folio) &&
 						   !folio_entire_mapcount(folio);
-- 
2.34.1

Re: [PATCH 1/3] mm/rmap: not necessary to mask off FOLIO_PAGES_MAPPED

[Lorenzo Stoakes]

On Fri, Aug 15, 2025 at 08:49:41AM +0000, Wei Yang wrote:
> At this point, we are sure (nr < ENTIRELY_MAPPED). This means the upper
> bits are already cleared.
>
> Not necessary to mask off it.

Worth saying it's because we are in an if branch conditional on nr <
ENTIRELY_MAPPED, and FOLIO_PAGES_MAPPED is equal to ENTIRELY_MAPPED - 1.

>
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>

I suspect that this is copy/pasta from __folio_add_rmap(), but it may also
be some way of asserting we don't underflow nr.

Anyway I think this is fine because the branch implies this is ok so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> Cc: David Hildenbrand <david@redhat.com>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Rik van Riel <riel@surriel.com>
> Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Harry Yoo <harry.yoo@oracle.com>
> ---
>  mm/rmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 1c5988dbd1e7..a927437a56c2 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1749,7 +1749,7 @@ static __always_inline void __folio_remove_rmap(struct folio *folio,
>  				nr_pages = folio_large_nr_pages(folio);
>  				if (level == PGTABLE_LEVEL_PMD)
>  					nr_pmdmapped = nr_pages;
> -				nr = nr_pages - (nr & FOLIO_PAGES_MAPPED);
> +				nr = nr_pages - nr;
>  				/* Raced ahead of another remove and an add? */
>  				if (unlikely(nr < 0))
>  					nr = 0;
> --
> 2.34.1
>
>

Re: [PATCH 3/3] mm/rmap: use folio_large_nr_pages() when we are sure it is a large folio

[Lorenzo Stoakes]

On Fri, Aug 15, 2025 at 08:49:43AM +0000, Wei Yang wrote:
> Non-large folio is handled at the beginning, so it is a large folio for
> sure.
>
> Use folio_large_nr_pages() here like elsewhere.
>
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>

This looks fine to me, so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> Cc: David Hildenbrand <david@redhat.com>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Rik van Riel <riel@surriel.com>
> Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Harry Yoo <harry.yoo@oracle.com>
> ---
>  mm/rmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 645d924bfc7d..1b7011afc663 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1703,7 +1703,7 @@ static __always_inline void __folio_remove_rmap(struct folio *folio,
>  			nr = folio_sub_return_large_mapcount(folio, nr_pages, vma);
>  			if (!nr) {
>  				/* Now completely unmapped. */
> -				nr = folio_nr_pages(folio);
> +				nr = folio_large_nr_pages(folio);
>  			} else {
>  				partially_mapped = nr < folio_large_nr_pages(folio) &&
>  						   !folio_entire_mapcount(folio);
> --
> 2.34.1
>
>

Re: [PATCH 2/3] mm/rmap: could be partially_mapped only after no entire map

[Lorenzo Stoakes]

On Fri, Aug 15, 2025 at 08:49:42AM +0000, Wei Yang wrote:
> If it is not the last entire map, we are sure the folio is not partially
> mapped.
>
> Move the check when there is no entire map.
>

This one I don't like, you're having to sit and think about why it is that
this would be the case, vs. just unconditionally doing it.

Again, as mentioned on previous series, just because we could do something
doesn't mean we should, unless there's statistically reliable perf data on
something real-world indicating we _must_, code clarity absolutely beats
everything else on importance.

So yeah, sorry but no to this patch, please resend with just the two
reviewed (unless David radically disagrees with me :)

> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Rik van Riel <riel@surriel.com>
> Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Harry Yoo <harry.yoo@oracle.com>
> ---
>  mm/rmap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index a927437a56c2..645d924bfc7d 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1757,9 +1757,9 @@ static __always_inline void __folio_remove_rmap(struct folio *folio,
>  				/* An add of ENTIRELY_MAPPED raced ahead */
>  				nr = 0;
>  			}
> -		}
>
> -		partially_mapped = nr && nr < nr_pmdmapped;
> +			partially_mapped = nr && nr < nr_pmdmapped;
> +		}
>  		break;
>  	default:
>  		BUILD_BUG();
> --
> 2.34.1
>
>

Re: [PATCH 0/3] mm/rmap: small cleanup for __folio_remove_rmap()

[Lorenzo Stoakes]

On Fri, Aug 15, 2025 at 08:49:40AM +0000, Wei Yang wrote:
> During code reading, I found some small places where we can improve based on
> the knowledge we already have.
>
> For example, since base folio is handled in another branch, we case use
> folio_large_nr_pages() to get the number directly.

I think this cover letter is pretty weak, please describe what you are
doing clearly and why.

>
> Hope I don't mess it up.

Please don't put this kind of thing in the cover letter, it gets reproduced
into the actual commit messages and this isn't useful for people reading
through commit messages.

>
> Wei Yang (3):
>   mm/rmap: not necessary to mask off FOLIO_PAGES_MAPPED
>   mm/rmap: could be partially_mapped only after no entire map
>   mm/rmap: use folio_large_nr_pages() when we are sure it is a large
>     folio

I find 1/3 and 3/3 ok, 2/3 I don't like, so please resend series as v2 with
only 1 and 2, propagating tags + improved cover letter please.

FWIW, I build-tested each commit and ran mm self tests + VMA tests against
series and all passed.

>
>  mm/rmap.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> --
> 2.34.1
>
>

Cheers, Lorenzo

Re: [PATCH 1/3] mm/rmap: not necessary to mask off FOLIO_PAGES_MAPPED

[Wei Yang]

On Fri, Aug 15, 2025 at 10:38:32AM +0100, Lorenzo Stoakes wrote:
>On Fri, Aug 15, 2025 at 08:49:41AM +0000, Wei Yang wrote:
>> At this point, we are sure (nr < ENTIRELY_MAPPED). This means the upper
>> bits are already cleared.
>>
>> Not necessary to mask off it.
>
>Worth saying it's because we are in an if branch conditional on nr <
>ENTIRELY_MAPPED, and FOLIO_PAGES_MAPPED is equal to ENTIRELY_MAPPED - 1.
>

Will adjust it.

>>
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>
>I suspect that this is copy/pasta from __folio_add_rmap(), but it may also
>be some way of asserting we don't underflow nr.
>
>Anyway I think this is fine because the branch implies this is ok so:
>
>Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>

Thanks

-- 
Wei Yang
Help you, Help me

Re: [PATCH 0/3] mm/rmap: small cleanup for __folio_remove_rmap()

[Wei Yang]

On Fri, Aug 15, 2025 at 11:11:24AM +0100, Lorenzo Stoakes wrote:
>On Fri, Aug 15, 2025 at 08:49:40AM +0000, Wei Yang wrote:
>> During code reading, I found some small places where we can improve based on
>> the knowledge we already have.
>>
>> For example, since base folio is handled in another branch, we case use
>> folio_large_nr_pages() to get the number directly.
>
>I think this cover letter is pretty weak, please describe what you are
>doing clearly and why.
>

Will adjust it.

>>
>> Hope I don't mess it up.
>
>Please don't put this kind of thing in the cover letter, it gets reproduced
>into the actual commit messages and this isn't useful for people reading
>through commit messages.
>

Sure.


-- 
Wei Yang
Help you, Help me

Re: [PATCH 1/3] mm/rmap: not necessary to mask off FOLIO_PAGES_MAPPED

[David Hildenbrand]

On 15.08.25 10:49, Wei Yang wrote:
> At this point, we are sure (nr < ENTIRELY_MAPPED). This means the upper
> bits are already cleared.
> 
> Not necessary to mask off it.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Rik van Riel <riel@surriel.com>
> Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Harry Yoo <harry.yoo@oracle.com>
> ---
>   mm/rmap.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 1c5988dbd1e7..a927437a56c2 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1749,7 +1749,7 @@ static __always_inline void __folio_remove_rmap(struct folio *folio,
>   				nr_pages = folio_large_nr_pages(folio);
>   				if (level == PGTABLE_LEVEL_PMD)
>   					nr_pmdmapped = nr_pages;
> -				nr = nr_pages - (nr & FOLIO_PAGES_MAPPED);
> +				nr = nr_pages - nr;
>   				/* Raced ahead of another remove and an add? */
>   				if (unlikely(nr < 0))
>   					nr = 0;

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb

Re: [PATCH 2/3] mm/rmap: could be partially_mapped only after no entire map

[David Hildenbrand]

On 15.08.25 12:08, Lorenzo Stoakes wrote:
> On Fri, Aug 15, 2025 at 08:49:42AM +0000, Wei Yang wrote:
>> If it is not the last entire map, we are sure the folio is not partially
>> mapped.
>>
>> Move the check when there is no entire map.
>>
> 
> This one I don't like, you're having to sit and think about why it is that
> this would be the case, vs. just unconditionally doing it.
> 
> Again, as mentioned on previous series, just because we could do something
> doesn't mean we should, unless there's statistically reliable perf data on
> something real-world indicating we _must_, code clarity absolutely beats
> everything else on importance.
> 
> So yeah, sorry but no to this patch, please resend with just the two
> reviewed (unless David radically disagrees with me :)

The compiler can figure out that "nr == 0" if the "if (last)" branch is 
not taken.

So I tend to prefer keeping it as is.
-- 
Cheers

David / dhildenb

Re: [PATCH 3/3] mm/rmap: use folio_large_nr_pages() when we are sure it is a large folio

[David Hildenbrand]

On 15.08.25 10:49, Wei Yang wrote:
> Non-large folio is handled at the beginning, so it is a large folio for
> sure.
> 
> Use folio_large_nr_pages() here like elsewhere.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Rik van Riel <riel@surriel.com>
> Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Harry Yoo <harry.yoo@oracle.com>
> ---

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb

Re: [PATCH 2/3] mm/rmap: could be partially_mapped only after no entire map

[Wei Yang]

On Sat, Aug 16, 2025 at 08:31:59AM +0200, David Hildenbrand wrote:
>On 15.08.25 12:08, Lorenzo Stoakes wrote:
>> On Fri, Aug 15, 2025 at 08:49:42AM +0000, Wei Yang wrote:
>> > If it is not the last entire map, we are sure the folio is not partially
>> > mapped.
>> > 
>> > Move the check when there is no entire map.
>> > 
>> 
>> This one I don't like, you're having to sit and think about why it is that
>> this would be the case, vs. just unconditionally doing it.
>> 
>> Again, as mentioned on previous series, just because we could do something
>> doesn't mean we should, unless there's statistically reliable perf data on
>> something real-world indicating we _must_, code clarity absolutely beats
>> everything else on importance.
>> 
>> So yeah, sorry but no to this patch, please resend with just the two
>> reviewed (unless David radically disagrees with me :)
>
>The compiler can figure out that "nr == 0" if the "if (last)" branch is not
>taken.
>

Per my understanding, last is a run time value. I don't figure out how
compiler could help here.

I may miss something. Would you mind giving more hint?

>So I tend to prefer keeping it as is.
>-- 
>Cheers
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me

Re: [PATCH 2/3] mm/rmap: could be partially_mapped only after no entire map

[David Hildenbrand]

On 16.08.25 11:06, Wei Yang wrote:
> On Sat, Aug 16, 2025 at 08:31:59AM +0200, David Hildenbrand wrote:
>> On 15.08.25 12:08, Lorenzo Stoakes wrote:
>>> On Fri, Aug 15, 2025 at 08:49:42AM +0000, Wei Yang wrote:
>>>> If it is not the last entire map, we are sure the folio is not partially
>>>> mapped.
>>>>
>>>> Move the check when there is no entire map.
>>>>
>>>
>>> This one I don't like, you're having to sit and think about why it is that
>>> this would be the case, vs. just unconditionally doing it.
>>>
>>> Again, as mentioned on previous series, just because we could do something
>>> doesn't mean we should, unless there's statistically reliable perf data on
>>> something real-world indicating we _must_, code clarity absolutely beats
>>> everything else on importance.
>>>
>>> So yeah, sorry but no to this patch, please resend with just the two
>>> reviewed (unless David radically disagrees with me :)
>>
>> The compiler can figure out that "nr == 0" if the "if (last)" branch is not
>> taken.
>>
> 
> Per my understanding, last is a run time value. I don't figure out how
> compiler could help here.
> 
> I may miss something. Would you mind giving more hint?

The compiler can figure out that it can move the whole statement to the "if (last)" branch because it knows that nr == 0 otherwise and partially_mapped == 0 already.

Best to see if there is any actual change in generated code, though.

Note that if we're already moving that around, couldn't we move it further in?

diff --git a/mm/rmap.c b/mm/rmap.c
index 0e9c4041f8687..fb83db88cd1fd 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1753,13 +1753,13 @@ static __always_inline void __folio_remove_rmap(struct folio *folio,
                                 /* Raced ahead of another remove and an add? */
                                 if (unlikely(nr < 0))
                                         nr = 0;
+                               partially_mapped = nr && nr < nr_pmdmapped;
                         } else {
                                 /* An add of ENTIRELY_MAPPED raced ahead */
                                 nr = 0;
                         }
                 }
  
-               partially_mapped = nr && nr < nr_pmdmapped;
                 break;
         default:
                 BUILD_BUG();


-- 
Cheers

David / dhildenb

Re: [PATCH 2/3] mm/rmap: could be partially_mapped only after no entire map

[Giorgi Tchankvetadze]

Since the compiler can prove the expression will always be false in one 
branch, there's no need to execute that computation at all in that 
branch, right?

On 8/16/2025 1:16 PM, David Hildenbrand wrote:
> The compiler can figure out that it can move the whole statement to the 
> "if (last)" branch because it knows that nr == 0 otherwise and 
> partially_mapped == 0 already.

Re: [PATCH 2/3] mm/rmap: could be partially_mapped only after no entire map

[David Hildenbrand]

Hi,

please don't top-post.

On 16.08.25 12:05, Giorgi Tchankvetadze wrote:
> Since the compiler can prove the expression will always be false in one
> branch, there's no need to execute that computation at all in that
> branch, right?

It's mostly a matter of making the code easier to read. The way we have 
it right now, we calculate partially_mapped in all cases just before the 
end of the case.

For example, partially_mapped is irrelevant for !folio_test_anon() and 
!folio_test_large(), but we rely on the compiler to figure that out and 
optimize the code accordingly.

Having something like

if (folio_test_large(folio) && folio_test_anon(folio) &&
     !folio_test_partially_mapped(folio)) {
	switch (level) {
	case PGTABLE_LEVEL_PTE:
		if (IS_ENABLED(CONFIG_NO_PAGE_MAPCOUNT)) {
			...
			break;
		}
		partially_mapped = ...
	case PGTABLE_LEVEL_PMD:
		...
	}
	if (partially_mapped)
		deferred_split_folio(folio, true);
}

Could be done, but it's arguably harder to read.

> 
> On 8/16/2025 1:16 PM, David Hildenbrand wrote:
>> The compiler can figure out that it can move the whole statement to the
>> "if (last)" branch because it knows that nr == 0 otherwise and
>> partially_mapped == 0 already.
> 


-- 
Cheers

David / dhildenb

Re: [PATCH 2/3] mm/rmap: could be partially_mapped only after no entire map

[Wei Yang]

On Sat, Aug 16, 2025 at 11:16:37AM +0200, David Hildenbrand wrote:
>On 16.08.25 11:06, Wei Yang wrote:
>> On Sat, Aug 16, 2025 at 08:31:59AM +0200, David Hildenbrand wrote:
>> > On 15.08.25 12:08, Lorenzo Stoakes wrote:
>> > > On Fri, Aug 15, 2025 at 08:49:42AM +0000, Wei Yang wrote:
>> > > > If it is not the last entire map, we are sure the folio is not partially
>> > > > mapped.
>> > > > 
>> > > > Move the check when there is no entire map.
>> > > > 
>> > > 
>> > > This one I don't like, you're having to sit and think about why it is that
>> > > this would be the case, vs. just unconditionally doing it.
>> > > 
>> > > Again, as mentioned on previous series, just because we could do something
>> > > doesn't mean we should, unless there's statistically reliable perf data on
>> > > something real-world indicating we _must_, code clarity absolutely beats
>> > > everything else on importance.
>> > > 
>> > > So yeah, sorry but no to this patch, please resend with just the two
>> > > reviewed (unless David radically disagrees with me :)
>> > 
>> > The compiler can figure out that "nr == 0" if the "if (last)" branch is not
>> > taken.
>> > 
>> 
>> Per my understanding, last is a run time value. I don't figure out how
>> compiler could help here.
>> 
>> I may miss something. Would you mind giving more hint?
>
>The compiler can figure out that it can move the whole statement to the "if (last)" branch because it knows that nr == 0 otherwise and partially_mapped == 0 already.

Wow, this is interesting. Compiler is smarter than I thought.

Thanks

>
>Best to see if there is any actual change in generated code, though.
>

Take a look into the generated code, there is no difference.

>Note that if we're already moving that around, couldn't we move it further in?
>

Yes, I have thought about this. But I was afraid it was too aggressive.

While after a second look, this may make the logic similar with
CONFIG_NO_PAGE_MAPCOUNT. I personally like this one, but it seems not a big
deal.

>diff --git a/mm/rmap.c b/mm/rmap.c
>index 0e9c4041f8687..fb83db88cd1fd 100644
>--- a/mm/rmap.c
>+++ b/mm/rmap.c
>@@ -1753,13 +1753,13 @@ static __always_inline void __folio_remove_rmap(struct folio *folio,
>                                /* Raced ahead of another remove and an add? */
>                                if (unlikely(nr < 0))
>                                        nr = 0;
>+                               partially_mapped = nr && nr < nr_pmdmapped;
>                        } else {
>                                /* An add of ENTIRELY_MAPPED raced ahead */
>                                nr = 0;
>                        }
>                }
>-               partially_mapped = nr && nr < nr_pmdmapped;
>                break;
>        default:
>                BUILD_BUG();
>
>
>-- 
>Cheers
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me