Re: [PATCH -V3] mm: Add PageLayzyFree() helper functions for MADV_FREE

[David Rientjes]

On Mon, 9 Mar 2020, David Hildenbrand wrote:

> >> I still prefer something like
> >>
> >> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> >> index fd6d4670ccc3..7538501230bd 100644
> >> --- a/include/linux/page-flags.h
> >> +++ b/include/linux/page-flags.h
> >> @@ -63,6 +63,10 @@
> >>   * page_waitqueue(page) is a wait queue of all tasks waiting for the page
> >>   * to become unlocked.
> >>   *
> >> + * PG_swapbacked used with anonymous pages (PageAnon()) indicates that a
> >> + * page is backed by swap. Anonymous pages without PG_swapbacked are
> >> + * pages that can be lazily freed (e.g., MADV_FREE) on demand.
> >> + *
> >>   * PG_uptodate tells whether the page's contents is valid.  When a read
> >>   * completes, the page becomes uptodate, unless a disk I/O error happened.
> >>   *
> > 
> > Why not just send a formal patch?  So Andrew can just pick anything he
> > likes.  I am totally OK with that.
> 
> Because you're working on cleaning this up.
> 
> > 
> >> and really don't like the use of !__PageLazyFree() instead of PageSwapBacked().
> > 
> > If adopted, !__PageLazyFree() should only be used in the context where
> > we really want to check whether pages are freed lazily.  Otherwise,
> > PageSwapBacked() should be used.
> > 
> 
> Yeah, and once again, personally, I don't like this approach. E.g.,
> ClearPageLazyFree() sets PG_swapbacked. You already have to be aware
> that this is a single flag being used in the background and what the
> implications are. IMHO, in no way better than the current approach. I
> prefer better documentation instead.
> 

Fully agreed.