Re: [PATCH v4] mm/vmalloc: make vm_struct.nr_pages an unsigned long

Uladzislau Rezki <[email protected]>
Newsgroups gmane.linux.kernel,gmane.linux.kernel.mm
Message-ID <anRK-y0nBUnbli_u@milan>
On Mon, Aug 03, 2026 at 05:39:35PM -0700, Andrew Morton wrote:
> On Sun, 2 Aug 2026 17:52:26 +0200 Uladzislau Rezki <[email protected]> wrote:
> 
> > On Sat, Aug 01, 2026 at 11:52:02AM -0700, Andrew Morton wrote:
> > > On Sat,  1 Aug 2026 14:49:15 +0300 Artem Lytkin <[email protected]> wrote:
> > > 
> >
> > ...
> >
> > > Ulad, AI review suggests that vrealloc() has an issue handling
> > > __GFP_ZERO.  Can you please check?
> > > 
> > > 	https://sashiko.dev/#/patchset/[email protected]
> > > 
> > I have checked. I think the AI is missing at least one point.
> > AI argument which is:
> > 
> > <snip>
> > If a driver initially allocates memory using vmalloc() without __GFP_ZERO 
> > (leaving spare page capacity uninitialized), and then grows the allocation 
> > using vrealloc() with __GFP_ZERO, the caller expects the newly exposed bytes 
> > to be zeroed.
> > <snip>
> > 
> > In the vrealloc_node_align_noprof() header documentation there is a statement:
> > 
> > <snip>
> >  * 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.
> > <snip>
> > 
> > AI argument violates the documentation, i.e. mixing __GFP_ZERO is not allowed.
> 
> Sashiko is talking about the initial allocation not using __GFP_ZERO
> but vrealloc() *does* use __GFP_ZERO.  The documentation you quoted
> doesn't address that case?
> 
But this is not allowed according to doc :)

<snip>
...
callers must ensure that, starting with the initial memory allocation
...
<snip>

Also, AI describes the situation like:

vmalloc()
vrealloc(grow, since need more) 

i.e. from the description:
<snip>
and then grows the allocation using vrealloc() with __GFP_ZERO, the caller
expects the newly exposed bytes to be zeroed.
<snip>

and it will be zeroed in fact, because the path would be:

<snip>
need_realloc:
	/* TODO: Grow the vm_area, i.e. allocate and map additional pages. */
	n = __vmalloc_node_noprof(size, align, flags, nid, __builtin_return_address(0));

	if (!n)
		return NULL;
<snip>

and not the one which AI pointed to.

The real scenario is:

1. vmalloc(!GFP_ZERO) - alloc size 10
2. vrealloc(!GFP_ZERO) - realloc to size 5
3. vrealloc(GFP_ZERO) - realloc back to 10.

3 - will not zeroed. As noted in doc ZERO should be used starting
from the beginning [1]. But in __most__ cases it will be zeroed
anyway because we free/unmap tail pages and if:

<snip>
		/*
		 * Free tail pages when shrink crosses a page boundary.
		 *
		 * Skip huge page allocations (page_order > 0) as partial
		 * freeing would require splitting.
		 *
		 * Skip VM_FLUSH_RESET_PERMS, as direct-map permissions must
		 * be reset before pages are returned to the allocator.
		 *
		 * Skip VM_USERMAP, as remap_vmalloc_range_partial() validates
		 * mapping requests against the unchanged vm->size; freeing
		 * tail pages would cause vmalloc_to_page() to return NULL for
		 * the unmapped range.
		 *
		 * Skip if either GFP_NOFS or GFP_NOIO are used.
		 * kmemleak_free_part() internally allocates with
		 * GFP_KERNEL, which could trigger a recursive deadlock
		 * if we are under filesystem or I/O reclaim.
		 */
		if (new_nr_pages < vm->nr_pages && !vm_area_page_order(vm) &&
		    !(vm->flags & (VM_FLUSH_RESET_PERMS | VM_USERMAP)) &&
		    gfp_has_io_fs(flags)) {
<snip>

is true the next grow with GFP_ZERO will be zeroed.

For huge alloc it will not be zeroed and for other conditions.

> Also, developers don't read documentation ;) What happens if some
> caller *does* use vmalloc(!__GFP_ZERO) then vrealloc(__GFP_ZERO)? 
> Silent misbehavior would be bad - it would be good if vrealloc() were
> to drop a WARN() then ignore the __GFP_ZERO.
> 
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -4294,11 +4294,6 @@ EXPORT_SYMBOL(vzalloc_node_noprof);
> >   * __GFP_THISNODE flag should be set, otherwise the function will try to avoid
> >   * reallocation and possibly disregard the specified @nid.
> >   *
> > - * 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.
> > - *
> >   * Requesting an alignment that is bigger than the alignment of the existing
> >   * allocation will fail.
> >   *
> > @@ -4415,13 +4410,12 @@ void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align
> >  	 * We already have the bytes available in the allocation; use them.
> >  	 */
> >  	if (size <= vm->nr_pages << PAGE_SHIFT) {
> > -		/*
> > -		 * No need to zero memory here, as unused memory will have
> > -		 * already been zeroed at initial allocation time or during
> > -		 * realloc shrink time.
> > -		 */
> > -		vm->requested_size = size;
> >  		kasan_vrealloc(p, old_size, size);
> > +
> > +		if (want_init_on_alloc(flags))
> > +			memset((void *)p + old_size, 0, size - old_size);
> > +
> > +		vm->requested_size = size;
> >  		return (void *)p;
> >  	}
> 
> OK, thanks, I'll assume you'll prepare this for real when convenient.
>
OK. I will prepare something and send out the patch after testing.

--
Uladzislau Rezki
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.