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 <dpsm...@apertussolutions.com>
Thanks, but will need clarification first as per above.
Jan
