On 25.11.2020 13:15, Julien Grall wrote: > On 23/11/2020 14:23, Jan Beulich wrote: >> All of the array allocations in grant_table_init() can exceed a page's >> worth of memory, which xmalloc()-based interfaces aren't really suitable >> for after boot. We also don't need any of these allocations to be >> physically contiguous.. Introduce interfaces dynamically switching >> between xmalloc() et al and vmalloc() et al, based on requested size, >> and use them instead. >> >> All the wrappers in the new header get cloned mostly verbatim from >> xmalloc.h, with the sole adjustment to switch unsigned long to size_t >> for sizes and to unsigned int for alignments. >> >> Signed-off-by: Jan Beulich <jbeul...@suse.com> >> --- >> v2: Actually edit a copy-and-pasted comment in xvmalloc.h which was >> meant to be edited from the beginning. >> --- >> I'm unconvinced of the mentioning of "physically contiguous" in the >> comment at the top of the new header: I don't think xmalloc() provides >> such a guarantee. Any use assuming so would look (latently) broken to >> me. > > I haven't had the chance to reply to the first version about this. So I > will reply here to avoid confusion. > > I can at least spot one user in Arm that would use xmalloc() that way > (see the allocation of itt_addr in arch/arm/gic-v3-its.c).
And I surely wouldn't have spotted this, even if I had tried to find "offenders", i.e. as said before not wanting to alter the behavior of existing code (beyond the explicit changes done here) was ... > AFAIK, the memory is for the sole purpose of the ITS and should not be > accessed by Xen. So I think we can replace by a new version of > alloc_domheap_pages(). > > However, I still question the usefulness of introducing yet another way > to allocate memory (we already have alloc_xenheap_pages(), xmalloc(), > alloc_domheap_pages(), vmap()) if you think users cannot rely on > xmalloc() to allocate memory physically contiguous. ... the reason to introduce a separate new interface. Plus of course this parallels what Linux has. > It definitely makes more difficult to figure out when to use xmalloc() > vs xvalloc(). I don't see the difficulty: - if you need physically contiguous memory, use alloc_xen*_pages(), - if you know the allocation size is always no more than a page, use xmalloc(), - if you know the allocation size is always more than a page, use vmalloc(), - otherwise use xvmalloc(). Exceptions may of course apply, i.e. this is just a rule of thumb. > I would like to hear an opinion from the other maintainers. Let's hope at least one will voice theirs. Jan
