From: Johannes Weiner <hannes@cmpxchg.org>
To: Zhongkun He <hezhongkun.hzk@bytedance.com>
Cc: Matthew Wilcox <willy@infradead.org>,
akpm@linux-foundation.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org,
Vitaly Wool <vitaly.wool@konsulko.com>
Subject: Re: [External] Re: [PATCH] mm/z3fold: remove unneeded spinlock
Date: Thu, 8 Feb 2024 04:29:08 +0100 [thread overview]
Message-ID: <20240208032908.GB185687@cmpxchg.org> (raw)
In-Reply-To: <CACSyD1MEM+KnNMimmbdSFXOsok9S=vN=DxzS9x90_6+pt5qjFA@mail.gmail.com>
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@infradead.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.
next prev parent reply other threads:[~2024-02-08 3:29 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-04 12:54 Zhongkun He
2024-02-04 18:46 ` Matthew Wilcox
2024-02-05 1:08 ` [External] " Zhongkun He
2024-02-08 3:29 ` Johannes Weiner [this message]
2024-02-08 17:39 ` Zhongkun He
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=20240208032908.GB185687@cmpxchg.org \
--to=hannes@cmpxchg.org \
--cc=akpm@linux-foundation.org \
--cc=hezhongkun.hzk@bytedance.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=vitaly.wool@konsulko.com \
--cc=willy@infradead.org \
/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