From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id C88AE3246E8 for ; Wed, 2 Sep 2026 04:31:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788323480; cv=none; b=TbD12yfp6qR9Cifj8nKKsbu+PJo7XHjdUqCCy0B0wJrc5Ok4A/0ep27CLcI+m2TL1gJWmvxBu+fx/I+2kJAJG4PzSZBSHoLU/sC8ViwiK3ZlJGr1Qlh7drQzLDymzEVsayHWk0597hyO1Kg9uG4eMMZZO6xuTegmATH7Owisr0E= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788323480; c=relaxed/simple; bh=DvhAqyMZEAE6hMWjH1OG4Gf63XX1X25bMKQ9gvEIbXA=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=Bw3J/hEJY1y8LD+sVNl1pSkoMwYkLzoOs9BPVt3QOS9sDtOs/gKT/akOlwuecB1j7ziyOKv1NaFtrfvRni1hXmdkpiAIy4Ryuvg6mNiGllFiLPL5tpHMAjerfhiJU83EkDoKrxYc++hDkyv96RvttGUZ8APSQ1mYFL6bQ13gMPE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b=JhdQnC9v; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b="JhdQnC9v" Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D14E61596; Tue, 1 Sep 2026 21:31:11 -0700 (PDT) Received: from [10.164.148.40] (MacBook-Pro-3.blr.arm.com [10.164.148.40]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C2AE93F7D8; Tue, 1 Sep 2026 21:31:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1788323475; bh=DvhAqyMZEAE6hMWjH1OG4Gf63XX1X25bMKQ9gvEIbXA=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=JhdQnC9vUNH9SMsR7ARDISMkwDkTMKASJonQiTuboHjwMjWFrjD9h51kDllYk9Q18 fwhcnfBUdXA/VzLfMIOvUSspIIvdBP6zEyS6LhS+lyPtuTq5rxV7uYRRo7basLSanX DOKZCoROL7ed+h9Zi2yItAAwYyffh1MvRaxl+1ys= Message-ID: Date: Wed, 2 Sep 2026 10:00:57 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] mm: vmalloc: fix vmap_purge_lock livelock under memory pressure To: Uladzislau Rezki , Andrew Morton Cc: Ye Liu , Ye Liu , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20260828091753.299295-1-ye.liu@linux.dev> <20260828110503.e1eff32a9b7df9a8b2ddd4d6@linux-foundation.org> <54bd749f-04f5-4665-bd5f-d485e7197132@arm.com> <2e9488bb-feb6-4994-92e3-e5ef0c1ff5ec@linux.dev> Content-Language: en-US From: Dev Jain In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 01/09/26 10:29 pm, Uladzislau Rezki wrote: > On Tue, Sep 01, 2026 at 06:43:43PM +0200, Uladzislau Rezki wrote: >> 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 wrote: >>>>>>> >>>>>>>> From: Ye Liu >>>>>>>> >>>>>>>> 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 . >>>> >>>> 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. >> > And i sent out already the WQ_UNBOUND | WQ_MEM_RECLAIM and separate WQ > for vmap drain logic. It looks like Andrew/me forgot about it: > > https://lore.kernel.org/all/20260331202352.879718-1-urezki@gmail.com/ Great! Perhaps resend it and we can review it? > > -- > Uladzislau Rezki