Re: [PATCH v3] EFI/runtime: switch to xv[mz]alloc_array()

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 8/12/25 02:12, Jan Beulich wrote:
> On 12.08.2025 02:19, Daniel P. Smith wrote:
> > On 7/23/25 09:39, Jan Beulich wrote:
> > > Use the more "modern" form, thus doing away with effectively open-coding
> > > xmalloc_array() at the same time. While there is a difference in
> > > generated code, as xmalloc_bytes() forces SMP_CACHE_BYTES alignment, if
> > > code really cared about such higher than default alignment, it should
> > > request so explicitly.
> > While I don't object to the change itself, I think this description is a
> > bit over simplification of the change. If the allocation is under
> > PAGE_SIZE, then they are equivalent, but if it is over the page size
> > there are a few more differences than just cache alignment. It
> > completely changes the underlying allocator. I personally also find it a
> > bit of a stretch to call xmalloc_bytes(size) an open coded version of
> > xmalloc_array(char, size).
> My take is that xmalloc_bytes() should never have existed. Hence why I
> didn't add xzmalloc_bytes() when introducing that family of interfaces.
Right, which would be a valid argument for replacing it with 
xmalloc_array(). Though, I would note that there is an xzalloc_bytes(). 
My concern was that you stated there was an open coding, which had me 
expecting there was a line of the form, xmanlloc_bytes(count * 
size_of_something bigger), being replaced by 
xvmalloc_arryay(something_bigger, count).
IMHO, while the C spec does specify char as 1 byte and thus 
interchangeable, I would agree that from a contextual perspective, 
xmalloc_array() is the more appropriate call. The use of the allocation 
is a character array and not a chunk of bytes for an arbitrary buffer.
> > With a stronger description of the change,
> So what exactly do you mean by "stronger"? I can add that in the unlikely
> event that one of the allocations is (near) PAGE_SIZE or larger, we now
> wouldn't require contiguous memory anymore. Yet based on your comment at
> the top I'm not quite sure if that's what you're after and/or enough to
> satisfy your request.
The phrasing stronger was meant to be more clear on the change/effect, 
specifically that the underlying allocator is being changed when the 
allocation is greater than a PAGE_SIZE. Not necessarily a long 
explanation, just the fact that the allocation will be coming from the 
dom heap allocator as opposed to the xen heap allocator. There are 
implications to changing the allocater, e.g.,  at a minimum the 
allocation order and nonphysical vs. physically contiguous effects. 
Having it noted in the commit makes it more obvious what this change is 
actually doing. Which may not be obvious when seeing the simple line 
changes occurring in the diff. Later, if there is an unexpected 
consequence caused by this change, a stronger commit will be helpful 
with the bisection investigations.
dps

> > Acked-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> Thanks, but will need clarification first as per above.
>
> Jan