From: Mel Gorman <mgorman@techsingularity.net>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Yu Zhao <yuzhao@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
Nicolas Saenz Julienne <nsaenzju@redhat.com>,
Marcelo Tosatti <mtosatti@redhat.com>,
Michal Hocko <mhocko@kernel.org>, Hugh Dickins <hughd@google.com>,
LKML <linux-kernel@vger.kernel.org>,
Linux-MM <linux-mm@kvack.org>
Subject: Re: [PATCH 7/7] mm/page_alloc: Replace local_lock with normal spinlock
Date: Tue, 21 Jun 2022 10:27:28 +0100 [thread overview]
Message-ID: <20220621092728.GE15453@techsingularity.net> (raw)
In-Reply-To: <8354ba5b-d1ce-90c7-be76-328ab9321550@suse.cz>
On Fri, Jun 17, 2022 at 09:57:06AM +0200, Vlastimil Babka wrote:
> On 6/16/22 23:07, Yu Zhao wrote:
> > On Thu, Jun 16, 2022 at 11:02 AM Vlastimil Babka <vbabka@suse.cz> wrote:
> >>
> >>
> >> > @@ -3794,19 +3805,29 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone,
> >> > struct list_head *list;
> >> > struct page *page;
> >> > unsigned long flags;
> >> > + unsigned long __maybe_unused UP_flags;
> >> >
> >> > - local_lock_irqsave(&pagesets.lock, flags);
> >> > + /*
> >> > + * spin_trylock_irqsave is not necessary right now as it'll only be
> >> > + * true when contending with a remote drain. It's in place as a
> >> > + * preparation step before converting pcp locking to spin_trylock
> >> > + * to protect against IRQ reentry.
> >> > + */
> >> > + pcp_trylock_prepare(UP_flags);
> >> > + pcp = pcp_spin_trylock_irqsave(zone->per_cpu_pageset, flags);
> >> > + if (!pcp)
> >>
> >> Besides the missing unpin Andrew fixed, I think also this is missing
> >> pcp_trylock_finish(UP_flags); ?
> >
> > spin_trylock only fails when trylock_finish is a NOP.
>
> True, so it's not an active bug, but I would still add it, so it's not
> confusing and depending on non-obvious details that might later change and
> break the code.
Yes. Even though it may work, it's still wrong.
--
Mel Gorman
SUSE Labs
next prev parent reply other threads:[~2022-06-21 9:27 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-13 12:56 [PATCH v4 00/7] Drain remote per-cpu directly Mel Gorman
2022-06-13 12:56 ` [PATCH 1/7] mm/page_alloc: Add page->buddy_list and page->pcp_list Mel Gorman
2022-06-13 12:56 ` [PATCH 2/7] mm/page_alloc: Use only one PCP list for THP-sized allocations Mel Gorman
2022-06-13 12:56 ` [PATCH 3/7] mm/page_alloc: Split out buddy removal code from rmqueue into separate helper Mel Gorman
2022-06-13 12:56 ` [PATCH 4/7] mm/page_alloc: Remove mistaken page == NULL check in rmqueue Mel Gorman
2022-06-13 12:56 ` [PATCH 5/7] mm/page_alloc: Protect PCP lists with a spinlock Mel Gorman
2022-06-16 15:59 ` Vlastimil Babka
2022-06-13 12:56 ` [PATCH 6/7] mm/page_alloc: Remotely drain per-cpu lists Mel Gorman
2022-06-16 16:41 ` Vlastimil Babka
2022-06-13 12:56 ` [PATCH 7/7] mm/page_alloc: Replace local_lock with normal spinlock Mel Gorman
2022-06-15 22:43 ` Yu Zhao
[not found] ` <CGME20220615224855eucas1p1ea6d90c23ec9423dfe04b267f6dddd2a@eucas1p1.samsung.com>
2022-06-15 22:48 ` Marek Szyprowski
2022-06-15 23:04 ` Andrew Morton
2022-06-16 3:05 ` Yu Zhao
2022-06-17 7:55 ` Vlastimil Babka
2022-06-17 6:47 ` Marek Szyprowski
2022-06-21 9:21 ` Mel Gorman
2022-06-16 17:01 ` Vlastimil Babka
2022-06-16 21:07 ` Yu Zhao
2022-06-17 7:57 ` Vlastimil Babka
2022-06-21 9:27 ` Mel Gorman [this message]
2022-06-21 9:26 ` Mel Gorman
2022-06-17 9:39 ` Nicolas Saenz Julienne
2022-06-21 9:29 ` Mel Gorman
2022-06-21 9:31 ` Nicolas Saenz Julienne
2022-07-03 9:44 ` [mm/page_alloc] 2bd8eec68f: BUG:sleeping_function_called_from_invalid_context_at_mm/gup.c kernel test robot
2022-07-03 9:44 ` kernel test robot
2022-07-03 20:22 ` Andrew Morton
2022-07-03 20:22 ` Andrew Morton
2022-07-05 13:51 ` Oliver Sang
2022-07-05 13:51 ` Oliver Sang
2022-07-06 9:55 ` Mel Gorman
2022-07-06 9:55 ` Mel Gorman
2022-07-06 11:53 ` Mel Gorman
2022-07-06 11:53 ` Mel Gorman
2022-07-06 14:21 ` Oliver Sang
2022-07-06 14:21 ` Oliver Sang
2022-07-06 14:52 ` Mel Gorman
2022-07-06 14:52 ` Mel Gorman
2022-07-07 8:22 ` Oliver Sang
2022-07-07 8:22 ` Oliver Sang
2022-07-06 14:25 ` Oliver Sang
2022-07-06 14:25 ` Oliver Sang
2022-07-06 14:53 ` Mel Gorman
2022-07-06 14:53 ` Mel Gorman
2022-07-07 21:55 ` Vlastimil Babka
2022-07-07 21:55 ` Vlastimil Babka
2022-07-08 10:56 ` Mel Gorman
2022-07-08 10:56 ` Mel Gorman
2022-07-12 5:04 ` Oliver Sang
2022-07-12 5:04 ` Oliver Sang
2022-06-24 12:54 [PATCH v5 00/7] Drain remote per-cpu directly Mel Gorman
2022-06-24 12:54 ` [PATCH 7/7] mm/page_alloc: Replace local_lock with normal spinlock Mel Gorman
2022-06-24 18:59 ` Yu Zhao
2022-07-04 14:39 ` Vlastimil Babka
2022-07-04 16:33 ` Nicolas Saenz Julienne
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=20220621092728.GE15453@techsingularity.net \
--to=mgorman@techsingularity.net \
--cc=akpm@linux-foundation.org \
--cc=hughd@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=mtosatti@redhat.com \
--cc=nsaenzju@redhat.com \
--cc=vbabka@suse.cz \
--cc=yuzhao@google.com \
/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.