diff for duplicates of <52D79D6B.10304@parallels.com> diff --git a/a/1.txt b/N1/1.txt index 8be49f3..246936e 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -61,3 +61,70 @@ On 01/16/2014 02:53 AM, Andrew Morton wrote: >> if (delta < max_pass / 4) >> total_scan = min(total_scan, max_pass / 2); > Oh, is that what's it's for. Where did you discover this gem? + +>From 3567b59aa80ac4417002bf58e35dce5c777d4164 Mon Sep 17 00:00:00 2001 +From: Dave Chinner <dchinner@redhat.com> +Date: Fri, 8 Jul 2011 14:14:36 +1000 +Subject: [PATCH] vmscan: reduce wind up shrinker->nr when shrinker can't do + work + +>> while (total_scan >= batch_size) ... +>> +>> With this biasing, it is impossible to achieve the ideal behavior I've +>> described above, because we will never accumulate max_pass objects in +>> nr_deferred if memory pressure is low. So, if applied to the real code, +>> this patch takes on a slightly different sense, which I tried to reflect +>> in the comment to the code: it will call ->scan() with nr_to_scan < +>> batch_size only if: +>> +>> 1) max_pass < batch_size && total_scan >= max_pass +>> +>> and +>> +>> 2) we're tight on memory, i.e. the current delta is high (otherwise +>> total_scan will be biased as max_pass / 2 and condition 1 won't be +>> satisfied). +> (is max_pass misnamed?) + +Yes, the name is misleading. I guess, it should be called freeable_cnt, +because we actually scan up to 2*max_pass objects in one pass. + +>> >From our discussion it seems condition 2 is not necessary at all, but it +>> follows directly from the biasing rule. So I propose to tweak the +>> biasing a bit so that total_scan won't be lowered < batch_size: +>> +>> diff --git a/mm/vmscan.c b/mm/vmscan.c +>> index eea668d..78ddd5e 100644 +>> --- a/mm/vmscan.c +>> +++ b/mm/vmscan.c +>> @@ -267,7 +267,7 @@ shrink_slab_node(struct shrink_control *shrinkctl, +>> struct shrinker *shrinker, +>> * a large delta change is calculated directly. +>> */ +>> if (delta < max_pass / 4) +>> - total_scan = min(total_scan, max_pass / 2); +>> + total_scan = min(total_scan, max(max_pass / 2, batch_size)); +>> +>> /* +>> * Avoid risking looping forever due to too large nr value: +>> @@ -281,7 +281,7 @@ shrink_slab_node(struct shrink_control *shrinkctl, +>> struct shrinker *shrinker, +>> nr_pages_scanned, lru_pages, +>> max_pass, delta, total_scan); +>> +>> - while (total_scan >= batch_size) { +>> + while (total_scan >= batch_size || total_scan >= max_pass) { +>> unsigned long ret; +>> +>> shrinkctl->nr_to_scan = batch_size; +>> +>> The first hunk guarantees that total_scan will always accumulate at +>> least batch_size objects no matter how small max_pass is. That means +>> that when max_pass is < batch_size we will eventually get >= max_pass +>> objects to scan and shrink the slab to 0 as we need. What do you think +>> about that? +> I'm a bit lost :( + +I'll try to clean up shrink_slab_node() and resend the patch then. + +Thanks. diff --git a/a/content_digest b/N1/content_digest index 927d69e..4bd1562 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -10,9 +10,9 @@ "Subject\0Re: [PATCH 1/5] mm: vmscan: shrink all slab objects if tight on memory\0" "Date\0Thu, 16 Jan 2014 12:50:51 +0400\0" "To\0Andrew Morton <akpm@linux-foundation.org>\0" - "Cc\0linux-kernel@vger.kernel.org" - linux-mm@kvack.org - devel@openvz.org + "Cc\0<linux-kernel@vger.kernel.org>" + <linux-mm@kvack.org> + <devel@openvz.org> Mel Gorman <mgorman@suse.de> Michal Hocko <mhocko@suse.cz> Johannes Weiner <hannes@cmpxchg.org> @@ -83,6 +83,73 @@ ">>\n" ">> if (delta < max_pass / 4)\n" ">> total_scan = min(total_scan, max_pass / 2);\n" - > Oh, is that what's it's for. Where did you discover this gem? + "> Oh, is that what's it's for. Where did you discover this gem?\n" + "\n" + ">From 3567b59aa80ac4417002bf58e35dce5c777d4164 Mon Sep 17 00:00:00 2001\n" + "From: Dave Chinner <dchinner@redhat.com>\n" + "Date: Fri, 8 Jul 2011 14:14:36 +1000\n" + "Subject: [PATCH] vmscan: reduce wind up shrinker->nr when shrinker can't do\n" + " work\n" + "\n" + ">> while (total_scan >= batch_size) ...\n" + ">>\n" + ">> With this biasing, it is impossible to achieve the ideal behavior I've\n" + ">> described above, because we will never accumulate max_pass objects in\n" + ">> nr_deferred if memory pressure is low. So, if applied to the real code,\n" + ">> this patch takes on a slightly different sense, which I tried to reflect\n" + ">> in the comment to the code: it will call ->scan() with nr_to_scan <\n" + ">> batch_size only if:\n" + ">>\n" + ">> 1) max_pass < batch_size && total_scan >= max_pass\n" + ">>\n" + ">> and\n" + ">>\n" + ">> 2) we're tight on memory, i.e. the current delta is high (otherwise\n" + ">> total_scan will be biased as max_pass / 2 and condition 1 won't be\n" + ">> satisfied).\n" + "> (is max_pass misnamed?)\n" + "\n" + "Yes, the name is misleading. I guess, it should be called freeable_cnt,\n" + "because we actually scan up to 2*max_pass objects in one pass.\n" + "\n" + ">> >From our discussion it seems condition 2 is not necessary at all, but it\n" + ">> follows directly from the biasing rule. So I propose to tweak the\n" + ">> biasing a bit so that total_scan won't be lowered < batch_size:\n" + ">>\n" + ">> diff --git a/mm/vmscan.c b/mm/vmscan.c\n" + ">> index eea668d..78ddd5e 100644\n" + ">> --- a/mm/vmscan.c\n" + ">> +++ b/mm/vmscan.c\n" + ">> @@ -267,7 +267,7 @@ shrink_slab_node(struct shrink_control *shrinkctl,\n" + ">> struct shrinker *shrinker,\n" + ">> * a large delta change is calculated directly.\n" + ">> */\n" + ">> if (delta < max_pass / 4)\n" + ">> - total_scan = min(total_scan, max_pass / 2);\n" + ">> + total_scan = min(total_scan, max(max_pass / 2, batch_size));\n" + ">> \n" + ">> /*\n" + ">> * Avoid risking looping forever due to too large nr value:\n" + ">> @@ -281,7 +281,7 @@ shrink_slab_node(struct shrink_control *shrinkctl,\n" + ">> struct shrinker *shrinker,\n" + ">> nr_pages_scanned, lru_pages,\n" + ">> max_pass, delta, total_scan);\n" + ">> \n" + ">> - while (total_scan >= batch_size) {\n" + ">> + while (total_scan >= batch_size || total_scan >= max_pass) {\n" + ">> unsigned long ret;\n" + ">> \n" + ">> shrinkctl->nr_to_scan = batch_size;\n" + ">>\n" + ">> The first hunk guarantees that total_scan will always accumulate at\n" + ">> least batch_size objects no matter how small max_pass is. That means\n" + ">> that when max_pass is < batch_size we will eventually get >= max_pass\n" + ">> objects to scan and shrink the slab to 0 as we need. What do you think\n" + ">> about that?\n" + "> I'm a bit lost :(\n" + "\n" + "I'll try to clean up shrink_slab_node() and resend the patch then.\n" + "\n" + Thanks. -37c6fd4fc445483b1720ed67945290567e0c50141fa60729981199a67d389e15 +70d087a222c083df887e4c97f7389e395d3cbb00599256ac6cd476ee59a0eba4
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.