On Wed, Nov 21, 2018 at 07:05:55PM -0800, Andrew Morton wrote:
>On Tue, 20 Nov 2018 11:31:19 +0800 Wei Yang <richard.weiyang@xxxxxxxxx> wrote:
>
>> 1. Background
>>
>> Current slub has three layers:
>>
>> * cpu_slab
>> * percpu_partial
>> * per node partial list
>>
>> Slub allocator tries to get an object from top to bottom. When it can't
>> get an object from the upper two layers, it will search the per node
>> partial list. The is done in get_partial().
>>
>> The abstraction of get_partial() may looks like this:
>>
>> get_partial()
>> get_partial_node()
>> get_any_partial()
>> for_each_zone_zonelist()
>>
>> The idea behind this is: it first try a local node, then try other nodes
>> if caller doesn't specify a node.
>>
>> 2. Room for Improvement
>>
>> When we look one step deeper in get_any_partial(), it tries to get a
>> proper node by for_each_zone_zonelist(), which iterates on the
>> node_zonelists.
>>
>> This behavior would introduce some redundant check on the same node.
>> Because:
>>
>> * the local node is already checked in get_partial_node()
>> * one node may have several zones on node_zonelists
>>
>> 3. Solution Proposed in Patch
>>
>> We could reduce these redundant check by record the last unsuccessful
>> node and then skip it.
>>
>> 4. Tests & Result
>>
>> After some tests, the result shows this may improve the system a little,
>> especially on a machine with only one node.
>>
>> 4.1 Test Description
>>
>> There are two cases for two system configurations.
>>
>> Test Cases:
>>
>> 1. counter comparison
>> 2. kernel build test
>>
>> System Configuration:
>>
>> 1. One node machine with 4G
>> 2. Four node machine with 8G
>>
>> 4.2 Result for Test 1
>>
>> Test 1: counter comparison
>>
>> This is a test with hacked kernel to record times function
>> get_any_partial() is invoked and times the inner loop iterates. By
>> comparing the ratio of two counters, we get to know how many inner
>> loops we skipped.
>>
>> Here is a snip of the test patch.
>>
>> ---
>> static void *get_any_partial() {
>>
>> get_partial_count++;
>>
>> do {
>> for_each_zone_zonelist() {
>> get_partial_try_count++;
>> }
>> } while();
>>
>> return NULL;
>> }
>> ---
>>
>> The result of (get_partial_count / get_partial_try_count):
>>
>> +----------+----------------+------------+-------------+
>> | | Base | Patched | Improvement|
>> +----------+----------------+------------+-------------+
>> |One Node | 1:3 | 1:0 | - 100% |
>> +----------+----------------+------------+-------------+
>> |Four Nodes| 1:5.8 | 1:2.5 | - 56% |
>> +----------+----------------+------------+-------------+
>>
>> 4.3 Result for Test 2
>>
>> Test 2: kernel build
>>
>> Command used:
>>
>> > time make -j8 bzImage
>>
>> Each version/system configuration combination has four round kernel
>> build tests. Take the average result of real to compare.
>>
>> +----------+----------------+------------+-------------+
>> | | Base | Patched | Improvement|
>> +----------+----------------+------------+-------------+
>> |One Node | 4m41s | 4m32s | - 4.47% |
>> +----------+----------------+------------+-------------+
>> |Four Nodes| 4m45s | 4m39s | - 2.92% |
>> +----------+----------------+------------+-------------+
>>
>> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
>>
>
>Looks good to me, but I'll await input from the slab maintainers before
>proceeding any further.
>
>I didn't like the variable name much, and the comment could be
>improved. Please review:
>
Looks much better, thanks :-)
>
>--- a/mm/slub.c~mm-slub-improve-performance-by-skipping-checked-node-in-get_any_partial-fix
>+++ a/mm/slub.c
>@@ -1873,7 +1873,7 @@ static void *get_partial_node(struct kme
> * Get a page from somewhere. Search in increasing NUMA distances.
> */
> static void *get_any_partial(struct kmem_cache *s, gfp_t flags,
>- struct kmem_cache_cpu *c, int except)
>+ struct kmem_cache_cpu *c, int exclude_nid)
> {
> #ifdef CONFIG_NUMA
> struct zonelist *zonelist;
>@@ -1911,7 +1911,7 @@ static void *get_any_partial(struct kmem
> for_each_zone_zonelist(zone, z, zonelist, high_zoneidx) {
> struct kmem_cache_node *n;
>
>- if (except == zone_to_nid(zone))
>+ if (exclude_nid == zone_to_nid(zone))
> continue;
>
> n = get_node(s, zone_to_nid(zone));
>@@ -1931,12 +1931,13 @@ static void *get_any_partial(struct kmem
> }
> }
> /*
>- * Fail to get object from this node, either because
>- * 1. Fails in if check
>- * 2. NULl object returns from get_partial_node()
>- * Skip it next time.
>+ * Failed to get an object from this node, either
>+ * because
>+ * 1. Failure in the above if check
>+ * 2. NULL return from get_partial_node()
>+ * So skip this node next time.
> */
>- except = zone_to_nid(zone);
>+ exclude_nid = zone_to_nid(zone);
> }
> } while (read_mems_allowed_retry(cpuset_mems_cookie));
> #endif
>_
--
Wei Yang
Help you, Help me
