From: "Huang\, Ying" <ying.huang@intel.com>
To: David Rientjes <rientjes@google.com>
Cc: David Hildenbrand <david@redhat.com>,
Andrew Morton <akpm@linux-foundation.org>, <linux-mm@kvack.org>,
<linux-kernel@vger.kernel.org>,
Dave Hansen <dave.hansen@linux.intel.com>,
Mel Gorman <mgorman@suse.de>, Vlastimil Babka <vbabka@suse.cz>,
Zi Yan <ziy@nvidia.com>, Michal Hocko <mhocko@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Minchan Kim <minchan@kernel.org>,
Johannes Weiner <hannes@cmpxchg.org>,
Hugh Dickins <hughd@google.com>
Subject: Re: [PATCH -V2] mm: Add PageLayzyFree() helper functions for MADV_FREE
Date: Mon, 09 Mar 2020 10:15:22 +0800 [thread overview]
Message-ID: <87eeu2z32d.fsf@yhuang-dev.intel.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2003061240480.181741@chino.kir.corp.google.com> (David Rientjes's message of "Fri, 6 Mar 2020 12:41:32 -0800")
David Rientjes <rientjes@google.com> writes:
> On Thu, 5 Mar 2020, Huang, Ying wrote:
>
>> > In general, I don't think this patch really improves the situation ...
>> > it's only a handful of places where this change slightly makes the code
>> > easier to understand. And there, only slightly ... I'd prefer better
>> > comments instead (e.g., in PageAnon()), documenting what it means for a
>> > anon page to either have PageSwapBacked() set or not.
>>
>> Personally, I still prefer the better named functions than the comments
>> here and there. But I can understand that people may have different
>> flavor.
>>
>
> Maybe add a comment to page-flags.h referring to what PageSwapBacked
> indicates when PageAnon is true?
If someone find a confusing PageSwapBacked() invocation, and if we only
want to use comments to resolve the confusing, the best place to add the
comments is above the line where PageSwapBacked() is invoked. Because
it's harder for people to dig out the right comments in page-flags.h.
The appropriate named helper functions can replace that comments and be
more elegant.
Best Regards,
Huang, Ying
prev parent reply other threads:[~2020-03-09 2:15 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-04 8:17 Huang, Ying
2020-03-04 8:28 ` David Hildenbrand
2020-03-04 20:45 ` David Rientjes
2020-03-05 4:41 ` Huang, Ying
2020-03-06 20:41 ` David Rientjes
2020-03-09 2:15 ` Huang, Ying [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=87eeu2z32d.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=peterz@infradead.org \
--cc=rientjes@google.com \
--cc=vbabka@suse.cz \
--cc=ziy@nvidia.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