Re: [PATCH] mm: make folio page count functions return unsigned

[David Hildenbrand <david@redhat.com>]

On 26.08.25 17:37, Aristeu Rozanski wrote:
> 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".
> 
> [1] https://lore.kernel.org/linux-mm/20250503182858.5a02729fcffd6d4723afcfc2@linux-foundation.org/
> 
> Link: https://lore.kernel.org/linux-mm/20250503182858.5a02729fcffd6d4723afcfc2@linux-foundation.org/
> Signed-off-by: Aristeu Rozanski <aris@ruivo.org>
> Suggested-by: Andrew Morton <akpm@linux-foundation.org>
> Suggested-by: David Hildenbrand <david@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Simona Vetter <simona@ffwll.ch>
> 

TL;DR

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



I agree with Andrew that using an unsigned type is clearer, and it confused me in
the past.

When I last looked at that, I realized that "unsigned int" is problematic
(the hard way ...)  and concluded that "unsigned long" should work as well.

commit 1ea5212aed0682ae1249cba9df7ad7ef539d0a7d
Author: David Hildenbrand <david@redhat.com>
Date:   Mon Mar 3 17:29:55 2025 +0100

     mm: factor out large folio handling from folio_nr_pages() into folio_large_nr_pages()
     
     Let's factor it out into a simple helper function.  This helper will also
     come in handy when working with code where we know that our folio is
     large.
     
     While at it, let's consistently return a "long" value from all these
     similar functions.  Note that we cannot use "unsigned int" (even though
     _folio_nr_pages is of that type), because it would break some callers that
     do stuff like "-folio_nr_pages()".  Both "int" or "unsigned long" would
     work as well.


compound_nr() used "unsigned long" before.

I did a quick test below to make sure I am not missing something.

"unsigned long" works as expected except. The only exception would be 32bit
when we would have a function that expects a signed or unsigned "long long".

Should we be using "unsigned long long"? I don't think so.


Using long long / unsigned long long for something nr_pages (or PFN) related
is highly questionable and should be cleaned up.

Because I am curious, si I looked around (I hope I caught most cases):

$ git grep -e "\- *folio_nr_pages"
include/linux/mm_inline.h:                      -folio_nr_pages(folio));

-> consumes "long nr_pages"

include/linux/vmstat.h: __mod_zone_page_state(folio_zone(folio), item, -folio_nr_pages(folio));
include/linux/vmstat.h: mod_zone_page_state(folio_zone(folio), item, -folio_nr_pages(folio));
include/linux/vmstat.h: __mod_node_page_state(folio_pgdat(folio), item, -folio_nr_pages(folio));
include/linux/vmstat.h: mod_node_page_state(folio_pgdat(folio), item, -folio_nr_pages(folio));

-> consume "long delta"

include/linux/vmstat.h: __lruvec_stat_mod_folio(folio, idx, -folio_nr_pages(folio));
include/linux/vmstat.h: lruvec_stat_mod_folio(folio, idx, -folio_nr_pages(folio));

-> Consume "int val"

mm/khugepaged.c:                        -folio_nr_pages(folio));

-> Consumes "long nr"

mm/memcontrol-v1.c:     memcg1_charge_statistics(memcg, -folio_nr_pages(folio));

-> Consumes "int nr_pages"

mm/migrate.c:                                   folio_is_file_lru(folio), -folio_nr_pages(folio));

-> consume "long delta"

mm/migrate.c:   folio_ref_unfreeze(src, expected_count - folio_nr_pages(src));

-> expected_count is an "int"

mm/migrate.c:                               folio_is_file_lru(src), -folio_nr_pages(src));

-> Consumes "long nr"


$ git grep -e "\-= *folio_nr_pages"
fs/bcachefs/fs-io-buffered.c:                   ractl->_index -= folio_nr_pages(folio);

-> "pgoff_t _index" -> "unsigned long"

fs/btrfs/extent_io.c:                   wbc->nr_to_write -= folio_nr_pages(folio);
fs/ext4/inode.c:        mpd->wbc->nr_to_write -= folio_nr_pages(folio);
fs/ext4/inode.c:        mpd->wbc->nr_to_write -= folio_nr_pages(folio);

-> "long nr_to_write"

mm/memory-failure.c:            count -= folio_nr_pages(page_folio(p));

