Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Uladzislau Rezki <urezki@gmail.com>
To: Dev Jain <dev.jain@arm.com>
Cc: Uladzislau Rezki <urezki@gmail.com>, Ye Liu <ye.liu@linux.dev>,
	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 18:40:08 +0200	[thread overview]
Message-ID: <apb_6DUfmilJGsp1@milan> (raw)
In-Reply-To: <54bd749f-04f5-4665-bd5f-d485e7197132@arm.com>

On Tue, Sep 01, 2026 at 11:52:11AM +0530, Dev Jain wrote:
> 
> 
> 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?
> 
It is possible what you describe. Usually when no memory, the work item
can be delayed for a while. As it was noted WQ_RECLAIM and separate WQ
which has a rescue worker.

IMO, it should be a separate patch.

--
Uladzislau Rezki


      parent reply	other threads:[~2026-09-01 16:40 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
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 [this message]

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=apb_6DUfmilJGsp1@milan \
    --to=urezki@gmail.com \
    --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=ye.liu@linux.dev \
    /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