Re: [PATCH v2 3/3] mm: remove superfluous __ClearPageWaiters()

[Yu Zhao <yuzhao@google.com>]

On Thu, Aug 20, 2020 at 08:18:27AM +0200, Michal Hocko wrote:
> On Tue 18-08-20 12:47:04, Yu Zhao wrote:
> > Presumably __ClearPageWaiters() was added to follow the previously
> > removed __ClearPageActive() pattern.
> 
> I do not think so. Please have a look at 62906027091f ("mm: add
> PageWaiters indicating tasks are waiting for a page bit") and a
> discussion when the patch has been proposed. Sorry I do not have a link
> handy but I do remember that the handling was quite subtle.
>  
> > Only flags that are in PAGE_FLAGS_CHECK_AT_FREE needs to be properly
> > cleared because otherwise we think there may be some kind of leak.
> > PG_waiters is not one of those flags and leaving the clearing to
> > PAGE_FLAGS_CHECK_AT_PREP is more appropriate.
> 
> What is the point of this patch in the first place? Page waiters is
> quite subtle and I wouldn't touch it without having a very good reason.

I appreciate your caution. And I just studied the history [1] (I admit
this is something I should have done beforehand), and didn't find any
discussion on __ClearPageWaiters() specifically. So I would ask why it
was added originally. I was hoping Nicholas could help us.

[1] https://lore.kernel.org/lkml/20161225030030.23219-3-npiggin@gmail.com/

Given its triviality, I can't argue how useful this patch is. So I'll
go with how evident it is: we are removing __ClearPageWaiters() from
paths where pages have no references left -- they can't have any
waiters or be on any wait queues.