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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240208032908.GB185687@cmpxchg.org>
Date: Thu, 8 Feb 2024 04:29:08 +0100
From: Johannes Weiner <hannes@...xchg.org>
To: Zhongkun He <hezhongkun.hzk@...edance.com>
Cc: Matthew Wilcox <willy@...radead.org>, akpm@...ux-foundation.org,
	linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	Vitaly Wool <vitaly.wool@...sulko.com>
Subject: Re: [External] Re: [PATCH] mm/z3fold: remove unneeded spinlock

On Mon, Feb 05, 2024 at 09:08:05AM +0800, Zhongkun He wrote:
> On Mon, Feb 5, 2024 at 2:46 AM Matthew Wilcox <willy@...radead.org> wrote:
> >
> > On Sun, Feb 04, 2024 at 08:54:04PM +0800, Zhongkun He wrote:
> > > There is no need to use spinlock in this section, so
> > > remove it.
> >
> > I don't know this code at all, but the idiom is (relatively) common.
> > It waits until anybody _currently_ holding the lock has released it.
> >
> > That would, eg, make it safe to free the 'pool' memory.
> >
> > > -     spin_lock(&pool->lock);
> > > -     spin_unlock(&pool->lock);
> >
> 
> no,  please see the commit 'e774a7bc7f0adb'.
> 
>         spin_lock(&pool->lock);
> -       if (!list_empty(&page->lru))
> -               list_del_init(&page->lru);
>         spin_unlock(&pool->lock);
> 
> The original purpose of this lock was to protect  page->lru,
> which was removed now, so the spinlock is unnecessary.

But pool->lock protects other stuff too? This doesn't rule out that
there is some other ordering dependency on cycling the lock before
freeing the entry. The person who would know best is the maintainer of
this code, Vitaly. Let's CC him.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