Re: [Xen-devel] [PATCH v6 08/15] x86/efi: create new early memory allocator

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.09.16 at 17:04, <daniel.kiper@xxxxxxxxxx> wrote:
> On Mon, Sep 19, 2016 at 06:12:35AM -0600, Jan Beulich wrote:
>> >>> On 12.09.16 at 22:18, <daniel.kiper@xxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/setup.c
>> > +++ b/xen/arch/x86/setup.c
>> > @@ -520,6 +520,8 @@ static void noinline init_done(void)
>> >
>> >      system_state = SYS_STATE_active;
>> >
>> > +    free_ebmalloc_unused_mem();
>>
>> Now that the allocator properly lives in common code, this appears
>> to lack an ARM side counterpart.
> 
> Why? It is called only from xen/arch/x86/setup.c:__start_xen() and all
> ebmalloc stuff is in #ifndef CONFIG_ARM. So, free_ebmalloc_unused_mem()
> will be needed only if we add ARM support here.

Well, it being inside that conditional is part of the problem - there's
no apparent point for all of it to be. Arguably the one static function
may better be, as other workarounds to avoid the "unused"
compiler warning wouldn't be any better.

>> > +static unsigned long __initdata ebmalloc_allocated;
>> > +
>> > +/* EFI boot allocator. */
>> > +static void __init *ebmalloc(size_t size)
>> > +{
>> > +    void *ptr = ebmalloc_mem + ebmalloc_allocated;
>> > +
>> > +    ebmalloc_allocated += (size + sizeof(void *) - 1) & 
>> > ~((typeof(size))sizeof(void *) - 1);
>>
>> What's the point of this ugly cast?
> 
> In general ALIGN_UP() would be nice here. However, there is no such thing
> in Xen headers (or I cannot find it). Should I add one? As separate patch?

I understand what you want the expression for, but you didn't
answer my question. Or do you not realize that all this cast is
about is a strange way of converting an expression of type
size_t to type size_t?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel