All of 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 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.