Re: [PATCH v13 0/5] mm/vmalloc: free unused pages on vrealloc() shrink

[Uladzislau Rezki <urezki@gmail.com>]

On Mon, May 18, 2026 at 04:29:11AM +0530, Shivam Kalra wrote:
> On 13/05/26 03:31, Andrew Morton wrote:
> > On Mon, 11 May 2026 13:47:28 +0530 Shivam Kalra via B4 Relay <devnull+shivamkalra98.zohomail.in@kernel.org> wrote:
> > 
> >> This series implements the TODO in vrealloc() to unmap and free unused
> >> pages when shrinking across a page boundary.
> > 
> > Thanks.  AI review asked a few questions.  Does anything here look
> > legitimate?
> > 
> > 	https://sashiko.dev/#/patchset/20260511-vmalloc-shrink-v13-0-643b7ec277a9@zohomail.in
> 
> Thanks for flagging this. I went through the Sashiko review, and,
> one of the findings is legitimate and needs a fix.
> 
> 1) Patch 3 - vmap() mappings with nr_pages == 0 (LEGITIMATE)
> This is correct. For vmap() areas created without VM_MAP_PUT_PAGES,
> nr_pages is never set and stays 0 (vm_struct is kzalloc'd). With the
> current patch, vread_iter() would compute size = 0 for these areas,
> skip them entirely, and zero-fill the region in /proc/kcore.
> 
> Ulad, this is the approach you suggested during v10 review [1]; we
> both checked whether nr_pages could be 0 and concluded it couldn't,
> but we only considered the VM_ALLOC (vmalloc) path. vmap() areas also
> have vm != NULL but never set nr_pages unless VM_MAP_PUT_PAGES is
> passed, which most callers don't use.
> 
> Since vrealloc() only operates on VM_ALLOC areas, I think we can
> scope the nr_pages path to just that:
> 	if (vm)
> 		/*
> 		 * For VM_ALLOC areas, use nr_pages rather than
> 		 * get_vm_area_size() because vrealloc() may shrink
> 		 * the mapping without updating area->size. Other
> 		 * mapping types (vmap, ioremap) don't set nr_pages.
> 		 */
> 		size = (vm->flags & VM_ALLOC) ?
> 			(vm->nr_pages << PAGE_SHIFT) :
> 			get_vm_area_size(vm);
> 	else
> 		size = va_size(va);
> Ulad, does this approach look right to you, or would you prefer to
> initialize nr_pages unconditionally in vmap() instead?
> 
> [1] https://lore.kernel.org/lkml/aeYBYqPSEJRC8mjh@milan/
>
Indeed, we missed that point regarding vmap() code. Checking VM_ALLOC
sounds correct to me.

--
Uladzislau Rezki