From: Hugh Dickins <hughd@google.com>
To: David Hildenbrand <david@redhat.com>
Cc: Hugh Dickins <hughd@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
Matthew Wilcox <willy@infradead.org>,
Nicholas Piggin <npiggin@gmail.com>, Yu Zhao <yuzhao@google.com>,
Yang Shi <shy828301@gmail.com>, Michal Hocko <mhocko@suse.com>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH mmotm] mm: delete __ClearPageWaiters()
Date: Fri, 4 Mar 2022 13:17:21 -0800 (PST) [thread overview]
Message-ID: <6e5262ff-8596-a86-7388-eddb2b2c53c@google.com> (raw)
In-Reply-To: <90aafe84-fe7d-c70e-8e15-c222869f30fc@redhat.com>
On Fri, 4 Mar 2022, David Hildenbrand wrote:
>
> In this context we can consider ZONE_DEVICE pages just like any other
> pages that, although getting freed, are not returned to the buddy, but
> instead are returned to another pool. So PAGE_FLAGS_CHECK_AT_PREP won't
> apply and free_pages_prepare() won't apply.
>
> Another example would be hugetlb pages, that are returned to the hugetlb
> pool, but not back to the buddy unless the huge page pool is shrunk.
>
>
> So I feel like the underlying principle here is: we don't *care* if
> PG_waiter is cleared when a page gets freed, because it will simply get
> cleared by the next waker if it sticks around.
I think we were focused on different issues here. I was focused on
how it was redundant for those places to clear the bit, because it
was going to get cleared anyway just after (in the buddy case).
Whereas you are focused on how it doesn't matter at all whether
it gets cleared when freeing. Both valid points.
>
> Then, I agree, we can just drop the comment regarding
> PAGE_FLAGS_CHECK_AT_PREP and instead have something like
Okay, the reference to PAGE_FLAGS_CHECK_AT_PREP in the commit message is
good enough for me, no need to make a point of it in the code comment.
>
>
> "
> That's okay, it's a rare case and the next waker will just clear it.
> Note that, depending on the page pool (buddy, ZONE_DEVICE, hugetlb), we
> might clear the flag while freeing the page, however, this is not
> required for correctness.
> "
Okay, v2 coming up: I've taken largely your wording (but not exactly).
Thanks,
Hugh
next prev parent reply other threads:[~2022-03-04 21:17 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-03 1:56 Hugh Dickins
2022-03-03 2:15 ` Yu Zhao
2022-03-03 8:54 ` David Hildenbrand
2022-03-03 22:28 ` Hugh Dickins
2022-03-03 22:41 ` Matthew Wilcox
2022-03-04 17:25 ` David Hildenbrand
2022-03-04 21:17 ` Hugh Dickins [this message]
2022-03-04 21:25 ` [PATCH mmotm v2] " Hugh Dickins
2022-03-07 8:41 ` David Hildenbrand
2022-03-07 20:17 ` Yang Shi
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=6e5262ff-8596-a86-7388-eddb2b2c53c@google.com \
--to=hughd@google.com \
--cc=akpm@linux-foundation.org \
--cc=david@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=npiggin@gmail.com \
--cc=shy828301@gmail.com \
--cc=willy@infradead.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox