From: Michal Hocko <mhocko@kernel.org>
To: Wei Yang <richard.weiyang@gmail.com>
Cc: cl@linux.com, penberg@kernel.org, akpm@linux-foundation.org,
linux-mm@kvack.org
Subject: Re: [PATCH] mm/slub: skip node in case there is no slab to acquire
Date: Tue, 13 Nov 2018 14:34:43 +0100 [thread overview]
Message-ID: <20181113133443.GQ15120@dhcp22.suse.cz> (raw)
In-Reply-To: <20181113132624.xjnvxhrt4jk7mt3m@master>
On Tue 13-11-18 13:26:24, Wei Yang wrote:
> On Tue, Nov 13, 2018 at 02:17:51PM +0100, Michal Hocko wrote:
> >On Thu 08-11-18 09:12:04, Wei Yang wrote:
> >> for_each_zone_zonelist() iterates the zonelist one by one, which means
> >> it will iterate on zones on the same node. While get_partial_node()
> >> checks available slab on node base instead of zone.
> >>
> >> This patch skip a node in case get_partial_node() fails to acquire slab
> >> on that node.
> >
> >If this is an optimization then it should be accompanied by some
> >numbers.
>
> Let me try to get some test result.
>
> Do you have some suggestion on the test suite? Is kernel build a proper
> test?
Make sure that the workload is hitting hard on this particular code path
that it matters. I am not aware of any such workload but others might
know better.
In any case, if you are up to optimize something then you should
evaluate what kind of workload might benefit from it. If there is no
workload then it is likely not worth bothering. Some changes might look
like obvious improvements but then they might add a maintenance burden
or they might be even wrong for other reasons. Recent patches you have
posted show both issues.
I would encourage you to look at a practical issues instead. Throwing
random patches by reading the code without having a larger picture is
usually not the best way to go.
[...]
> In get_page_from_freelist(), we use last_pgdat_dirty_limit to track the
> last node out of dirty limit. I am willing to borrow this idea in
> get_any_partial() to skip a node.
>
> Well, let me do some tests to see whether this is visible.
See the above. Each and every change has its cost and patches make sense
only when both the future maintenance cost and the review cost are payed
off.
--
Michal Hocko
SUSE Labs
next prev parent reply other threads:[~2018-11-13 13:34 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-08 1:12 [PATCH] mm/slub: skip node in case there is no slab to acquire Wei Yang
2018-11-09 20:48 ` Andrew Morton
2018-11-09 23:47 ` Wei Yang
2018-11-13 9:12 ` [PATCH v2] " Wei Yang
2018-11-13 13:17 ` [PATCH] " Michal Hocko
2018-11-13 13:26 ` Wei Yang
2018-11-13 13:34 ` Michal Hocko [this message]
2018-11-20 3:31 ` [PATCH v2] mm/slub: improve performance by skipping checked node in get_any_partial() Wei Yang
2018-11-22 3:05 ` Andrew Morton
2018-11-22 9:13 ` Wei Yang
2018-11-22 23:41 ` Wei Yang
2018-11-23 13:39 ` Michal Hocko
2018-11-23 13:49 ` Michal Hocko
2018-11-23 15:27 ` Wei Yang
2018-12-20 22:41 ` Andrew Morton
2018-12-21 0:25 ` Alexander Duyck
2018-12-21 3:29 ` Wei Yang
2018-12-21 1:37 ` Christopher Lameter
2018-12-21 1:37 ` Christopher Lameter
2018-12-21 3:33 ` Wei Yang
2018-12-24 22:03 ` Wei Yang
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=20181113133443.GQ15120@dhcp22.suse.cz \
--to=mhocko@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=cl@linux.com \
--cc=linux-mm@kvack.org \
--cc=penberg@kernel.org \
--cc=richard.weiyang@gmail.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).