Re: [PATCH v3 1/2] x86/mm: Handle alloc failure in phys_*_init()

[Em Sharnoff <sharnoff@neon.tech>]

On 6/11/25 15:16, Dave Hansen wrote:
> On 6/11/25 01:38, Em Sharnoff wrote:
>>> Could you please find a way to reduce the number of casts?
>> What do you think about changing the return for these functions to just 'int'
>> for errors?
> Fine with me. No reason to cram errno's into a physical address that's
> never used as a physical address.

Just realized paddr_last is actually used to set 'max_pfn_mapped'.
In init_memory_mapping():

> add_pfn_range_mapped(start >> PAGE_SHIFT, ret >> PAGE_SHIFT);

which in turn only uses it to update max_pfn_mapped:

> max_pfn_mapped = max(max_pfn_mapped, end_pfn)

This was added in cc6150321903 ("x86: account overlapped mappings in
max_pfn_mapped").

---

Some other options to reduce the number of casts:

1. Add helpers to do the '(void *)' casting for ERR_PTR, keeping everything
   else the same.
2. Change the phys_*_init() functions to return int, and directly update
   max_pfn_mapped from within them. They already call update_page_count(),
   maybe this is similar?
3. Change the phys_*_init() functions to return int, and calculate the
   expected paddr_last externally.

The third option I think is possible in theory, but probably too complicated
and fragile. (at a glance, there's complex emergent logic - but maybe someone
familiar with the code could make the case for something simple)


Thoughts?

Em