>>> On 12.10.16 at 14:51, <julien.gr...@arm.com> wrote: > Hello Jan, > > On 12/10/2016 12:45, Jan Beulich wrote: >>>>> On 11.10.16 at 15:39, <julien.gr...@arm.com> wrote: >>> On 06/10/16 13:21, Jan Beulich wrote: >>>>>>> On 05.10.16 at 20:30, <julien.gr...@arm.com> wrote: >>>>> On 30/09/2016 02:46, Jan Beulich wrote: >>>>>>>>> On 29.09.16 at 23:42, <daniel.ki...@oracle.com> wrote: >>>>>>> +#else >>>>>>> +static void __init free_ebmalloc_unused_mem(void) >>>>>>> +{ >>>>>>> +} >>>>>>> +#endif >>>>>> >>>>>> Did you build test this for ARM? The function ought to be unused, >>>>>> as ... >>>>>> >>>>>>> @@ -1251,6 +1301,8 @@ void __init efi_init_memory(void) >>>>>>> } *extra, *extra_head = NULL; >>>>>>> #endif >>>>>>> >>>>>>> + free_ebmalloc_unused_mem(); >>>>>> >>>>>> ... the whole function here doesn't get built on ARM. >>>>>> >>>>>> Julien - we're still awaiting your input on general aspects here. >>>>> >>>>> efi_init_memory would need to be called during Xen boot on ARM. I am not >>>>> sure where as I we don't yet have runtime support on ARM. >>>>> >>>>> Other than that, the patch looks good to me. >>>> >>>> But that wasn't the question. My goal is to have as little code >>>> inside #ifndef CONFIG_ARM as possible, and hence I'd like to have >>>> as much of this new code as possible outside of such conditionals. >>>> So the question really is whether that alternative approach would >>>> be fine with you, or what problems you might see. >>> >>> I am not sure to get it. The current approach looks good to me, however, >>> the implementation should not be exposed to ARM until all the TODOs >>> mentioned by Daniel are fixed. >> >> Which is precisely the opposite of what I'm aiming at. Once again: >> Don't you think it is desirable to keep the #ifndef CONFIG_ARM >> instances to cover as little code as possible? Not all of the named >> TODOs really need to be addressed in order to compile most of >> what comprises this new allocator; in fact none of them really >> needs addressing: >> - if the size estimation turns out to low once ARM starts actually >> using this, let's just bump it (perhaps by making it a per-arch >> constant), >> - if the section chosen needs to be different (which it really >> shouldn't be), let's simply adjust it, > > If we keep the section in BSS, then we really need to move the > initialization of BSS earlier.
Right, but that should be simple enough. Or we do ... > This TODO really needs to be fixed now otherwise it will be a nightmare > to debug later on. > >> - as we've already figured there's no need for the stub >> free_ebmalloc_unused_mem() right now anyway. > > But we would need to call free_ebmalloc_unused_mem from somewhere. The > idea to not expose the early memory allocator on ARM is avoid to have an > implementation with may not fully work on ARM because of known missing > pieces. > >> And then (as another alternative) we have the option of ARM >> simply defining EBMALLOC_SIZE to zero for the time being. That >> would eliminate the need to actually call free_ebmalloc_unused_mem() >> and turn the other two items into non-issues. ... this, which you didn't comment on at all. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel