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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <CAJD7tkY=ztvdozH-yC55YFM4NvCUa77bH42ZYu_ewJYCff0TKw@mail.gmail.com>
Date: Tue, 7 Jan 2025 16:54:02 -0800
From: Yosry Ahmed <yosryahmed@...gle.com>
To: Barry Song <baohua@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Johannes Weiner <hannes@...xchg.org>, 
	Nhat Pham <nphamcs@...il.com>, Chengming Zhou <chengming.zhou@...ux.dev>, 
	Vitaly Wool <vitalywool@...il.com>, Sam Sun <samsun1006219@...il.com>, 
	Kanchana P Sridhar <kanchana.p.sridhar@...el.com>, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org, syzbot <syzkaller@...glegroups.com>
Subject: Re: [PATCH v2 1/2] Revert "mm: zswap: fix race between
 [de]compression and CPU hotunplug"

On Tue, Jan 7, 2025 at 4:34 PM Barry Song <baohua@...nel.org> wrote:
>
> On Wed, Jan 8, 2025 at 12:39 PM Yosry Ahmed <yosryahmed@...gle.com> wrote:
> >
> > On Tue, Jan 7, 2025 at 3:01 PM Barry Song <baohua@...nel.org> wrote:
> > >
> > > On Wed, Jan 8, 2025 at 11:22 AM Yosry Ahmed <yosryahmed@...gle.com> 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.
>
> Not sure if it can be as simple as the following:
>
>     locked = cpus_read_trylock();
>     ....
>     if (locked)
>         cpus_read_unlock();
>
> If this works, it seems better than migrate_disable(), which could affect
> the scheduler's select_rq especially given that swap is a hot path :-)

I didn't look too closely into this, but I'd prefer the simpler fix
unless it causes any noticeable regressions. Unless others are also
concerned about disabling migration..

>
> Thanks
> Barry