-> "int count"

mm/page-writeback.c:            wbc->nr_to_write -= folio_nr_pages(folio);

-> "long nr_to_write"

--

$ gcc tst.c -o tst; ./tst
Testing: receive_unsigned_long_long(-return_unsigned_long_long())
         PASS
Testing: receive_unsigned_long_long(-return_long_long())
         PASS
Testing: receive_unsigned_long_long(-return_unsigned_long())
         PASS
Testing: receive_unsigned_long_long(-return_long())
         PASS
Testing: receive_unsigned_long_long(-return_unsigned_int())
         FAIL
Testing: receive_unsigned_long_long(-return_int())
         PASS
Testing: receive_long_long(-return_unsigned_long_long())
         PASS
Testing: receive_long_long(-return_long_long())
         PASS
Testing: receive_long_long(-return_unsigned_long())
         PASS
Testing: receive_long_long(-return_long())
         PASS
Testing: receive_long_long(-return_unsigned_int())
         FAIL
Testing: receive_long_long(-return_int())
         PASS
Testing: receive_unsigned_long(-return_unsigned_long_long())
         PASS
Testing: receive_unsigned_long(-return_long_long())
         PASS
Testing: receive_unsigned_long(-return_unsigned_long())
         PASS
Testing: receive_unsigned_long(-return_long())
         PASS
Testing: receive_unsigned_long(-return_unsigned_int())
         FAIL
Testing: receive_unsigned_long(-return_int())
         PASS
Testing: receive_long(-return_unsigned_long_long())
         PASS
Testing: receive_long(-return_long_long())
         PASS
Testing: receive_long(-return_unsigned_long())
         PASS
Testing: receive_long(-return_long())
         PASS
Testing: receive_long(-return_unsigned_int())
         FAIL
Testing: receive_long(-return_int())
         PASS
Testing: receive_unsigned_int(-return_unsigned_long_long())
         PASS
Testing: receive_unsigned_int(-return_long_long())
         PASS
Testing: receive_unsigned_int(-return_unsigned_long())
         PASS
Testing: receive_unsigned_int(-return_long())
         PASS
Testing: receive_unsigned_int(-return_unsigned_int())
         PASS
Testing: receive_unsigned_int(-return_int())
         PASS
Testing: receive_int(-return_unsigned_long_long())
         PASS
Testing: receive_int(-return_long_long())
         PASS
Testing: receive_int(-return_unsigned_long())
         PASS
Testing: receive_int(-return_long())
         PASS
Testing: receive_int(-return_unsigned_int())
         PASS
Testing: receive_int(-return_int())
         PASS

$ gcc -m32 tst.c -o tst; ./tst
Testing: receive_unsigned_long_long(-return_unsigned_long_long())
         PASS
Testing: receive_unsigned_long_long(-return_long_long())
         PASS
Testing: receive_unsigned_long_long(-return_unsigned_long())
         FAIL
Testing: receive_unsigned_long_long(-return_long())
         PASS
Testing: receive_unsigned_long_long(-return_unsigned_int())
         FAIL
Testing: receive_unsigned_long_long(-return_int())
         PASS
Testing: receive_long_long(-return_unsigned_long_long())
         PASS
Testing: receive_long_long(-return_long_long())
         PASS
Testing: receive_long_long(-return_unsigned_long())
         FAIL
Testing: receive_long_long(-return_long())
         PASS
Testing: receive_long_long(-return_unsigned_int())
         FAIL
Testing: receive_long_long(-return_int())
         PASS
Testing: receive_unsigned_long(-return_unsigned_long_long())
         PASS
Testing: receive_unsigned_long(-return_long_long())
         PASS
Testing: receive_unsigned_long(-return_unsigned_long())
         PASS
Testing: receive_unsigned_long(-return_long())
         PASS
Testing: receive_unsigned_long(-return_unsigned_int())
         PASS
Testing: receive_unsigned_long(-return_int())
         PASS
Testing: receive_long(-return_unsigned_long_long())
         PASS
Testing: receive_long(-return_long_long())
         PASS
Testing: receive_long(-return_unsigned_long())
         PASS
Testing: receive_long(-return_long())
         PASS
Testing: receive_long(-return_unsigned_int())
         PASS
