From: Michal Hocko <mhocko@kernel.org>
To: zijun_hu <zijun_hu@zoho.com>
Cc: David Rientjes <rientjes@google.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
zijun_hu@htc.com, Andrew Morton <akpm@linux-foundation.org>,
tj@kernel.org, mingo@kernel.org, iamjoonsoo.kim@lge.com,
mgorman@techsingularity.net
Subject: Re: [PATCH 3/5] mm/vmalloc.c: correct lazy_max_pages() return value
Date: Thu, 22 Sep 2016 14:37:36 +0200 [thread overview]
Message-ID: <20160922123736.GA11204@dhcp22.suse.cz> (raw)
In-Reply-To: <57E3304E.4060401@zoho.com>
On Thu 22-09-16 09:13:50, zijun_hu wrote:
> On 09/22/2016 08:35 AM, David Rientjes wrote:
[...]
> > The intent is as it is implemented; with your change, lazy_max_pages() is
> > potentially increased depending on the number of online cpus. This is
> > only a heuristic, changing it would need justification on why the new
> > value is better. It is opposite to what the comment says: "to be
> > conservative and not introduce a big latency on huge systems, so go with
> > a less aggressive log scale." NACK to the patch.
> >
> my change potentially make lazy_max_pages() decreased not increased, i seems
> conform with the comment
>
> if the number of online CPUs is not power of 2, both have no any difference
> otherwise, my change remain power of 2 value, and the original code rounds up
> to next power of 2 value, for instance
>
> my change : (32, 64] -> 64
> 32 -> 32, 64 -> 64
> the original code: [32, 63) -> 64
> 32 -> 64, 64 -> 128
You still completely failed to explain _why_ this is an improvement/fix
or why it matters. This all should be in the changelog.
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2016-09-22 12:37 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-21 4:27 [PATCH 3/5] mm/vmalloc.c: correct lazy_max_pages() return value zijun_hu
2016-09-21 21:21 ` David Rientjes
2016-09-21 23:30 ` zijun_hu
2016-09-22 0:35 ` David Rientjes
2016-09-22 1:13 ` zijun_hu
2016-09-22 12:37 ` Michal Hocko [this message]
2016-09-22 16:30 ` zijun_hu
2016-09-23 3:30 ` Nicholas Piggin
2016-09-23 5:00 ` zijun_hu
2016-09-23 7:27 ` Nicholas Piggin
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=20160922123736.GA11204@dhcp22.suse.cz \
--to=mhocko@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=iamjoonsoo.kim@lge.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@techsingularity.net \
--cc=mingo@kernel.org \
--cc=rientjes@google.com \
--cc=tj@kernel.org \
--cc=zijun_hu@htc.com \
--cc=zijun_hu@zoho.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).