vm_phys_early_alloc(): Switch order of arguments

[olce]

Summary

It seems more intuitive to specify the requested size first, as the
domain argument is "optional" (the value -1).

Additionally, this will improve consistency with the next commit
introducing vm_phys_early_alloc_ex().

[markj]

It seems more intuitive to specify the requested size first.

I don't really see why. The current interface is consistent with other NUMA-aware allocators, e.g., kmem_malloc_domain(), vm_page_alloc_contig_domain(), vm_phys_alloc_pages().

[olce]

In D48634#1109782, @markj wrote:

I don't really see why. The current interface is consistent with other NUMA-aware allocators, e.g., kmem_malloc_domain(), vm_page_alloc_contig_domain(), vm_phys_alloc_pages().

The difference with these NUMA-aware allocators is that here specifying the domain is optional (the -1 value), whereas in the former it is mandatory. I usually prefer that "optional" arguments come after mandatory one. I can change that if it's too disturbing. That said, I'm considering just removing the domain argument of vm_phys_early_alloc(), as most callers pass -1, and convert those that do not to use vm_phys_early_alloc_ex() instead.

Would your concern also apply to vm_phys_early_alloc_ex*() in D48635, which has a long list of arguments maybe only mildly comparable to the functions you cited?

[olce]

Update description, and the diff as the base has changed.

[markj]

In D48634#1114529, @olce wrote:

In D48634#1109782, @markj wrote:

I don't really see why. The current interface is consistent with other NUMA-aware allocators, e.g., kmem_malloc_domain(), vm_page_alloc_contig_domain(), vm_phys_alloc_pages().

The difference with these NUMA-aware allocators is that here specifying the domain is optional (the -1 value), whereas in the former it is mandatory. I usually prefer that "optional" arguments come after mandatory one. I can change that if it's too disturbing. That said, I'm considering just removing the domain argument of vm_phys_early_alloc(), as most callers pass -1, and convert those that do not to use vm_phys_early_alloc_ex() instead.

That sounds better to me. If a patch is going to introduce churn (i.e., make git blame history harder to follow), I'd prefer some stronger justification, such as simplifying code somehow or making it more consistent in a clear way. I will leave it up to you though, since I already read through it.