All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ye Liu <ye.liu@linux.dev>
To: Dev Jain <dev.jain@arm.com>, Uladzislau Rezki <urezki@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Ye Liu <liuye@kylinos.cn>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] mm: vmalloc: fix vmap_purge_lock livelock under memory pressure
Date: Tue, 1 Sep 2026 17:07:30 +0800	[thread overview]
Message-ID: <2e9488bb-feb6-4994-92e3-e5ef0c1ff5ec@linux.dev> (raw)
In-Reply-To: <54bd749f-04f5-4665-bd5f-d485e7197132@arm.com>



在 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@linux.dev> wrote:
>>>>
>>>>> From: Ye Liu <liuye@kylinos.cn>
>>>>>
>>>>> 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@linux.dev
>>>
>>>
>>> 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@linux.dev>.
> 
> 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;


>>
>> --
>> Uladzislau Rezki
> 

-- 
Thanks,
Ye Liu



  reply	other threads:[~2026-09-01  9:07 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-28  9:17 [PATCH v2] mm: vmalloc: fix vmap_purge_lock livelock under memory pressure Ye Liu
2026-08-28 16:54 ` Uladzislau Rezki
2026-08-28 18:05 ` Andrew Morton
2026-08-31  6:09   ` Dev Jain
2026-08-31  9:54     ` Uladzislau Rezki
2026-09-01  1:42       ` Andrew Morton
2026-09-01 16:36         ` Uladzislau Rezki
2026-09-01  6:22       ` Dev Jain
2026-09-01  9:07         ` Ye Liu [this message]
2026-09-01 16:43           ` Uladzislau Rezki
2026-09-01 16:59             ` Uladzislau Rezki
2026-09-02  4:30               ` Dev Jain
2026-09-03  9:10                 ` Uladzislau Rezki
2026-09-01 16:40         ` 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=2e9488bb-feb6-4994-92e3-e5ef0c1ff5ec@linux.dev \
    --to=ye.liu@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=dev.jain@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=liuye@kylinos.cn \
    --cc=urezki@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.