linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
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


  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