mirror of https://lore.kernel.org/lkml/
 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 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®