Testing: receive_long(-return_int())
         PASS
Testing: receive_unsigned_int(-return_unsigned_long_long())
         PASS
Testing: receive_unsigned_int(-return_long_long())
         PASS
Testing: receive_unsigned_int(-return_unsigned_long())
         PASS
Testing: receive_unsigned_int(-return_long())
         PASS
Testing: receive_unsigned_int(-return_unsigned_int())
         PASS
Testing: receive_unsigned_int(-return_int())
         PASS
Testing: receive_int(-return_unsigned_long_long())
         PASS
Testing: receive_int(-return_long_long())
         PASS
Testing: receive_int(-return_unsigned_long())
         PASS
Testing: receive_int(-return_long())
         PASS
Testing: receive_int(-return_unsigned_int())
         PASS
Testing: receive_int(-return_int())
         PASS

--

#include <stdio.h>
#include <stdbool.h>

volatile int orig_val = 1234;
volatile int receive_val = -1234;

static unsigned long long return_unsigned_long_long(void)
{
	return orig_val;
}

static long long return_long_long(void)
{
	return orig_val;
}

static unsigned long return_unsigned_long(void)
{
	return orig_val;
}

static long return_long(void)
{
	return orig_val;
}

static unsigned int return_unsigned_int(void)
{
	return orig_val;
}

static int return_int(void)
{
	return orig_val;
}

static bool receive_unsigned_long_long(unsigned long long val)
{
	return val == receive_val;
}

static bool receive_long_long(long long val)
{
	return val == receive_val;
}

static bool receive_unsigned_long(unsigned long val)
{
	return val == receive_val;
}

static bool receive_long(long val)
{
	return val == receive_val;
}

static bool receive_unsigned_int(unsigned int val)
{
	return val == receive_val;
}

static bool receive_int(int val)
{
	return val == receive_val;
}

#define TEST(cmd) \
	do { \
		printf("Testing: " #cmd "\n"); \
		if (cmd) \
			printf("\tPASS\n"); \
		else \
			printf("\tFAIL\n"); \
	} while(0)

int main(int ac, char** av) {
	TEST(receive_unsigned_long_long(-return_unsigned_long_long()));
	TEST(receive_unsigned_long_long(-return_long_long()));
	TEST(receive_unsigned_long_long(-return_unsigned_long()));
	TEST(receive_unsigned_long_long(-return_long()));
	TEST(receive_unsigned_long_long(-return_unsigned_int()));
	TEST(receive_unsigned_long_long(-return_int()));
	TEST(receive_long_long(-return_unsigned_long_long()));
	TEST(receive_long_long(-return_long_long()));
	TEST(receive_long_long(-return_unsigned_long()));
	TEST(receive_long_long(-return_long()));
	TEST(receive_long_long(-return_unsigned_int()));
	TEST(receive_long_long(-return_int()));
	TEST(receive_unsigned_long(-return_unsigned_long_long()));
	TEST(receive_unsigned_long(-return_long_long()));
	TEST(receive_unsigned_long(-return_unsigned_long()));
	TEST(receive_unsigned_long(-return_long()));
	TEST(receive_unsigned_long(-return_unsigned_int()));
	TEST(receive_unsigned_long(-return_int()));
	TEST(receive_long(-return_unsigned_long_long()));
	TEST(receive_long(-return_long_long()));
	TEST(receive_long(-return_unsigned_long()));
	TEST(receive_long(-return_long()));
	TEST(receive_long(-return_unsigned_int()));
	TEST(receive_long(-return_int()));
	TEST(receive_unsigned_int(-return_unsigned_long_long()));
	TEST(receive_unsigned_int(-return_long_long()));
	TEST(receive_unsigned_int(-return_unsigned_long()));
	TEST(receive_unsigned_int(-return_long()));
	TEST(receive_unsigned_int(-return_unsigned_int()));
	TEST(receive_unsigned_int(-return_int()));
	TEST(receive_int(-return_unsigned_long_long()));
	TEST(receive_int(-return_long_long()));
	TEST(receive_int(-return_unsigned_long()));
	TEST(receive_int(-return_long()));
	TEST(receive_int(-return_unsigned_int()));
	TEST(receive_int(-return_int()));
	return 0;
}

-- 
Cheers

David / dhildenb