* Vitaly Wool <[email protected]> [250710 02:21]: > > > > On Jul 9, 2025, at 9:01 PM, Liam R. Howlett <[email protected]> wrote: > > > > * Vitaly Wool <[email protected] <mailto:[email protected]>> > > [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 <[email protected]> > >> Reviewed-by: Uladzislau Rezki (Sony) <[email protected]> > >> Reviewed-by: Vlastimil Babka <[email protected]> > >> --- > >> 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. > > > > 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. >
I'm not trying to be difficult but I am trying to nicely say that this is a mess of words. It's easier to just go find the code and try and figure out what is going on instead of decoding what is written in the description. The pattern garbage. I was trying to tell you this nicely, but your change is better left undocumented than to add the above description. The wording here is difficult to follow for a native English speaker. You don't have to keep stating "this function", that wasn't part of the pattern. There are too many unnecessary negative statements "if @nis is not NUMA_NO_NODE" or "if reallocation is not necessary". The commas indicate that you can rewrite the sentences to be more clear by stating the facts up front. Stating "the latter case" is a barrier to non-native English speakers and is unclear when you have multiple statements crammed into the preceding sentence. And it's easy to fix, but apparently you don't want to change it and would rather follow a broken pattern? Regards, Liam
