All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Huang\, Ying" <ying.huang@intel.com>
To: David Rientjes <rientjes@google.com>
Cc: 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>,
	 Johannes Weiner <hannes@cmpxchg.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 13:22:54 +0800	[thread overview]
Message-ID: <87imjbv51t.fsf@yhuang-dev.intel.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2003102204450.255869@chino.kir.corp.google.com> (David Rientjes's message of "Tue, 10 Mar 2020 22:08:54 -0700")

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.

Best Regards,
Huang, Ying


  reply	other threads:[~2020-03-11  5:23 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 [this message]
2020-03-11 14:47     ` Johannes Weiner

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=87imjbv51t.fsf@yhuang-dev.intel.com \
    --to=ying.huang@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=dave.hansen@linux.intel.com \
    --cc=david@redhat.com \
    --cc=hannes@cmpxchg.org \
    --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 \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.