Re: [PATCH v2] mm: vmalloc: fix vmap_purge_lock livelock under memory pressure

[Uladzislau Rezki]

On Tue, Sep 01, 2026 at 05:07:30PM +0800, Ye Liu wrote:
> 
> 
> 在 2026/9/1 14:22, Dev Jain 写道:
> > 
> > 
> > On 31/08/26 3:24 pm, Uladzislau Rezki wrote:
> >> On Mon, Aug 31, 2026 at 11:39:14AM +0530, Dev Jain wrote:
> >>>
> >>>
> >>> On 28/08/26 11:35 pm, Andrew Morton wrote:
> >>>> On Fri, 28 Aug 2026 17:17:53 +0800 Ye Liu <ye.liu@xxxxxxxxx> wrote:
> >>>>
> >>>>> From: Ye Liu <liuye@xxxxxxxxxx>
> >>>>>
> >>>>> The vmap_purge_lock mutex can be held for an extended period by
> >>>>> __purge_vmap_area_lazy() which calls flush_work() to wait for
> >>>>> purge_vmap_node workers while holding the lock.  Under memory
> >>>>> pressure, those workers may themselves be blocked in direct
> >>>>> reclaim trying to acquire the same lock via the
> >>>>> vmap_node_shrink_scan() shrinker callback, creating a circular
> >>>>> dependency that deadlocks the entire system.
> >>>>>
> >>>>> Two places acquire vmap_purge_lock from paths that can be reached
> >>>>> during direct reclaim:
> >>>>>
> >>>>> 1. vmap_node_shrink_scan(): replace blocking guard(mutex) with
> >>>>>    mutex_trylock().  This is a shrinker that only decays the vmap
> >>>>>    pool and returns SHRINK_STOP without freeing memory; skipping a
> >>>>>    decay cycle when the lock is contended is harmless and prevents
> >>>>>    tasks from piling up on the mutex in the direct reclaim path.
> >>>>>
> >>>>> 2. reclaim_and_purge_vmap_areas(): replace mutex_lock() with
> >>>>>    mutex_trylock().  This is called from the vmalloc allocation
> >>>>>    overflow path; if trylock fails, another thread is already
> >>>>>    purging and the allocator's retry will find freed space.  The
> >>>>>    notifier chain provides a fallback if the retry still fails.
> >>>>>
> >>>>> Both trylock failures break the circular dependency: the lock
> >>>>> holder's flush_work() can complete because workers are no longer
> >>>>> blocked on vmap_purge_lock in the direct reclaim path.
> >>>>
> >>>> Thanks.  AI review expressed a couple of concerns:
> >>>> 	https://sashiko.dev/#/patchset/20260828091753.299295-1-ye.liu@xxxxxxxxx
> >>>
> >>>
> >>> Sounds legit to me. Now there is no guarantee of purge being successful, and we
> >>> can get a spurious failure.
> >>>
> >>> How about using a WQ_RECLAIM workqueue:
> >>>
> >>> vmap_purge_wq = alloc_workqueue("vmap_purge",
> >>> 				WQ_MEM_RECLAIM | WQ_PERCPU, 0);
> >>>
> >>> I see the same pattern in __lru_add_drain_all() and kmem_cache_init_late().
> >>>
> >> WQ_MEM_RECLAIM makes sense but this is another patch.
> >>
> >> I copied here AI comment:
> >>>
> >>> Does replacing this blocking lock with a trylock break synchronization for
> >>> the callers?
> >>>
> >> No it does not. If someone is doing reclaim we do not wait and do not try
> >> to do it again thus fail allocation.
> >>
> >>> When vmalloc space is exhausted, alloc_vmap_area() calls
> >>> reclaim_and_purge_vmap_areas() and relies on its blocking behavior to ensure
> >>> that free space has actually been reclaimed before looping back to retry:
> >>> mm/vmalloc.c:alloc_vmap_area() {
> >>>    ...
> >>> overflow:
> >>> 	if (!purged) {
> >>> 		reclaim_and_purge_vmap_areas();
> >>> 		purged = 1;
> >>> 		goto retry;
> >>> 	}
> >>>    ...
> >>> }
> >>> With this patch, if another thread holds vmap_purge_lock, mutex_trylock()
> >>> fails and the function returns immediately.
> >>>
> >> If reclaim is in progress and trylock fails a caller repeats only one
> >> time to retry an allocation. There is no any infinite loop.
> >>
> >>>
> >>> The allocator then retries
> >>> instantly without waiting for the concurrent purge to complete.
> >>> Because the retry fails and purged is already 1, could this cause the
> >>> allocation to abort and return a spurious vmalloc allocation failure
> >>> (-EBUSY or -ENOMEM)?
> >>>
> >> vmap space can be fragmented and not avail for 32-bit systems. For
> >> 64-bit system it is likely impossible.
> >>
> >> But, i think we can overt mutex_lock() into mutex_trylock() just only
> >> in the:
> >>
> >> static unsigned long
> >> vmap_node_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
> >> {
> >> 	struct vmap_node *vn;
> >>
> >> 	guard(mutex)(&vmap_purge_lock);
> >> 	for_each_vmap_node(vn)
> >> 		decay_va_pool_node(vn, true);
> >>
> >> 	return SHRINK_STOP;
> >> }
> >>
> >> so the reclaim path is not blocked. It should also address an issue
> >> reported by the Ye Liu <ye.liu@xxxxxxxxx>.
> > 
> > IIUC you are suggesting mutex_trylock() only in the shrinker path. But
> > then, the following is possible no: take purge lock, try to get a
> > worker thread, worker thread is stuck in vmalloc -> alloc_vmap_area
> > -> reclaim_and_purge_vmap_areas -> take purge lock?
> > 
> > 
> 
> Personally, I lean toward the WQ_MEM_RECLAIM workqueue solution. 
> After taking a closer look at the code, I noticed a subtle but 
> potentially problematic scenario:
> 
> When drain_vmap_area_work acquires vmap_purge_lock and calls into 
> __purge_vmap_area_lazy, it may subsequently invoke queue_work/queue_work_on 
> on the same CPU's system_wq. If the newly queued work ends up waiting 
> for an available worker on that same CPU, while the current worker is
> blocked waiting for that very work to complete (via flush_work), 
> we could end up with a self-deadlock on a single CPU.
> 
> Theoretically, this seems possible. I suspect the reason we don't 
> see widespread reports of such deadlocks is that the nr_purge_helpers 
> logic limits the number of asynchronous workers; when resources are tight, 
> it falls back to synchronous execution (purge_vmap_node directly), 
> which avoids queuing additional work. 
> 
> Using a dedicated workqueue with WQ_MEM_RECLAIM would provide a clean, 
> explicit isolation—ensuring forward progress under memory pressure and 
> eliminating the risk of interfering with other subsystems' workqueues. 
> I believe this approach is more robust in the long run.
> 
> Perhaps like the code below：
> the dedicated queue eliminates the self‑deadlock risk, and the trylock 
> in the shrinker prevents recursive lock attempts from reclaim contexts.
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index bea9f76ed7e7..68fc1f5acb2f 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2218,6 +2218,9 @@ static unsigned long lazy_max_pages(void)
>   */
>  static DEFINE_MUTEX(vmap_purge_lock);
>  
> +/* Workqueue for lazy vmap purging; WQ_MEM_RECLAIM guarantees progress. */
> +static struct workqueue_struct *vmap_purge_wq;
> +
>  /* for per-CPU blocks */
>  static void purge_fragmented_blocks_allcpus(void);
>  
> @@ -2408,9 +2411,9 @@ static bool __purge_vmap_area_lazy(unsigned long start, unsigned long end,
>                                 INIT_WORK(&vn->purge_work, purge_vmap_node);
>  
>                                 if (cpumask_test_cpu(i, cpu_online_mask))
> -                                       schedule_work_on(i, &vn->purge_work);
> +                                       queue_work_on(i, vmap_purge_wq, &vn->purge_work);
>                                 else
> -                                       schedule_work(&vn->purge_work);
> +                                       queue_work(vmap_purge_wq, &vn->purge_work);
>  
>                                 nr_purge_helpers--;
>                         } else {
> @@ -5519,10 +5522,14 @@ vmap_node_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
>  {
>         struct vmap_node *vn;
>  
> -       guard(mutex)(&vmap_purge_lock);
> +       if (!mutex_trylock(&vmap_purge_lock))
> +               return SHRINK_STOP;
> +
>         for_each_vmap_node(vn)
>                 decay_va_pool_node(vn, true);
>  
> +       mutex_unlock(&vmap_purge_lock);
> +
>         return SHRINK_STOP;
>  }
>  
> @@ -5575,6 +5582,17 @@ void __init vmalloc_init(void)
>          * Now we can initialize a free vmap space.
>          */
>         vmap_init_free_space();
> +
> +       /*
> +        * A dedicated workqueue for lazy vmap purging.  WQ_MEM_RECLAIM
> +        * reserves a rescue worker so queued purge work items are executed
> +        * even under memory pressure, when workers of the system workqueue
> +        * may be stuck in direct reclaim.
> +        */
> +       vmap_purge_wq = alloc_workqueue("vmap_purge",
> +                                       WQ_MEM_RECLAIM | WQ_PERCPU, 0);
> +       WARN_ON(!vmap_purge_wq);
> +
>         vmap_initialized = true;
> 
I agree. We should have it and it should be as separate patch, i.e.
split vmap_node_shrink_scan() and dedicated per-cpu WQs per vmap drain.

--
Uladzislau Rezki