Currently, nr_pages is defined as unsigned long. We use nr_pages to
manipulate mm rss counters for lazyfree folios as follows:

	add_mm_counter(mm, MM_ANONPAGES, -nr_pages);

Suppose nr_pages = 3. -nr_pages underflows and becomes ULONG_MAX - 2. Then,
since add_mm_counter() uses this -nr_pages as a long, ULONG_MAX - 2 does
not fit into the positive range of long, and is converted to -3. Eventually
all of this works out, but for keeping things simple, declare nr_pages as
a signed variable.

Signed-off-by: Dev Jain <dev.jain@arm.com>
---
 mm/rmap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/rmap.c b/mm/rmap.c
index 6398d7eef393f..087c9f5b884fe 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1979,9 +1979,10 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
 	struct page *subpage;
 	struct mmu_notifier_range range;
 	enum ttu_flags flags = (enum ttu_flags)(long)arg;
-	unsigned long nr_pages = 1, end_addr;
+	unsigned long end_addr;
 	unsigned long pfn;
 	unsigned long hsz = 0;
+	long nr_pages = 1;
 	int ptes = 0;
 
 	/*
-- 
2.34.1

On Tue, Mar 10, 2026 at 01:00:05PM +0530, Dev Jain wrote:
> Currently, nr_pages is defined as unsigned long. We use nr_pages to
> manipulate mm rss counters for lazyfree folios as follows:
>
> 	add_mm_counter(mm, MM_ANONPAGES, -nr_pages);
>
> Suppose nr_pages = 3. -nr_pages underflows and becomes ULONG_MAX - 2. Then,
> since add_mm_counter() uses this -nr_pages as a long, ULONG_MAX - 2 does
> not fit into the positive range of long, and is converted to -3. Eventually
> all of this works out, but for keeping things simple, declare nr_pages as
> a signed variable.
>
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
>  mm/rmap.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 6398d7eef393f..087c9f5b884fe 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1979,9 +1979,10 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>  	struct page *subpage;
>  	struct mmu_notifier_range range;
>  	enum ttu_flags flags = (enum ttu_flags)(long)arg;
> -	unsigned long nr_pages = 1, end_addr;
> +	unsigned long end_addr;
>  	unsigned long pfn;
>  	unsigned long hsz = 0;
> +	long nr_pages = 1;

This is a non-issue that makes the code confusing, so let's not?

The convention throughout the kernel is nr_pages generally is unsigned because
you can't have negative nr_pages.

>  	int ptes = 0;
>
>  	/*
> --
> 2.34.1
>

On 3/10/26 08:56, Lorenzo Stoakes (Oracle) wrote:
> On Tue, Mar 10, 2026 at 01:00:05PM +0530, Dev Jain wrote:
>> Currently, nr_pages is defined as unsigned long. We use nr_pages to
>> manipulate mm rss counters for lazyfree folios as follows:
>>
>> 	add_mm_counter(mm, MM_ANONPAGES, -nr_pages);
>>
>> Suppose nr_pages = 3. -nr_pages underflows and becomes ULONG_MAX - 2. Then,
>> since add_mm_counter() uses this -nr_pages as a long, ULONG_MAX - 2 does
>> not fit into the positive range of long, and is converted to -3. Eventually
>> all of this works out, but for keeping things simple, declare nr_pages as
>> a signed variable.
>>
>> Signed-off-by: Dev Jain <dev.jain@arm.com>
>> ---
>>  mm/rmap.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index 6398d7eef393f..087c9f5b884fe 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -1979,9 +1979,10 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>>  	struct page *subpage;
>>  	struct mmu_notifier_range range;
>>  	enum ttu_flags flags = (enum ttu_flags)(long)arg;
>> -	unsigned long nr_pages = 1, end_addr;
>> +	unsigned long end_addr;
>>  	unsigned long pfn;
>>  	unsigned long hsz = 0;
>> +	long nr_pages = 1;
> 
> This is a non-issue that makes the code confusing, so let's not?
> 
> The convention throughout the kernel is nr_pages generally is unsigned because
> you can't have negative nr_pages.

Indeed. Documented in:

commit fa17bcd5f65ed702df001579cca8c885fa6bf3e7
Author: Aristeu Rozanski <aris@ruivo.org>
Date:   Tue Aug 26 11:37:21 2025 -0400

    mm: make folio page count functions return unsigned
    
    As raised by Andrew [1], a folio/compound page never spans a negative
    number of pages.  Consequently, let's use "unsigned long" instead of
    "long" consistently for folio_nr_pages(), folio_large_nr_pages() and
    compound_nr().
    
    Using "unsigned long" as return value is fine, because even
    "(long)-folio_nr_pages()" will keep on working as expected.  Using
    "unsigned int" instead would actually break these use cases.
    
    This patch takes the first step changing these to return unsigned long
    (and making drm_gem_get_pages() use the new types instead of replacing
    min()).
    
    In the future, we might want to make more callers of these functions to
    consistently use "unsigned long".
    


-- 
Cheers,

David

On 10/03/26 1:36 pm, David Hildenbrand (Arm) wrote:
> On 3/10/26 08:56, Lorenzo Stoakes (Oracle) wrote:
>> On Tue, Mar 10, 2026 at 01:00:05PM +0530, Dev Jain wrote:
>>> Currently, nr_pages is defined as unsigned long. We use nr_pages to
>>> manipulate mm rss counters for lazyfree folios as follows:
>>>
>>> 	add_mm_counter(mm, MM_ANONPAGES, -nr_pages);
>>>
>>> Suppose nr_pages = 3. -nr_pages underflows and becomes ULONG_MAX - 2. Then,
>>> since add_mm_counter() uses this -nr_pages as a long, ULONG_MAX - 2 does
>>> not fit into the positive range of long, and is converted to -3. Eventually
>>> all of this works out, but for keeping things simple, declare nr_pages as
>>> a signed variable.
>>>
>>> Signed-off-by: Dev Jain <dev.jain@arm.com>
>>> ---
>>>  mm/rmap.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mm/rmap.c b/mm/rmap.c
>>> index 6398d7eef393f..087c9f5b884fe 100644
>>> --- a/mm/rmap.c
>>> +++ b/mm/rmap.c
>>> @@ -1979,9 +1979,10 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>>>  	struct page *subpage;
>>>  	struct mmu_notifier_range range;
>>>  	enum ttu_flags flags = (enum ttu_flags)(long)arg;
>>> -	unsigned long nr_pages = 1, end_addr;
>>> +	unsigned long end_addr;
>>>  	unsigned long pfn;
>>>  	unsigned long hsz = 0;
>>> +	long nr_pages = 1;
>>
>> This is a non-issue that makes the code confusing, so let's not?
>>
>> The convention throughout the kernel is nr_pages generally is unsigned because
>> you can't have negative nr_pages.
> 
> Indeed. Documented in:
> 
> commit fa17bcd5f65ed702df001579cca8c885fa6bf3e7
> Author: Aristeu Rozanski <aris@ruivo.org>
> Date:   Tue Aug 26 11:37:21 2025 -0400
> 
>     mm: make folio page count functions return unsigned
>     
>     As raised by Andrew [1], a folio/compound page never spans a negative
>     number of pages.  Consequently, let's use "unsigned long" instead of
>     "long" consistently for folio_nr_pages(), folio_large_nr_pages() and
>     compound_nr().
>     
>     Using "unsigned long" as return value is fine, because even
>     "(long)-folio_nr_pages()" will keep on working as expected.  Using
>     "unsigned int" instead would actually break these use cases.
>     
>     This patch takes the first step changing these to return unsigned long
>     (and making drm_gem_get_pages() use the new types instead of replacing
>     min()).
>     
>     In the future, we might want to make more callers of these functions to
>     consistently use "unsigned long".

So when I was playing around with the code, I noticed that passing
unsigned int nr_pages to add_mm_counter(-nr_pages) messes up things. Then
I noticed we have an unsigned long here, which prevents that. This is quite
non-trivial information for me, especially when I searched around the
codebase and found this is the only place where we pass a negative unsigned
long.

But thanks for pointing out this commit. If it is a well-known fact that
(long)-folio_nr_pages() will work correctly, then we can drop this patch.

>     
> 
>

On Tue, Mar 10, 2026 at 01:53:21PM +0530, Dev Jain wrote:
> So when I was playing around with the code, I noticed that passing
> unsigned int nr_pages to add_mm_counter(-nr_pages) messes up things. Then

Using int (whether signed or unsigned) to store nr_pages is a bad idea.
Look how many people are asking about supporting PUD-sized folios.
On an ARM 64k PAGE_SIZE machine, that's 2^26 pages which is uncomfortably
close to 2^32.  It'll only take one more level to exceed that, so, what,
five to ten more years?

Just use unsigned long everywhere now and save ourselves the grief later.

On 10/03/26 6:10 pm, Matthew Wilcox wrote:
> On Tue, Mar 10, 2026 at 01:53:21PM +0530, Dev Jain wrote:
>> So when I was playing around with the code, I noticed that passing
>> unsigned int nr_pages to add_mm_counter(-nr_pages) messes up things. Then
> 
> Using int (whether signed or unsigned) to store nr_pages is a bad idea.
> Look how many people are asking about supporting PUD-sized folios.
> On an ARM 64k PAGE_SIZE machine, that's 2^26 pages which is uncomfortably
> close to 2^32.  It'll only take one more level to exceed that, so, what,
> five to ten more years?
> 
> Just use unsigned long everywhere now and save ourselves the grief later.

Thanks, let us do this.