linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Chris Down <chris@chrisdown.name>
To: Michal Hocko <mhocko@suse.com>
Cc: Yafang Shao <laoar.shao@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux MM <linux-mm@kvack.org>,
	shaoyafang@didiglobal.com
Subject: Re: [PATCH] mm/memcontrol: avoid unnecessary PageTransHuge() when counting compound page
Date: Tue, 7 May 2019 15:21:48 +0100	[thread overview]
Message-ID: <20190507142148.GA55122@chrisdown.name> (raw)
In-Reply-To: <20190506191956.GF31017@dhcp22.suse.cz>

Michal Hocko writes:
>On Mon 06-05-19 23:22:11, Yafang Shao wrote:
>> It is a better code, I think.
>> Regarding the performance, I don't think it is easy to measure.
>
>I am not convinced the patch is worth it. The code aesthetic is a matter
>of taste. On the other hand, the change will be an additional step in
>the git history so git blame take an additional step to get to the
>original commit which is a bit annoying. Also every change, even a
>trivially looking one, can cause surprising side effects. These are all
>arguments make a change to the code.
>
>So unless the resulting code is really much more cleaner, easier to read
>or maintain, or it is a part of a larger series that makes further steps
>easier,then I would prefer not touching the code.

Aside from what Michal already said, which I agree with, when skimming code 
reading PageTransHuge has much clearer intent to me than checking nr_pages. We 
already have a non-trivial number of checks which are unclear at first glance 
in the mm code and, while this isn't nearly as bad as some of those, and might 
not make the situation much worse, I also don't think changing to nr_pages 
checks makes the situation any better, either.


  reply	other threads:[~2019-05-07 14:21 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-05  6:40 [PATCH] mm/memcontrol: avoid unnecessary PageTransHuge() when counting compound page Yafang Shao
2019-05-06 13:59 ` Michal Hocko
2019-05-06 15:22   ` Yafang Shao
2019-05-06 19:19     ` Michal Hocko
2019-05-07 14:21       ` Chris Down [this message]
2019-05-07 15:00         ` Yafang Shao

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=20190507142148.GA55122@chrisdown.name \
    --to=chris@chrisdown.name \
    --cc=akpm@linux-foundation.org \
    --cc=laoar.shao@gmail.com \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=shaoyafang@didiglobal.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).