From: Uladzislau Rezki <urezki@gmail.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Uladzislau Rezki <urezki@gmail.com>,
Artem Lytkin <iprintercanon@gmail.com>,
linux-mm@kvack.org, willy@infradead.org,
shivamkalra98@zohomail.in, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4] mm/vmalloc: make vm_struct.nr_pages an unsigned long
Date: Thu, 6 Aug 2026 10:51:07 +0200 [thread overview]
Message-ID: <anRK-y0nBUnbli_u@milan> (raw)
In-Reply-To: <20260803173935.132156ba2b86293ffecb0e8f@linux-foundation.org>
On Mon, Aug 03, 2026 at 05:39:35PM -0700, Andrew Morton wrote:
> On Sun, 2 Aug 2026 17:52:26 +0200 Uladzislau Rezki <urezki@gmail.com> 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 <iprintercanon@gmail.com> wrote:
> > >
> >
> > ...
> >
> > > Ulad, AI review suggests that vrealloc() has an issue handling
> > > __GFP_ZERO. Can you please check?
> > >
> > > https://sashiko.dev/#/patchset/20260801114915.115224-1-iprintercanon@gmail.com
> > >
> > 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
next prev parent reply other threads:[~2026-08-06 8:51 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-29 17:57 [PATCH] mm/vmalloc: make vm_struct.nr_pages an unsigned long Artem Lytkin
2026-07-29 18:28 ` Matthew Wilcox
2026-07-29 21:45 ` Andrew Morton
2026-07-30 9:07 ` Artem Lytkin
2026-07-30 9:06 ` [PATCH v2 0/3] mm/vmalloc: stop truncating byte counts derived from nr_pages Artem Lytkin
2026-07-30 17:11 ` [PATCH v3] mm/vmalloc: make vm_struct.nr_pages an unsigned long Artem Lytkin
2026-07-30 20:09 ` [PATCH v2 0/3] mm/vmalloc: stop truncating byte counts derived from nr_pages Andrew Morton
2026-08-01 11:49 ` [PATCH v4] mm/vmalloc: make vm_struct.nr_pages an unsigned long Artem Lytkin
2026-08-01 18:52 ` Andrew Morton
2026-08-02 15:52 ` Uladzislau Rezki
2026-08-04 0:39 ` Andrew Morton
2026-08-06 8:51 ` Uladzislau Rezki [this message]
2026-07-30 9:06 ` [PATCH v2 1/3] mm/vmalloc: fix 32-bit truncation of the area size in vread_iter() Artem Lytkin
2026-07-30 9:06 ` [PATCH v2 2/3] mm/vmalloc: fix 32-bit truncation in the vrealloc() grow-in-place check Artem Lytkin
2026-07-30 9:06 ` [PATCH v2 3/3] mm/vmalloc: make vm_struct.nr_pages an unsigned long Artem Lytkin
2026-07-30 13:04 ` [PATCH] " Uladzislau Rezki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=anRK-y0nBUnbli_u@milan \
--to=urezki@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=iprintercanon@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=shivamkalra98@zohomail.in \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox