Re: [PATCH v2 1/2] Revert "mm: zswap: fix race between [de]compression and CPU hotunplug"

[Yosry Ahmed]

On Tue, Jan 7, 2025 at 3:01 PM Barry Song <baohua@xxxxxxxxxx> wrote:
>
> On Wed, Jan 8, 2025 at 11:22 AM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote:
> >
> > This reverts commit eaebeb93922ca6ab0dd92027b73d0112701706ef.
> >
> > Commit eaebeb93922c ("mm: zswap: fix race between [de]compression and
> > CPU hotunplug") used the CPU hotplug lock in zswap compress/decompress
> > operations to protect against a race with CPU hotunplug making some
> > per-CPU resources go away.
> >
> > However, zswap compress/decompress can be reached through reclaim while
> > the lock is held, resulting in a potential deadlock as reported by
> > syzbot:
> > ======================================================
> > WARNING: possible circular locking dependency detected
> > 6.13.0-rc6-syzkaller-00006-g5428dc1906dd #0 Not tainted
> > ------------------------------------------------------
> > kswapd0/89 is trying to acquire lock:
> >  ffffffff8e7d2ed0 (cpu_hotplug_lock){++++}-{0:0}, at: acomp_ctx_get_cpu mm/zswap.c:886 [inline]
> >  ffffffff8e7d2ed0 (cpu_hotplug_lock){++++}-{0:0}, at: zswap_compress mm/zswap.c:908 [inline]
> >  ffffffff8e7d2ed0 (cpu_hotplug_lock){++++}-{0:0}, at: zswap_store_page mm/zswap.c:1439 [inline]
> >  ffffffff8e7d2ed0 (cpu_hotplug_lock){++++}-{0:0}, at: zswap_store+0xa74/0x1ba0 mm/zswap.c:1546
> >
> > but task is already holding lock:
> >  ffffffff8ea355a0 (fs_reclaim){+.+.}-{0:0}, at: balance_pgdat mm/vmscan.c:6871 [inline]
> >  ffffffff8ea355a0 (fs_reclaim){+.+.}-{0:0}, at: kswapd+0xb58/0x2f30 mm/vmscan.c:7253
> >
> > which lock already depends on the new lock.
>
> We have functions like percpu_is_write_locked(),
> percpu_is_read_locked(), and cpus_read_trylock().
> Could they help prevent circular locking dependencies if we perform a
> check before acquiring the lock?

Yeah we can do that but it feels a bit hacky, we may have to
unnecessarily fail the operation in some cases, right? Not sure tbh.