Re: [PATCH v1 2/2] mm/highmem: make nr_free_highpages() return "unsigned long"

David Hildenbrand
2024-06-10T08:22:58Z

On 10.06.24 05:40, Oscar Salvador wrote:
> On Fri, Jun 07, 2024 at 10:37:11AM +0200, David Hildenbrand wrote:
>> It looks rather weird that totalhigh_pages() returns an
>> "unsigned long" but nr_free_highpages() returns an "unsigned int".
>>
>> Let's return an "unsigned long" from nr_free_highpages() to be
>> consistent.
>>
>> While at it, use a plain "0" instead of a "0UL" in the !CONFIG_HIGHMEM
>> totalhigh_pages() implementation, to make these look alike as well.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
> ...
>> -static inline unsigned int nr_free_highpages(void) { return 0; }
>> -static inline unsigned long totalhigh_pages(void) { return 0UL; }
>> +static inline unsigned long nr_free_highpages(void) { return 0; }
>> +static inline unsigned long totalhigh_pages(void) { return 0; }
>
> Although I doubt it has any consequences, I would just leave them both with UL,
> so the return type is consistent with what we are returning.

These suffixes are only required when using constants that would not fit
into the native (int) type, or converting from that native (int) type to
something else automatically by the compiler would mess things up (for example,
undesired sign extension). For 0 that is certainly impossible :)

That's also the reason why in include/linux we now have:

t14s: ~/git/linux/include/linux $ git grep "return 0UL;"
skbuff.h: return 0UL;
uaccess.h:static inline unsigned long user_access_save(void) { return 0UL; }
t14s: ~/git/linux/include/linux $ git grep "0UL;"
bitmap.h: *dst = ~0UL;
dax.h: return ~0UL;
mtd/map.h: r.x[i] = ~0UL;
netfilter.h: return ((ul1[0] ^ ul2[0]) | (ul1[1] ^ ul2[1])) == 0UL;
skbuff.h: return 0UL;
uaccess.h:static inline unsigned long user_access_save(void) { return 0UL; }

... compared to a long list if "unsigned long" functions that simply "return 0;"

So I prefer to just drop it.

Thanks!

--
Cheers,

David / dhildenb