On Thu, Jul 10, 2025 at 08:21:19AM +0200, Vitaly Wool wrote: > > > > On Jul 9, 2025, at 9:01 PM, Liam R. Howlett <liam.howl...@oracle.com> wrote: > > > > * Vitaly Wool <vitaly.w...@konsulko.se <mailto:vitaly.w...@konsulko.se>> > > [250709 13:24]: > >> Reimplement vrealloc() to be able to set node and alignment should > >> a user need to do so. Rename the function to vrealloc_node_align() > >> to better match what it actually does now and introduce macros for > >> vrealloc() and friends for backward compatibility. > >> > >> With that change we also provide the ability for the Rust part of > >> the kernel to set node and alignment in its allocations. > >> > >> Signed-off-by: Vitaly Wool <vitaly.w...@konsulko.se> > >> Reviewed-by: Uladzislau Rezki (Sony) <ure...@gmail.com> > >> Reviewed-by: Vlastimil Babka <vba...@suse.cz> > >> --- > >> include/linux/vmalloc.h | 12 +++++++++--- > >> mm/nommu.c | 3 ++- > >> mm/vmalloc.c | 31 ++++++++++++++++++++++++++----- > >> 3 files changed, 37 insertions(+), 9 deletions(-) > >> > > ... > > > >> diff --git a/mm/vmalloc.c b/mm/vmalloc.c > >> index 6dbcdceecae1..03dd06097b25 100644 > >> --- a/mm/vmalloc.c > >> +++ b/mm/vmalloc.c > >> @@ -4089,19 +4089,31 @@ void *vzalloc_node_noprof(unsigned long size, int > >> node) > >> EXPORT_SYMBOL(vzalloc_node_noprof); > >> > >> /** > >> - * vrealloc - reallocate virtually contiguous memory; contents remain > >> unchanged > >> + * vrealloc_node_align_noprof - reallocate virtually contiguous memory; > >> contents > >> + * remain unchanged > >> * @p: object to reallocate memory for > >> * @size: the size to reallocate > >> + * @align: requested alignment > >> * @flags: the flags for the page level allocator > >> + * @nid: node number of the target node > >> + * > >> + * If @p is %NULL, vrealloc_XXX() behaves exactly like vmalloc(). If > >> @size is > >> + * 0 and @p is not a %NULL pointer, the object pointed to is freed. > >> * > >> - * If @p is %NULL, vrealloc() behaves exactly like vmalloc(). If @size is > >> 0 and > >> - * @p is not a %NULL pointer, the object pointed to is freed. > >> + * if @nid is not NUMA_NO_NODE, this function will try to allocate memory > >> on > >> + * the given node. If reallocation is not necessary (e. g. the new size > >> is less > >> + * than the current allocated size), the current allocation will be > >> preserved > >> + * unless __GFP_THISNODE is set. In the latter case a new allocation on > >> the > >> + * requested node will be attempted.
Agreed with Liam, this is completely unreadable. I think the numa node stuff is unnecesasry, that's pretty much inferred. I'd just go with something like 'if the function can void having to reallocate then it does'. Nice and simple :) > > > > I am having a very hard time understanding what you mean here. What is > > the latter case? > > > > If @nis is !NUMA_NO_NODE, the allocation will be attempted on the given > > node. Then things sort of get confusing. What is the latter case? > > The latter case is __GFP_THISNODE present in flags. That’s the latest > if-clause in this paragraph. > > > >> * > >> * If __GFP_ZERO logic is requested, callers must ensure that, starting > >> with the > >> * initial memory allocation, every subsequent call to this API for the > >> same > >> * memory allocation is flagged with __GFP_ZERO. Otherwise, it is possible > >> that > >> * __GFP_ZERO is not fully honored by this API. > >> * > >> + * If the requested alignment is bigger than the one the *existing* > >> allocation > >> + * has, this function will fail. > >> + * > > > > It might be better to say something like: > > Requesting an alignment that is bigger than the alignment of the > > *existing* allocation will fail. > > > > The whole function description in fact consists of several if-clauses (some > of which are nested) so I am just following the pattern here. Right, but in no sane world is essentially describing a series of if-clauses in a kerneldoc a thing. Just it keep it simple, this is meant to be an overview, people can go read the code if they need details :) > > ~Vitaly Cheers, Lorenzo
