From: Johannes Weiner <hannes@cmpxchg.org>
To: "Huang, Ying" <ying.huang@intel.com>
Cc: David Rientjes <rientjes@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
David Hildenbrand <david@redhat.com>,
Michal Hocko <mhocko@kernel.org>,
Dave Hansen <dave.hansen@linux.intel.com>,
Mel Gorman <mgorman@suse.de>, Vlastimil Babka <vbabka@suse.cz>,
Minchan Kim <minchan@kernel.org>, Hugh Dickins <hughd@google.com>,
Rik van Riel <riel@surriel.com>,
Matthew Wilcox <willy@infradead.org>
Subject: Re: [PATCH] mm: Add more comments for MADV_FREE
Date: Wed, 11 Mar 2020 10:47:51 -0400 [thread overview]
Message-ID: <20200311144751.GA29835@cmpxchg.org> (raw)
In-Reply-To: <87imjbv51t.fsf@yhuang-dev.intel.com>
On Wed, Mar 11, 2020 at 01:22:54PM +0800, Huang, Ying wrote:
> David Rientjes <rientjes@google.com> writes:
>
> > On Wed, 11 Mar 2020, Huang, Ying wrote:
> >
> >> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> >> index 6f2fef7b0784..01144dd02a5f 100644
> >> --- a/include/linux/mm_inline.h
> >> +++ b/include/linux/mm_inline.h
> >> @@ -9,10 +9,11 @@
> >> * page_is_file_cache - should the page be on a file LRU or anon LRU?
> >> * @page: the page to test
> >> *
> >> - * Returns 1 if @page is page cache page backed by a regular filesystem,
> >> - * or 0 if @page is anonymous, tmpfs or otherwise ram or swap backed.
> >> - * Used by functions that manipulate the LRU lists, to sort a page
> >> - * onto the right LRU list.
> >> + * Returns 1 if @page is page cache page backed by a regular filesystem or
> >> + * anonymous page lazily freed (e.g. via MADV_FREE). Returns 0 if @page is
> >> + * normal anonymous page, tmpfs or otherwise ram or swap backed. Used by
> >> + * functions that manipulate the LRU lists, to sort a page onto the right LRU
> >> + * list.
> >
> > The function name is misleading: anonymous pages that can be lazily freed
> > are not file cache. This returns 1 because of the question it is asking:
> > anonymous lazily freeable pages should be on the file lru, not the anon
> > lru. So before adjusting the comment I'd suggest renaming the function to
> > something like page_is_file_lru().
>
> Yes. I think page_is_file_lru() is a better name too. And whether
> tmpfs pages are file cache pages is confusing too. But I think we can
> do that after this patch if others think this is a good idea too.
I also think the rename is a great idea.
Personally, I'd prefer it in the same patch. Right now the name and
the documentation are out of date, but at least they're consistent in
their view of the world. Fixing this interface - name and
documentation - to reflect the existence of MADV_FREE anon pages is
one logical change, not two.
prev parent reply other threads:[~2020-03-11 14:47 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-11 1:11 [PATCH] mm: Add more comments for MADV_FREE Huang, Ying
2020-03-11 5:08 ` David Rientjes
2020-03-11 5:22 ` Huang, Ying
2020-03-11 14:47 ` Johannes Weiner [this message]
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=20200311144751.GA29835@cmpxchg.org \
--to=hannes@cmpxchg.org \
--cc=akpm@linux-foundation.org \
--cc=dave.hansen@linux.intel.com \
--cc=david@redhat.com \
--cc=hughd@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@suse.de \
--cc=mhocko@kernel.org \
--cc=minchan@kernel.org \
--cc=riel@surriel.com \
--cc=rientjes@google.com \
--cc=vbabka@suse.cz \
--cc=willy@infradead.org \
--cc=ying.huang@intel.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;
as well as URLs for NNTP newsgroup(s).