Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 4 Mar 2022 13:17:21 -0800 (PST)
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()
In-Reply-To: <90aafe84-fe7d-c70e-8e15-c222869f30fc@redhat.com>
Message-ID: <6e5262ff-8596-a86-7388-eddb2b2c53c@google.com>
References: <351df0af-78f2-c20-2a6d-e5f978e5ca1@google.com> <a2f6aa01-8003-6791-718d-3898f77ef643@redhat.com> <9dda55d5-eafa-3177-2a4c-32ccb7e146e3@google.com> <90aafe84-fe7d-c70e-8e15-c222869f30fc@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

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