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

[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?


> 
> --
> Uladzislau Rezki