Reduce impact to overall system of SLUB using high-order allocations V2
Subsystems (6)
cover Reduce impact to overall system of SLUB using high-order allocations V2
Changelog since V1
o kswapd should sleep if need_resched
o Remove __GFP_REPEAT from GFP flags when speculatively using high orders so direct/compaction exits earlier
o Remove __GFP_NORETRY for correctness
o Correct logic in sleeping_prematurely
o Leave SLUB using the default slub_max_order
There are a few reports of people experiencing hangs when copying large amounts of data with kswapd using a large amount of CPU which appear to be due to recent reclaim changes.
SLUB using high orders is the trigger but not the root cause as SLUB has been using high orders for a while. The following four patches aim to fix the problems in reclaim while reducing the cost for SLUB using those high orders.
Patch 1 corrects logic introduced by commit [1741c877: mm:
kswapd: keep kswapd awake for high-order allocations until a percentage of the node is balanced] to allow kswapd to go to sleep when balanced for high orders.
Patch 2 prevents kswapd waking up in response to SLUBs speculative
use of high orders.
Patch 3 further reduces the cost by prevent SLUB entering direct
compaction or reclaim paths on the grounds that falling back to order-0 should be cheaper.
Patch 4 notes that even when kswapd is failing to keep up with
allocation requests, it should still go to sleep when its quota has expired to prevent it spinning.
My own data on this is not great. I haven't really been able to reproduce the same problem locally.
The test case is simple. "download tar" wgets a large tar file and stores it locally. "unpack" is expanding it (15 times physical RAM in this case) and "delete source dirs" is the tarfile being deleted again. I also experimented with having the tar copied numerous times and into deeper directories to increase the size but the results were not particularly interesting so I left it as one tar.
In the background, applications are being launched to time to vaguely simulate activity on the desktop and to measure how long it takes applications to start.
Test server, 4 CPU threads, x86_64, 2G of RAM, no PREEMPT, no COMPACTION, X running LARGE COPY AND UNTAR
vanilla fixprematurely kswapd-nowwake slub-noexstep kswapdsleep
download tar 95 ( 0.00%) 94 ( 1.06%) 94 ( 1.06%) 94 ( 1.06%) 94 ( 1.06%)
unpack tar 654 ( 0.00%) 649 ( 0.77%) 655 (-0.15%) 589 (11.04%) 598 ( 9.36%)
copy source files 0 ( 0.00%) 0 ( 0.00%) 0 ( 0.00%) 0 ( 0.00%) 0 ( 0.00%)
delete source dirs 327 ( 0.00%) 334 (-2.10%) 318 ( 2.83%) 325 ( 0.62%) 320 ( 2.19%)
MMTests Statistics: duration
User/Sys Time Running Test (seconds) 1139.7 1142.55 1149.78 1109.32 1113.26 Total Elapsed Time (seconds) 1341.59 1342.45 1324.90 1271.02 1247.35
MMTests Statistics: application launch
evolution-wait30 mean 34.92 34.96 34.92 34.92 35.08 gnome-terminal-find mean 7.96 7.96 8.76 7.80 7.96 iceweasel-table mean 7.93 7.81 7.73 7.65 7.88
evolution-wait30 stddev 0.96 1.22 1.27 1.20 1.15 gnome-terminal-find stddev 3.02 3.09 3.51 2.99 3.02 iceweasel-table stddev 1.05 0.90 1.09 1.11 1.11
Having SLUB avoid expensive steps in reclaim improves performance by quite a bit with the overall test completing 1.5 minutes faster. Application launch times were not really affected but it's not something my test machine was suffering from in the first place so it's not really conclusive. The kswapd patches also did not appear to help but again, the test machine wasn't suffering that problem.
These patches are against 2.6.39-rc7. Again, testing would be appreciated.
Documentation/vm/slub.txt | 2 +- mm/page_alloc.c | 3 ++- mm/slub.c | 5 +++-- 3 files changed, 6 insertions(+), 4 deletions(-)
mm/page_alloc.c | 3 ++- mm/slub.c | 3 ++- mm/vmscan.c | 6 +++++- 3 files changed, 9 insertions(+), 3 deletions(-)
Signature · 8 lines
1.7.3.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 1/4 mm: vmscan: Correct use of pgdat_balanced in sleeping_prematurely
Johannes Weiner poined out that the logic in commit [1741c877: mm:
kswapd: keep kswapd awake for high-order allocations until a percentage of the node is balanced] is backwards. Instead of allowing kswapd to go to sleep when balancing for high order allocations, it keeps it kswapd running uselessly.
Will-sign-off-after-Johannes: Mel Gorman <[email protected]>
--- mm/vmscan.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
mm/vmscan.c +1 -1
@@ -2286,7 +2286,7 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining,* must be balanced */ if (order) -return pgdat_balanced (pgdat, balanced, classzone_idx); +return !pgdat_balanced (pgdat, balanced, classzone_idx);else return !all_zones_ok; }
Signature · 8 lines
1.7.3.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Johannes Weiner
maintainerRe: [PATCH 1/4] mm: vmscan: Correct use of pgdat_balanced in sleeping_prematurely
Quoted 7 lines · Mel Gorman
Johannes Weiner poined out that the logic in commit [
1741c877: mm:
kswapd: keep kswapd awake for high-order allocations until a percentage of the node is balanced] is backwards. Instead of allowing kswapd to go to sleep when balancing for high order allocations, it keeps it kswapd running uselessly.From-but-was-not-signed-off-by Johannes Weiner <[email protected]>
Thanks for picking it up, Mel.
Quoted 20 lines
Will-sign-off-after-Johannes: Mel Gorman <[email protected]>
--- mm/vmscan.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)mm/vmscan.c +1
-1@@ -2286,7 +2286,7 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining,* must be balanced */ if (order) -return pgdat_balanced (pgdat, balanced, classzone_idx); +return !pgdat_balanced (pgdat, balanced, classzone_idx);else return !all_zones_ok; }Signature · 1 line
1.7.3.4
Re: [PATCH 1/4] mm: vmscan: Correct use of pgdat_balanced in sleeping_prematurely
In reply to Mel GormanQuoted 8 lines · Mel Gorman
Johannes Weiner poined out that the logic in commit [
1741c877: mm:
kswapd: keep kswapd awake for high-order allocations until a percentage of the node is balanced] is backwards. Instead of allowing kswapd to go to sleep when balancing for high order allocations, it keeps it kswapd running uselessly.From-but-was-not-signed-off-by Johannes Weiner <[email protected]>Will-sign-off-after-Johannes: Mel Gorman <[email protected]>
Nice catch! Hannes.
Signature · 9 lines
Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 1/4] mm: vmscan: Correct use of pgdat_balanced in sleeping_prematurely
In reply to Mel GormanQuoted 8 lines · Mel Gorman
Johannes Weiner poined out that the logic in commit [
1741c877: mm:
kswapd: keep kswapd awake for high-order allocations until a percentage of the node is balanced] is backwards. Instead of allowing kswapd to go to sleep when balancing for high order allocations, it keeps it kswapd running uselessly.From-but-was-not-signed-off-by Johannes Weiner<[email protected]>Will-sign-off-after-Johannes: Mel Gorman<[email protected]>
Signature · 8 lines
All rights reversed -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 2/4 mm: slub: Do not wake kswapd for SLUBs speculative high-order allocations
In reply to Mel GormanTo avoid locking and per-cpu overhead, SLUB optimisically uses high-order allocations and falls back to lower allocations if they fail. However, by simply trying to allocate, kswapd is woken up to start reclaiming at that order. On a desktop system, two users report that the system is getting locked up with kswapd using large amounts of CPU. Using SLAB instead of SLUB made this problem go away.
This patch prevents kswapd being woken up for high-order allocations. Testing indicated that with this patch applied, the system was much harder to hang and even when it did, it eventually recovered.
--- mm/slub.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
mm/slub.c +1 -1
@@ -1170,7 +1170,7 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)* Let the initial higher-order allocation fail under memory pressure * so we fall-back to the minimum order allocation. */ - alloc_gfp = (flags | __GFP_NOWARN | __GFP_NORETRY) & ~__GFP_NOFAIL; + alloc_gfp = (flags | __GFP_NOWARN | __GFP_NORETRY | __GFP_NO_KSWAPD) & ~__GFP_NOFAIL; page =alloc_slab_page (alloc_gfp, node, oo);if (unlikely (!page)) {
Signature · 8 lines
1.7.3.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 2/4] mm: slub: Do not wake kswapd for SLUBs speculative high-order allocations
Quoted 7 lines · Mel Gorman
To avoid locking and per-cpu overhead, SLUB optimisically uses high-order allocations and falls back to lower allocations if they fail. However, by simply trying to allocate, kswapd is woken up to start reclaiming at that order. On a desktop system, two users report that the system is getting locked up with kswapd using large amounts of CPU. Using SLAB instead of SLUB made this problem go away.
Quoted 5 lines
This patch prevents kswapd being woken up for high-order allocations. Testing indicated that with this patch applied, the system was much harder to hang and even when it did, it eventually recovered.
Signed-off-by Mel Gorman <[email protected]>
Re: [PATCH 2/4] mm: slub: Do not wake kswapd for SLUBs speculative high-order allocations
Quoted 9 lines · David Rientjes
Quoted 7 lines · Mel Gorman
To avoid locking and per-cpu overhead, SLUB optimisically uses high-order allocations and falls back to lower allocations if they fail. However, by simply trying to allocate, kswapd is woken up to start reclaiming at that order. On a desktop system, two users report that the system is getting locked up with kswapd using large amounts of CPU. Using SLAB instead of SLUB made this problem go away.
Quoted 6 lines
Quoted 5 lines
This patch prevents kswapd being woken up for high-order allocations. Testing indicated that with this patch applied, the system was much harder to hang and even when it did, it eventually recovered.
Signed-off-by Mel Gorman<[email protected]>Acked-by David Rientjes<[email protected]>
Christoph? I think this patch is sane although the original rationale was to workaround kswapd problems.
Pekka
Re: [PATCH 2/4] mm: slub: Do not wake kswapd for SLUBs speculative high-order allocations
Quoted 14 lines · Pekka Enberg
Quoted 13 lines · David Rientjes
Quoted 11 lines · Mel Gorman
To avoid locking and per-cpu overhead, SLUB optimisically uses high-order allocations and falls back to lower allocations if they fail. However, by simply trying to allocate, kswapd is woken up to start reclaiming at that order. On a desktop system, two users report that the system is getting locked up with kswapd using large amounts of CPU. Using SLAB instead of SLUB made this problem go away.
This patch prevents kswapd being woken up for high-order allocations. Testing indicated that with this patch applied, the system was much harder to hang and even when it did, it eventually recovered.
Quoted 5 lines
Quoted 2 lines
Quoted 1 line
Signed-off-by Mel Gorman<[email protected]>Acked-by David Rientjes<[email protected]>Christoph? I think this patch is sane although the original rationale was to workaround kswapd problems.
I am mostly fine with it. The concerns that I have is if there is a large series of high order allocs then at some point you would want kswapd to be triggered instead of high order allocs constantly failing.
Can we have a "trigger once in a while" functionality?
PATCH 3/4 mm: slub: Do not take expensive steps for SLUBs speculative high-order allocations
In reply to Mel GormanTo avoid locking and per-cpu overhead, SLUB optimisically uses high-order allocations and falls back to lower allocations if they fail. However, by simply trying to allocate, the caller can enter compaction or reclaim - both of which are likely to cost more than the benefit of using high-order pages in SLUB. On a desktop system, two users report that the system is getting stalled with kswapd using large amounts of CPU.
This patch prevents SLUB taking any expensive steps when trying to use high-order allocations. Instead, it is expected to fall back to smaller orders more aggressively. Testing was somewhat inconclusive on how much this helped but it makes sense that falling back to order-0 allocations is faster than entering compaction or direct reclaim.
--- mm/page_alloc.c | 3 ++- mm/slub.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-)
mm/page_alloc.c +2 -1
@@ -1972,6 +1972,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask) {int alloc_flags =ALLOC_WMARK_MIN |ALLOC_CPUSET ;const gfp_t wait = gfp_mask & __GFP_WAIT; +const gfp_t can_wake_kswapd = !(gfp_mask & __GFP_NO_KSWAPD);/* __GFP_HIGH is assumed to be the same as ALLOC_HIGH to save a branch. */ BUILD_BUG_ON (__GFP_HIGH != (__force gfp_t )ALLOC_HIGH );
@@ -1984,7 +1985,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask)*/ alloc_flags |= (__force int ) (gfp_mask & __GFP_HIGH); -if (!wait) { +if (!wait && can_wake_kswapd) {/* * Not worth trying to allocate harder for * __GFP_NOMEMALLOC even if it can't schedule.
mm/slub.c +2 -1
@@ -1170,7 +1170,8 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)* Let the initial higher-order allocation fail under memory pressure * so we fall-back to the minimum order allocation. */ - alloc_gfp = (flags | __GFP_NOWARN | __GFP_NORETRY | __GFP_NO_KSWAPD) & ~__GFP_NOFAIL; + alloc_gfp = (flags | __GFP_NOWARN | __GFP_NO_KSWAPD) & + ~(__GFP_NOFAIL | __GFP_WAIT | __GFP_REPEAT); page =alloc_slab_page (alloc_gfp, node, oo);if (unlikely (!page)) {
Signature · 8 lines
1.7.3.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
David Rientjes
reviewerRe: [PATCH 3/4] mm: slub: Do not take expensive steps for SLUBs speculative high-order allocations
Quoted 26 lines · Mel Gorman
mm/page_alloc.c +2
-1@@ -1972,6 +1972,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask) {int alloc_flags =ALLOC_WMARK_MIN |ALLOC_CPUSET ;const gfp_t wait = gfp_mask & __GFP_WAIT; +const gfp_t can_wake_kswapd = !(gfp_mask & __GFP_NO_KSWAPD);/* __GFP_HIGH is assumed to be the same as ALLOC_HIGH to save a branch. */ BUILD_BUG_ON (__GFP_HIGH != (__force gfp_t )ALLOC_HIGH );@@ -1984,7 +1985,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask)*/ alloc_flags |= (__force int ) (gfp_mask & __GFP_HIGH); -if (!wait) { +if (!wait && can_wake_kswapd) {/* * Not worth trying to allocate harder for * __GFP_NOMEMALLOC even if it can't schedule. mm/slub.c +0
-0
mm/slub.c static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node) · line 1177
Quoted 9 lines
mm/slub.c +2
-1@@ -1170,7 +1170,8 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)* Let the initial higher-order allocation fail under memory pressure * so we fall-back to the minimum order allocation. */ - alloc_gfp = (flags | __GFP_NOWARN | __GFP_NORETRY | __GFP_NO_KSWAPD) & ~__GFP_NOFAIL; + alloc_gfp = (flags | __GFP_NOWARN | __GFP_NO_KSWAPD) & + ~(__GFP_NOFAIL | __GFP_WAIT | __GFP_REPEAT); page =alloc_slab_page (alloc_gfp, node, oo);if (unlikely (!page)) {
It's unnecessary to clear __GFP_REPEAT, these !__GFP_NOFAIL allocations will immediately fail.
alloc_gfp would probably benefit from having a comment about why __GFP_WAIT should be masked off here: that we don't want to do compaction or direct reclaim or retry the allocation more than once (so both __GFP_NORETRY and __GFP_REPEAT are no-ops).
Re: [PATCH 3/4] mm: slub: Do not take expensive steps for SLUBs speculative high-order allocations
Quoted 35 lines · David Rientjes
Quoted 33 lines · Mel Gorman
mm/page_alloc.c +2
-1@@ -1972,6 +1972,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask) {int alloc_flags =ALLOC_WMARK_MIN |ALLOC_CPUSET ;const gfp_t wait = gfp_mask & __GFP_WAIT; +const gfp_t can_wake_kswapd = !(gfp_mask & __GFP_NO_KSWAPD);/* __GFP_HIGH is assumed to be the same as ALLOC_HIGH to save a branch. */ BUILD_BUG_ON (__GFP_HIGH != (__force gfp_t )ALLOC_HIGH );@@ -1984,7 +1985,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask)*/ alloc_flags |= (__force int ) (gfp_mask & __GFP_HIGH); -if (!wait) { +if (!wait && can_wake_kswapd) {/* * Not worth trying to allocate harder for * __GFP_NOMEMALLOC even if it can't schedule. mm/slub.c +2
-1@@ -1170,7 +1170,8 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)* Let the initial higher-order allocation fail under memory pressure * so we fall-back to the minimum order allocation. */ - alloc_gfp = (flags | __GFP_NOWARN | __GFP_NORETRY | __GFP_NO_KSWAPD) & ~__GFP_NOFAIL; + alloc_gfp = (flags | __GFP_NOWARN | __GFP_NO_KSWAPD) & + ~(__GFP_NOFAIL | __GFP_WAIT | __GFP_REPEAT);
Quoted 6 lines
mm/slub.c static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node) · line 1177
Quoted 2 lines
mm/slub.c +0
-0page =alloc_slab_page (alloc_gfp, node, oo);if (unlikely (!page)) {It's unnecessary to clear
__GFP_REPEAT, these !__GFP_NOFAIL allocations will immediately fail.
We can enter enter direct compaction or direct reclaim
at least once. If compaction is enabled and we enter reclaim/compaction, the presense of __GFP_REPEAT makes a difference in should_continue_reclaim(). With compaction disabled, the presense of the flag is relevant in should_alloc_retry() with it being possible to loop in the allocator instead of failing the SLUB allocation and dropping back.
Maybe you meant !__GFP_WAIT instead of !__GFP_NOFAIL which makes more sense. In that case, we clear both flags because __GFP_REPEAT && !_GFP_WAIT is a senseless combination of flags. If for whatever reason the __GFP_WAIT was re-added, the presense of __GFP_REPEAT could cause problems in reclaim that would be hard to spot again.
Quoted 4 lines
alloc_gfpwould probably benefit from having a comment about why__GFP_WAITshould be masked off here: that we don't want to do compaction or direct reclaim or retry the allocation more than once (so both__GFP_NORETRYand__GFP_REPEATare no-ops).
That would have been helpful all right. I should have caught that and explained it properly. In the event there is a new version of the patch, I'll add one. For the moment, I'm dropping this patch entirely. Christoph wants to maintain historic behaviour of SLUB to maximise the number of high-order pages it uses and at the end of the day, which option performs better depends entirely on the workload and machine configuration.
Signature · 9 lines
Mel Gorman SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 3/4] mm: slub: Do not take expensive steps for SLUBs speculative high-order allocations
Quoted 4 lines · Mel Gorman
entirely. Christoph wants to maintain historic behaviour of SLUB to maximise the number of high-order pages it uses and at the end of the day, which option performs better depends entirely on the workload and machine configuration.
That is not what I meant. I would like more higher order allocations to succeed. That does not mean that slubs allocation methods and flags passed have to stay the same. You can change the slub behavior if it helps.
I am just suspicious of compaction. If these mods are needed to reduce the amount of higher order pages then compaction does not have the beneficial effect that it should have. It does not actually increase the available higher order pages. Fix that first.
Re: [PATCH 3/4] mm: slub: Do not take expensive steps for SLUBs speculative high-order allocations
Quoted 11 lines · Christoph Lameter
Quoted 4 lines · Mel Gorman
entirely. Christoph wants to maintain historic behaviour of SLUB to maximise the number of high-order pages it uses and at the end of the day, which option performs better depends entirely on the workload and machine configuration.
That is not what I meant. I would like more higher order allocations to succeed. That does not mean that slubs allocation methods and flags passed have to stay the same. You can change the slub behavior if it helps.
In this particular patch, the success rate for high order allocations would likely decrease in low memory conditions albeit the latency when calling the page allocator will be lower and the disruption to the system will be less (no copying or reclaim of pages). My expectation would be that it's cheaper for SLUB to fall back than compact memory or reclaim pages even if this means a slab page is smaller until more memory is free. However, if the "goodness" criteria is high order allocation success rate, the patch shouldn't be merged.
Quoted 5 lines
I am just suspicious of compaction. If these mods are needed to reduce the amount of higher order pages then compaction does not have the beneficial effect that it should have. It does not actually increase the available higher order pages. Fix that first.
The problem being addressed was the machine being hung at worst and in other cases having kswapd pinned at 99-100% CPU. It's now been shown that modifying SLUB is not necessary to fix this because the bug was in page reclaim. The high-order allocation success rate didn't come into it.
Signature · 9 lines
Mel Gorman SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 3/4] mm: slub: Do not take expensive steps for SLUBs speculative high-order allocations
Quoted 5 lines · Mel Gorman
Quoted 4 lines
That is not what I meant. I would like more higher order allocations to succeed. That does not mean that slubs allocation methods and flags passed have to stay the same. You can change the slub behavior if it helps.
Quoted 8 lines
In this particular patch, the success rate for high order allocations would likely decrease in low memory conditions albeit the latency when calling the page allocator will be lower and the disruption to the system will be less (no copying or reclaim of pages). My expectation would be that it's cheaper for SLUB to fall back than compact memory or reclaim pages even if this means a slab page is smaller until more memory is free. However, if the "goodness" criteria is high order allocation success rate, the patch shouldn't be merged.
The criteria is certainly overall system performance and not a high order allocation rate.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
David Rientjes
reviewerRe: [PATCH 3/4] mm: slub: Do not take expensive steps for SLUBs speculative high-order allocations
Quoted 9 lines · Christoph Lameter
Quoted 8 lines
In this particular patch, the success rate for high order allocations would likely decrease in low memory conditions albeit the latency when calling the page allocator will be lower and the disruption to the system will be less (no copying or reclaim of pages). My expectation would be that it's cheaper for SLUB to fall back than compact memory or reclaim pages even if this means a slab page is smaller until more memory is free. However, if the "goodness" criteria is high order allocation success rate, the patch shouldn't be merged.
Quoted 3 lines
The criteria is certainly overall system performance and not a high order allocation rate.
SLUB definitely depends on these higher order allocations being successful for performance, dropping back to the min order is a last resort as opposed to failing the kmalloc(). If it's the last resort, then it makes sense that we'd want to try both compaction and reclaim while we're already in the page allocator as we go down the slub slowpath. Why not try just a little harder (compaction and/or reclaim) to alloc the cache's preferred order?
David Rientjes
reviewerRe: [PATCH 3/4] mm: slub: Do not take expensive steps for SLUBs speculative high-order allocations
In reply to Mel GormanQuoted 40 lines · Mel Gorman
Quoted 33 lines
Quoted 33 lines
mm/page_alloc.c +2
-1@@ -1972,6 +1972,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask) {int alloc_flags =ALLOC_WMARK_MIN |ALLOC_CPUSET ;const gfp_t wait = gfp_mask & __GFP_WAIT; +const gfp_t can_wake_kswapd = !(gfp_mask & __GFP_NO_KSWAPD);/* __GFP_HIGH is assumed to be the same as ALLOC_HIGH to save a branch. */ BUILD_BUG_ON (__GFP_HIGH != (__force gfp_t )ALLOC_HIGH );@@ -1984,7 +1985,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask)*/ alloc_flags |= (__force int ) (gfp_mask & __GFP_HIGH); -if (!wait) { +if (!wait && can_wake_kswapd) {/* * Not worth trying to allocate harder for * __GFP_NOMEMALLOC even if it can't schedule. mm/slub.c +2
-1@@ -1170,7 +1170,8 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)* Let the initial higher-order allocation fail under memory pressure * so we fall-back to the minimum order allocation. */ - alloc_gfp = (flags | __GFP_NOWARN | __GFP_NORETRY | __GFP_NO_KSWAPD) & ~__GFP_NOFAIL; + alloc_gfp = (flags | __GFP_NOWARN | __GFP_NO_KSWAPD) & + ~(__GFP_NOFAIL | __GFP_WAIT | __GFP_REPEAT);Quoted 6 lines
mm/slub.c static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node) · line 1177
Quoted 2 lines
mm/slub.c +0
-0page =alloc_slab_page (alloc_gfp, node, oo);if (unlikely (!page)) {It's unnecessary to clear
__GFP_REPEAT, these !__GFP_NOFAIL allocations will immediately fail.
Quoted 8 lines
We can enter enter direct compaction or direct reclaim
at least once. If compaction is enabled and we enter reclaim/compaction, the presense of__GFP_REPEATmakes a difference inshould_continue_reclaim(). With compaction disabled, the presense of the flag is relevant inshould_alloc_retry()with it being possible to loop in the allocator instead of failing the SLUB allocation and dropping back.
You've cleared __GFP_WAIT, so it cannot enter direct compaction or direct reclaim, so clearing __GFP_REPEAT here doesn't actually do anything. That's why I suggested adding a comment about why you're clearing __GFP_WAIT: to make it obvious that these allocations will immediately fail if the alloc is unsuccessful and we don't need to add __GFP_NORETRY or remove __GFP_REPEAT.
Quoted 2 lines
Maybe you meant !__GFP_WAIT instead of !__GFP_NOFAIL which makes more sense.
No, I meant !__GFP_NOFAIL since the high priority allocations (if PF_MEMALLOC or TIF_MEMDIE) will not loop forever looking for a page without that bit. That allows this !__GFP_WAIT allocation to immediately fail. __GFP_NORETRY and __GFP_REPEAT are no-ops unless you have __GFP_WAIT.
PATCH 4/4 mm: vmscan: If kswapd has been running too long, allow it to sleep
In reply to Mel GormanUnder constant allocation pressure, kswapd can be in the situation where sleeping_prematurely() will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.
--- mm/vmscan.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
mm/vmscan.c +4 -0
@@ -2251,6 +2251,10 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining,unsigned long balanced =0 ;bool all_zones_ok = true; +/* If kswapd has been running too long, just sleep */ +if (need_resched ()) +return false; +/* If a direct reclaimer woke kswapd within HZ/10, it's premature */ if (remaining)return true;
Signature · 1 line
1.7.3.4
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 14 lines · Mel Gorman
Under constant allocation pressure, kswapd can be in the situation where
sleeping_prematurely()will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.Signed-off-by Mel Gorman<[email protected]>--- mm/vmscan.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)mm/vmscan.c +0
-0
mm/vmscan.c static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, · line 2257
Quoted 7 lines
mm/vmscan.c +4
-0@@ -2251,6 +2251,10 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining,unsigned long balanced =0 ;bool all_zones_ok = true; +/* If kswapd has been running too long, just sleep */ +if (need_resched ()) +return false; +
Hmm... I don't like this patch so much. because this code does
- don't sleep if kswapd got context switch at shrink_inactive_list
- sleep if kswapd didn't
It seems to be semi random behavior.
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 18 lines · KOSAKI Motohiro
Quoted 17 lines · Mel Gorman
Under constant allocation pressure, kswapd can be in the situation where
sleeping_prematurely()will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.Signed-off-by Mel Gorman<[email protected]>--- mm/vmscan.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)mm/vmscan.c +0
-0@@ -2251,6 +2251,10 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining,unsigned long balanced =0 ;bool all_zones_ok = true;
Quoted 8 lines
mm/vmscan.c static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, · line 2257
Quoted 4 lines
mm/vmscan.c +4
-0+/* If kswapd has been running too long, just sleep */ +if (need_resched ()) +return false; +Hmm... I don't like this patch so much. because this code does
- don't sleep if kswapd got context switch at
shrink_inactive_list
This isn't entirely true: need_resched() will be false, so we'll follow the normal path for determining whether to sleep or not, in effect leaving the current behaviour unchanged.
Quoted 1 line
- sleep if kswapd didn't
This also isn't entirely true: whether need_resched() is true at this point depends on a whole lot more that whether we did a context switch in shrink_inactive. It mostly depends on how long we've been running without giving up the CPU. Generally that will mean we've been round the shrinker loop hundreds to thousands of times without sleeping.
Quoted 1 line
It seems to be semi random behavior.
Well, we have to do something. Chris Mason first suspected the hang was a kswapd rescheduling problem a while ago. We tried putting cond_rescheds() in several places in the vmscan code, but to no avail. The need_resched() in sleeping_prematurely() seems to be about the best option. The other option might be just to put a cond_resched() in kswapd_try_to_sleep(), but that will really have about the same effect.
James
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 40 lines · James Bottomley
Quoted 18 lines · KOSAKI Motohiro
Quoted 17 lines · Mel Gorman
Under constant allocation pressure, kswapd can be in the situation where
sleeping_prematurely()will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.Signed-off-by Mel Gorman<[email protected]>---
mm/vmscan.c| 4 ++++1 files changed, 4 insertions(+), 0 deletions(-)mm/vmscan.c +0
-0@@ -2251,6 +2251,10 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, unsigned long balanced = 0; bool all_zones_ok = true;Quoted 8 lines
mm/vmscan.c static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, · line 2257
Quoted 4 lines
mm/vmscan.c +4
-0+ /* If kswapd has been running too long, just sleep */ + if (need_resched()) + return false; +Hmm... I don't like this patch so much. because this code does
- don't sleep if kswapd got context switch at
shrink_inactive_listThis isn't entirely true: need_resched() will be false, so we'll follow the normal path for determining whether to sleep or not, in effect leaving the current behaviour unchanged.
Quoted 1 line
- sleep if kswapd didn't
This also isn't entirely true: whether
need_resched()is true at this point depends on a whole lot more that whether we did a context switch inshrink_inactive. It mostly depends on how long we've been running without giving up the CPU. Generally that will mean we've been round the shrinker loop hundreds to thousands of times without sleeping.
Quoted 5 lines
Quoted 1 line
It seems to be semi random behavior.
Well, we have to do something. Chris Mason first suspected the hang was a kswapd rescheduling problem a while ago. We tried putting
cond_rescheds()in several places in the vmscan code, but to no avail.
Is it a result of test with patch of Hannes(ie, !pgdat_balanced)?
If it isn't, it would be nop regardless of putting cond_reshed at vmscan.c. Because, although we complete zone balancing, kswapd doesn't sleep as pgdat_balance returns wrong result. And at last VM calls balance_pgdat. In this case, balance_pgdat returns without any work as kswap couldn't find zones which have not enough free pages and goto out. kswapd could repeat this work infinitely. So you don't have a chance to call cond_resched.
But if your test was with Hanne's patch, I am very curious how come kswapd consumes CPU a lot.
Quoted 3 lines
The
need_resched()insleeping_prematurely()seems to be about the best option. The other option might be just to put acond_resched()inkswapd_try_to_sleep(), but that will really have about the same effect.
I don't oppose it but before that, I think we have to know why kswapd consumes CPU a lot although we applied Hannes' patch.
Quoted 11 lines
James
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Signature · 9 lines
Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 61 lines · Minchan Kim
Quoted 40 lines · James Bottomley
Quoted 18 lines · KOSAKI Motohiro
Quoted 17 lines · Mel Gorman
Under constant allocation pressure, kswapd can be in the situation where
sleeping_prematurely()will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.Signed-off-by Mel Gorman<[email protected]>---
mm/vmscan.c| 4 ++++1 files changed, 4 insertions(+), 0 deletions(-)mm/vmscan.c +0
-0@@ -2251,6 +2251,10 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, unsigned long balanced = 0; bool all_zones_ok = true;Quoted 8 lines
mm/vmscan.c static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, · line 2257
Quoted 4 lines
mm/vmscan.c +4
-0+ /* If kswapd has been running too long, just sleep */ + if (need_resched()) + return false; +Hmm... I don't like this patch so much. because this code does
- don't sleep if kswapd got context switch at
shrink_inactive_listThis isn't entirely true: need_resched() will be false, so we'll follow the normal path for determining whether to sleep or not, in effect leaving the current behaviour unchanged.
Quoted 1 line
- sleep if kswapd didn't
This also isn't entirely true: whether
need_resched()is true at this point depends on a whole lot more that whether we did a context switch inshrink_inactive. It mostly depends on how long we've been running without giving up the CPU. Generally that will mean we've been round the shrinker loop hundreds to thousands of times without sleeping.Quoted 5 lines
Quoted 1 line
It seems to be semi random behavior.
Well, we have to do something. Chris Mason first suspected the hang was a kswapd rescheduling problem a while ago. We tried putting
cond_rescheds()in several places in the vmscan code, but to no avail.Is it a result of test with patch of
Hannes(ie, !pgdat_balanced)?If it isn't, it would be nop regardless of putting
cond_reshedatvmscan.c. Because, although we complete zone balancing, kswapd doesn't sleep aspgdat_balancereturns wrong result. And at last VM callsbalance_pgdat. In this case,balance_pgdatreturns without any work as kswap couldn't find zones which have not enough free pages and goto out. kswapd could repeat this work infinitely. So you don't have a chance to callcond_resched.But if your test was with Hanne's patch, I am very curious how come kswapd consumes CPU a lot.
Quoted 7 lines
Quoted 3 lines
The
need_resched()insleeping_prematurely()seems to be about the best option. The other option might be just to put acond_resched()inkswapd_try_to_sleep(), but that will really have about the same effect.I don't oppose it but before that, I think we have to know why kswapd consumes CPU a lot although we applied Hannes' patch.
Because it's still possible for processes to allocate pages at the same rate kswapd is freeing them leading to a situation where kswapd does not consider the zone balanced for prolonged periods of time.
Signature · 6 lines
Mel Gorman SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 66 lines · Mel Gorman
Quoted 65 lines · Minchan Kim
Quoted 40 lines · James Bottomley
Quoted 18 lines · KOSAKI Motohiro
Quoted 17 lines · Mel Gorman
Under constant allocation pressure, kswapd can be in the situation where
sleeping_prematurely()will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.Signed-off-by Mel Gorman<[email protected]>---
mm/vmscan.c| 4 ++++1 files changed, 4 insertions(+), 0 deletions(-)mm/vmscan.c +0
-0@@ -2251,6 +2251,10 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, unsigned long balanced = 0; bool all_zones_ok = true;Quoted 8 lines
mm/vmscan.c static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, · line 2257
Quoted 4 lines
mm/vmscan.c +4
-0+ /* If kswapd has been running too long, just sleep */ + if (need_resched()) + return false; +Hmm... I don't like this patch so much. because this code does
- don't sleep if kswapd got context switch at
shrink_inactive_listThis isn't entirely true: need_resched() will be false, so we'll follow the normal path for determining whether to sleep or not, in effect leaving the current behaviour unchanged.
Quoted 1 line
- sleep if kswapd didn't
This also isn't entirely true: whether
need_resched()is true at this point depends on a whole lot more that whether we did a context switch inshrink_inactive. It mostly depends on how long we've been running without giving up the CPU. Generally that will mean we've been round the shrinker loop hundreds to thousands of times without sleeping.Quoted 5 lines
Quoted 1 line
It seems to be semi random behavior.
Well, we have to do something. Chris Mason first suspected the hang was a kswapd rescheduling problem a while ago. We tried putting
cond_rescheds()in several places in the vmscan code, but to no avail.Is it a result of test with patch of
Hannes(ie, !pgdat_balanced)?If it isn't, it would be nop regardless of putting
cond_reshedatvmscan.c. Because, although we complete zone balancing, kswapd doesn't sleep aspgdat_balancereturns wrong result. And at last VM callsbalance_pgdat. In this case,balance_pgdatreturns without any work as kswap couldn't find zones which have not enough free pages and goto out. kswapd could repeat this work infinitely. So you don't have a chance to callcond_resched.But if your test was with Hanne's patch, I am very curious how come kswapd consumes CPU a lot.
Quoted 3 lines
The
need_resched()insleeping_prematurely()seems to be about the best option. The other option might be just to put acond_resched()inkswapd_try_to_sleep(), but that will really have about the same effect.
Quoted 7 lines
Quoted 3 lines
I don't oppose it but before that, I think we have to know why kswapd consumes CPU a lot although we applied Hannes' patch.
Because it's still possible for processes to allocate pages at the same rate kswapd is freeing them leading to a situation where kswapd does not consider the zone balanced for prolonged periods of time.
We have cond_resched in shrink_page_list, shrink_slab and balance_pgdat. So I think kswapd can be scheduled out although it's scheduled in after a short time as task scheduled also need page reclaim. Although all task in system need reclaim, kswapd cpu 99% consumption is a natural result, I think.
Do I miss something?
Quoted 5 lines
--
Mel Gorman
SUSE Labs
Signature · 6 lines
Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 75 lines · Minchan Kim
Quoted 66 lines · Mel Gorman
Quoted 65 lines · Minchan Kim
Quoted 40 lines · James Bottomley
Quoted 18 lines · KOSAKI Motohiro
Quoted 17 lines · Mel Gorman
Under constant allocation pressure, kswapd can be in the situation where
sleeping_prematurely()will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.Signed-off-by Mel Gorman<[email protected]>---
mm/vmscan.c| 4 ++++1 files changed, 4 insertions(+), 0 deletions(-)mm/vmscan.c +0
-0@@ -2251,6 +2251,10 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, unsigned long balanced = 0; bool all_zones_ok = true;Quoted 8 lines
mm/vmscan.c static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, · line 2257
Quoted 4 lines
mm/vmscan.c +4
-0+ /* If kswapd has been running too long, just sleep */ + if (need_resched()) + return false; +Hmm... I don't like this patch so much. because this code does
- don't sleep if kswapd got context switch at
shrink_inactive_listThis isn't entirely true: need_resched() will be false, so we'll follow the normal path for determining whether to sleep or not, in effect leaving the current behaviour unchanged.
Quoted 1 line
- sleep if kswapd didn't
This also isn't entirely true: whether
need_resched()is true at this point depends on a whole lot more that whether we did a context switch inshrink_inactive. It mostly depends on how long we've been running without giving up the CPU. Generally that will mean we've been round the shrinker loop hundreds to thousands of times without sleeping.Quoted 5 lines
Quoted 1 line
It seems to be semi random behavior.
Well, we have to do something. Chris Mason first suspected the hang was a kswapd rescheduling problem a while ago. We tried putting
cond_rescheds()in several places in the vmscan code, but to no avail.Is it a result of test with patch of
Hannes(ie, !pgdat_balanced)?If it isn't, it would be nop regardless of putting
cond_reshedatvmscan.c. Because, although we complete zone balancing, kswapd doesn't sleep aspgdat_balancereturns wrong result. And at last VM callsbalance_pgdat. In this case,balance_pgdatreturns without any work as kswap couldn't find zones which have not enough free pages and goto out. kswapd could repeat this work infinitely. So you don't have a chance to callcond_resched.But if your test was with Hanne's patch, I am very curious how come kswapd consumes CPU a lot.
Quoted 3 lines
The
need_resched()insleeping_prematurely()seems to be about the best option. The other option might be just to put acond_resched()inkswapd_try_to_sleep(), but that will really have about the same effect.Quoted 7 lines
Quoted 3 lines
I don't oppose it but before that, I think we have to know why kswapd consumes CPU a lot although we applied Hannes' patch.
Because it's still possible for processes to allocate pages at the same rate kswapd is freeing them leading to a situation where kswapd does not consider the zone balanced for prolonged periods of time.
Quoted 7 lines
We have
cond_reschedinshrink_page_list,shrink_slabandbalance_pgdat. So I think kswapd can be scheduled out although it's scheduled in after a short time as task scheduled also need page reclaim. Although all task in system need reclaim, kswapd cpu 99% consumption is a natural result, I think.
Do I miss something?
Lets see;
shrink_page_list() only applies if inactive pages were isolated
which in turn may not happen if all_unreclaimable is set in shrink_zones(). If for whatver reason, all_unreclaimable is set on all zones, we can miss calling cond_resched().
shrink_slab only applies if we are reclaiming slab pages. If the first
shrinker returns -1, we do not call cond_resched(). If that first shrinker is dcache and __GFP_FS is not set, direct reclaimers will not shrink at all. However, if there are enough of them running or if one of the other shrinkers is running for a very long time, kswapd could be starved acquiring the shrinker_rwsem and never reaching the cond_resched().
balance_pgdat() only calls cond_resched if the zones are not
balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that was reclaiming for order-0 to kswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-enters balance_pgdat() without ever have called cond_resched().
While it appears unlikely, there are bad conditions which can result in cond_resched() being avoided.
Signature · 6 lines
Mel Gorman SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 91 lines · Mel Gorman
Quoted 75 lines · Minchan Kim
Quoted 66 lines · Mel Gorman
Quoted 65 lines · Minchan Kim
Quoted 40 lines · James Bottomley
Quoted 18 lines · KOSAKI Motohiro
Quoted 17 lines · Mel Gorman
Under constant allocation pressure, kswapd can be in the situation where
sleeping_prematurely()will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.Signed-off-by Mel Gorman<[email protected]>---
mm/vmscan.c| 4 ++++1 files changed, 4 insertions(+), 0 deletions(-)mm/vmscan.c +0
-0@@ -2251,6 +2251,10 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, unsigned long balanced = 0; bool all_zones_ok = true;Quoted 8 lines
mm/vmscan.c static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, · line 2257
Quoted 4 lines
mm/vmscan.c +4
-0+ /* If kswapd has been running too long, just sleep */ + if (need_resched()) + return false; +Hmm... I don't like this patch so much. because this code does
- don't sleep if kswapd got context switch at
shrink_inactive_listThis isn't entirely true: need_resched() will be false, so we'll follow the normal path for determining whether to sleep or not, in effect leaving the current behaviour unchanged.
Quoted 1 line
- sleep if kswapd didn't
This also isn't entirely true: whether
need_resched()is true at this point depends on a whole lot more that whether we did a context switch inshrink_inactive. It mostly depends on how long we've been running without giving up the CPU. Generally that will mean we've been round the shrinker loop hundreds to thousands of times without sleeping.Quoted 5 lines
Quoted 1 line
It seems to be semi random behavior.
Well, we have to do something. Chris Mason first suspected the hang was a kswapd rescheduling problem a while ago. We tried putting
cond_rescheds()in several places in the vmscan code, but to no avail.Is it a result of test with patch of
Hannes(ie, !pgdat_balanced)?If it isn't, it would be nop regardless of putting
cond_reshedatvmscan.c. Because, although we complete zone balancing, kswapd doesn't sleep aspgdat_balancereturns wrong result. And at last VM callsbalance_pgdat. In this case,balance_pgdatreturns without any work as kswap couldn't find zones which have not enough free pages and goto out. kswapd could repeat this work infinitely. So you don't have a chance to callcond_resched.But if your test was with Hanne's patch, I am very curious how come kswapd consumes CPU a lot.
Quoted 3 lines
The
need_resched()insleeping_prematurely()seems to be about the best option. The other option might be just to put acond_resched()inkswapd_try_to_sleep(), but that will really have about the same effect.Quoted 7 lines
Quoted 3 lines
I don't oppose it but before that, I think we have to know why kswapd consumes CPU a lot although we applied Hannes' patch.
Because it's still possible for processes to allocate pages at the same rate kswapd is freeing them leading to a situation where kswapd does not consider the zone balanced for prolonged periods of time.
Quoted 7 lines
We have
cond_reschedinshrink_page_list,shrink_slabandbalance_pgdat. So I think kswapd can be scheduled out although it's scheduled in after a short time as task scheduled also need page reclaim. Although all task in system need reclaim, kswapd cpu 99% consumption is a natural result, I think.
Do I miss something?Lets see;
shrink_page_list()only applies if inactive pages were isolated which in turn may not happen ifall_unreclaimableis set inshrink_zones(). If for whatver reason,all_unreclaimableis set on all zones, we can miss callingcond_resched().
Quoted 8 lines
shrink_slabonly applies if we are reclaiming slab pages. If the first shrinker returns -1, we do not callcond_resched(). If that first shrinker is dcache and__GFP_FSis not set, direct reclaimers will not shrink at all. However, if there are enough of them running or if one of the other shrinkers is running for a very long time, kswapd could be starved acquiring theshrinker_rwsemand never reaching thecond_resched().
Don't we have to move cond_resched?
mm/vmscan.c +7 -3
@@ -231,8 +231,10 @@ unsigned long shrink_slab(struct shrink_control *shrink,if (scanned ==0 ) scanned =SWAP_CLUSTER_MAX ; -if (!down_read_trylock (&shrinker_rwsem)) -return 1 ;/* Assume we'll be able to shrink next time */ +if (!down_read_trylock (&shrinker_rwsem)) { + ret =1 ; + goto out;/* Assume we'll be able to shrink next time */ + }list_for_each_entry (shrinker, &shrinker_list, list) {unsigned long long delta;
@@ -280,12 +282,14 @@ unsigned long shrink_slab(struct shrink_control *shrink,count_vm_events (SLABS_SCANNED , this_scan); total_scan -= this_scan; -cond_resched (); } shrinker->nr += total_scan; +cond_resched (); }up_read (&shrinker_rwsem); +out: +cond_resched ();return ret; }
Quoted 8 lines
balance_pgdat()only callscond_reschedif the zones are not balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that was reclaiming for order-0 tokswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-entersbalance_pgdat()without ever have calledcond_resched().
If kswapd reclaims order-o followed by high order, it would have a chance to call cond_resched in shrink_page_list. But if all zones are all_unreclaimable is set, balance_pgdat could return any work. Okay. It does make sense.
By your scenario, someone wakes up kswapd with higher order, again. So re-enters balance_pgdat without ever have called cond_resched. But if someone wakes up higher order again, we can't have a chance to call kswapd_try_to_sleep. So your patch effect would be nop, too.
It would be better to put cond_resched after balance_pgdat?
mm/vmscan.c +1 -0
@@ -2753,6 +2753,7 @@ static int kswapd(void *p)if (!ret) {trace_mm_vmscan_kswapd_wake (pgdat->node_id, order); order =balance_pgdat (pgdat, order, &classzone_idx); +cond_resched (); } }return 0 ;
Quoted 9 lines
While it appears unlikely, there are bad conditions which can result in
cond_resched()being avoided.--
Mel Gorman
SUSE Labs
Signature · 9 lines
Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 146 lines · Minchan Kim
Quoted 91 lines · Mel Gorman
Quoted 75 lines · Minchan Kim
Quoted 66 lines · Mel Gorman
Quoted 65 lines · Minchan Kim
Quoted 40 lines · James Bottomley
Quoted 18 lines · KOSAKI Motohiro
Quoted 17 lines · Mel Gorman
Under constant allocation pressure, kswapd can be in the situation where
sleeping_prematurely()will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.Signed-off-by Mel Gorman<[email protected]>---
mm/vmscan.c| 4 ++++1 files changed, 4 insertions(+), 0 deletions(-)mm/vmscan.c +0
-0@@ -2251,6 +2251,10 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, unsigned long balanced = 0; bool all_zones_ok = true;Quoted 8 lines
mm/vmscan.c static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, · line 2257
Quoted 4 lines
mm/vmscan.c +4
-0+ /* If kswapd has been running too long, just sleep */ + if (need_resched()) + return false; +Hmm... I don't like this patch so much. because this code does
- don't sleep if kswapd got context switch at
shrink_inactive_listThis isn't entirely true: need_resched() will be false, so we'll follow the normal path for determining whether to sleep or not, in effect leaving the current behaviour unchanged.
Quoted 1 line
- sleep if kswapd didn't
This also isn't entirely true: whether
need_resched()is true at this point depends on a whole lot more that whether we did a context switch inshrink_inactive. It mostly depends on how long we've been running without giving up the CPU. Generally that will mean we've been round the shrinker loop hundreds to thousands of times without sleeping.Quoted 5 lines
Quoted 1 line
It seems to be semi random behavior.
Well, we have to do something. Chris Mason first suspected the hang was a kswapd rescheduling problem a while ago. We tried putting
cond_rescheds()in several places in the vmscan code, but to no avail.Is it a result of test with patch of
Hannes(ie, !pgdat_balanced)?If it isn't, it would be nop regardless of putting
cond_reshedatvmscan.c. Because, although we complete zone balancing, kswapd doesn't sleep aspgdat_balancereturns wrong result. And at last VM callsbalance_pgdat. In this case,balance_pgdatreturns without any work as kswap couldn't find zones which have not enough free pages and goto out. kswapd could repeat this work infinitely. So you don't have a chance to callcond_resched.But if your test was with Hanne's patch, I am very curious how come kswapd consumes CPU a lot.
Quoted 3 lines
The
need_resched()insleeping_prematurely()seems to be about the best option. The other option might be just to put acond_resched()inkswapd_try_to_sleep(), but that will really have about the same effect.Quoted 7 lines
Quoted 3 lines
I don't oppose it but before that, I think we have to know why kswapd consumes CPU a lot although we applied Hannes' patch.
Because it's still possible for processes to allocate pages at the same rate kswapd is freeing them leading to a situation where kswapd does not consider the zone balanced for prolonged periods of time.
Quoted 7 lines
We have
cond_reschedinshrink_page_list,shrink_slabandbalance_pgdat. So I think kswapd can be scheduled out although it's scheduled in after a short time as task scheduled also need page reclaim. Although all task in system need reclaim, kswapd cpu 99% consumption is a natural result, I think.
Do I miss something?Lets see;
shrink_page_list()only applies if inactive pages were isolated which in turn may not happen ifall_unreclaimableis set inshrink_zones(). If for whatver reason,all_unreclaimableis set on all zones, we can miss callingcond_resched().Quoted 8 lines
shrink_slabonly applies if we are reclaiming slab pages. If the first shrinker returns -1, we do not callcond_resched(). If that first shrinker is dcache and__GFP_FSis not set, direct reclaimers will not shrink at all. However, if there are enough of them running or if one of the other shrinkers is running for a very long time, kswapd could be starved acquiring theshrinker_rwsemand never reaching thecond_resched().Don't we have to move
cond_resched?mm/vmscan.c +7
-3@@ -231,8 +231,10 @@ unsigned long shrink_slab(struct shrink_control *shrink, if (scanned == 0) scanned = SWAP_CLUSTER_MAX; - if (!down_read_trylock(&shrinker_rwsem)) - return 1; /* Assume we'll be able to shrink next time */ + if (!down_read_trylock(&shrinker_rwsem)) { + ret = 1; + goto out; /* Assume we'll be able to shrink next time */ + } list_for_each_entry(shrinker, &shrinker_list, list) { unsigned long long delta;@@ -280,12 +282,14 @@ unsigned long shrink_slab(struct shrink_control *shrink, count_vm_events(SLABS_SCANNED, this_scan); total_scan -= this_scan; - cond_resched(); } shrinker->nr += total_scan; + cond_resched(); } up_read(&shrinker_rwsem); +out: + cond_resched(); return ret; }Quoted 9 lines
balance_pgdat()only callscond_reschedif the zones are not balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that was reclaiming for order-0 tokswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-entersbalance_pgdat()without ever have calledcond_resched().
Quoted 3 lines
If kswapd reclaims order-o followed by high order, it would have a chance to call
cond_reschedinshrink_page_list. But if all zones areall_unreclaimableis set,balance_pgdatcould return any work.
Typo : without any work.
Signature · 6 lines
Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
In reply to Minchan KimQuoted 126 lines · Minchan Kim
Quoted 91 lines · Mel Gorman
Quoted 75 lines · Minchan Kim
Quoted 66 lines · Mel Gorman
Quoted 65 lines · Minchan Kim
Quoted 40 lines · James Bottomley
Quoted 18 lines · KOSAKI Motohiro
Quoted 17 lines · Mel Gorman
Under constant allocation pressure, kswapd can be in the situation where
sleeping_prematurely()will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.Signed-off-by Mel Gorman<[email protected]>---
mm/vmscan.c| 4 ++++1 files changed, 4 insertions(+), 0 deletions(-)mm/vmscan.c +0
-0@@ -2251,6 +2251,10 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, unsigned long balanced = 0; bool all_zones_ok = true;Quoted 8 lines
mm/vmscan.c static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, · line 2257
Quoted 4 lines
mm/vmscan.c +4
-0+ /* If kswapd has been running too long, just sleep */ + if (need_resched()) + return false; +Hmm... I don't like this patch so much. because this code does
- don't sleep if kswapd got context switch at
shrink_inactive_listThis isn't entirely true: need_resched() will be false, so we'll follow the normal path for determining whether to sleep or not, in effect leaving the current behaviour unchanged.
Quoted 1 line
- sleep if kswapd didn't
This also isn't entirely true: whether
need_resched()is true at this point depends on a whole lot more that whether we did a context switch inshrink_inactive. It mostly depends on how long we've been running without giving up the CPU. Generally that will mean we've been round the shrinker loop hundreds to thousands of times without sleeping.Quoted 5 lines
Quoted 1 line
It seems to be semi random behavior.
Well, we have to do something. Chris Mason first suspected the hang was a kswapd rescheduling problem a while ago. We tried putting
cond_rescheds()in several places in the vmscan code, but to no avail.Is it a result of test with patch of
Hannes(ie, !pgdat_balanced)?If it isn't, it would be nop regardless of putting
cond_reshedatvmscan.c. Because, although we complete zone balancing, kswapd doesn't sleep aspgdat_balancereturns wrong result. And at last VM callsbalance_pgdat. In this case,balance_pgdatreturns without any work as kswap couldn't find zones which have not enough free pages and goto out. kswapd could repeat this work infinitely. So you don't have a chance to callcond_resched.But if your test was with Hanne's patch, I am very curious how come kswapd consumes CPU a lot.
Quoted 3 lines
The
need_resched()insleeping_prematurely()seems to be about the best option. The other option might be just to put acond_resched()inkswapd_try_to_sleep(), but that will really have about the same effect.Quoted 7 lines
Quoted 3 lines
I don't oppose it but before that, I think we have to know why kswapd consumes CPU a lot although we applied Hannes' patch.
Because it's still possible for processes to allocate pages at the same rate kswapd is freeing them leading to a situation where kswapd does not consider the zone balanced for prolonged periods of time.
Quoted 7 lines
We have
cond_reschedinshrink_page_list,shrink_slabandbalance_pgdat. So I think kswapd can be scheduled out although it's scheduled in after a short time as task scheduled also need page reclaim. Although all task in system need reclaim, kswapd cpu 99% consumption is a natural result, I think.
Do I miss something?Lets see;
shrink_page_list()only applies if inactive pages were isolated which in turn may not happen ifall_unreclaimableis set inshrink_zones(). If for whatver reason,all_unreclaimableis set on all zones, we can miss callingcond_resched().Quoted 8 lines
shrink_slabonly applies if we are reclaiming slab pages. If the first shrinker returns -1, we do not callcond_resched(). If that first shrinker is dcache and__GFP_FSis not set, direct reclaimers will not shrink at all. However, if there are enough of them running or if one of the other shrinkers is running for a very long time, kswapd could be starved acquiring theshrinker_rwsemand never reaching thecond_resched().Don't we have to move
cond_resched?mm/vmscan.c +4
-3@@ -231,8 +231,10 @@ unsigned long shrink_slab(struct shrink_control *shrink,if (scanned ==0 ) scanned =SWAP_CLUSTER_MAX ; -if (!down_read_trylock (&shrinker_rwsem)) -return 1 ;/* Assume we'll be able to shrink next time */ +if (!down_read_trylock (&shrinker_rwsem)) { + ret =1 ; + goto out;/* Assume we'll be able to shrink next time */ + }list_for_each_entry (shrinker, &shrinker_list, list) {unsigned long long delta;@@ -280,12 +282,14 @@ unsigned long shrink_slab(struct shrink_control *shrink,count_vm_events (SLABS_SCANNED , this_scan); total_scan -= this_scan; -cond_resched (); }
mm/vmscan.c unsigned long shrink_slab(struct shrink_control *shrink, · line 295
Quoted 10 lines
mm/vmscan.c +3
-0shrinker->nr += total_scan; +cond_resched (); }up_read (&shrinker_rwsem); +out: +cond_resched ();return ret; }
This makes some sense for the exit path but if one or more of the shrinkers takes a very long time without sleeping (extremely long list searches for example) then kswapd will not call cond_resched() between shrinkers and still consume a lot of CPU.
Quoted 18 lines
Quoted 8 lines
balance_pgdat()only callscond_reschedif the zones are not balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that was reclaiming for order-0 tokswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-entersbalance_pgdat()without ever have calledcond_resched().If kswapd reclaims order-o followed by high order, it would have a chance to call
cond_reschedinshrink_page_list. But if all zones areall_unreclaimableis set,balance_pgdatcould return any work. Okay. It does make sense.
By your scenario, someone wakes up kswapd with higher order, again. So re-entersbalance_pgdatwithout ever have calledcond_resched. But if someone wakes up higher order again, we can't have a chance to callkswapd_try_to_sleep. So your patch effect would be nop, too.
Quoted 2 lines
It would be better to put
cond_reschedafterbalance_pgdat?
Which will leave kswapd runnable instead of going to sleep but guarantees a scheduling point. Lets see if the problem is that cond_resched is being missed although if this was the case then patch 4 would truly be a no-op but Colin has already reported that patch 1 on its own didn't fix his problem. If the problem is sandybridge-specific where kswapd remains runnable and consuming large amounts of CPU in turbo mode then we know that there are other cond_resched() decisions that will need to be revisited.
Colin or James, would you be willing to test with patch 1 from this series and Minchan's patch below? Thanks.
Quoted 28 lines
mm/vmscan.c +1
-0@@ -2753,6 +2753,7 @@ static int kswapd(void *p)if (!ret) {trace_mm_vmscan_kswapd_wake (pgdat->node_id, order); order =balance_pgdat (pgdat, order, &classzone_idx); +cond_resched (); } }return 0 ;Quoted 10 lines
While it appears unlikely, there are bad conditions which can result in
cond_resched()being avoided.--
Mel Gorman
SUSE LabsSignature · 2 lines
Kind regards, Minchan Kim
Signature · 9 lines
Mel Gorman SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 173 lines · Mel Gorman
Quoted 126 lines · Minchan Kim
Quoted 91 lines · Mel Gorman
Quoted 75 lines · Minchan Kim
Quoted 66 lines · Mel Gorman
Quoted 65 lines · Minchan Kim
Quoted 40 lines · James Bottomley
Quoted 18 lines · KOSAKI Motohiro
Quoted 17 lines · Mel Gorman
Under constant allocation pressure, kswapd can be in the situation where
sleeping_prematurely()will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.Signed-off-by Mel Gorman<[email protected]>--- mm/vmscan.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)mm/vmscan.c +0
-0@@ -2251,6 +2251,10 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining,unsigned long balanced =0 ;bool all_zones_ok = true;Quoted 8 lines
mm/vmscan.c static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, · line 2257
Quoted 4 lines
mm/vmscan.c +4
-0+/* If kswapd has been running too long, just sleep */ +if (need_resched ()) +return false; +Hmm... I don't like this patch so much. because this code does
- don't sleep if kswapd got context switch at
shrink_inactive_listThis isn't entirely true:
need_resched()will be false, so we'll follow the normal path for determining whether to sleep or not, in effect leaving the current behaviour unchanged.Quoted 1 line
- sleep if kswapd didn't
This also isn't entirely true: whether
need_resched()is true at this point depends on a whole lot more that whether we did a context switch inshrink_inactive. It mostly depends on how long we've been running without giving up the CPU. Generally that will mean we've been round the shrinker loop hundreds to thousands of times without sleeping.Quoted 5 lines
Quoted 1 line
It seems to be semi random behavior.
Well, we have to do something. Chris Mason first suspected the hang was a kswapd rescheduling problem a while ago. We tried putting
cond_rescheds()in several places in the vmscan code, but to no avail.Is it a result of test with patch of
Hannes(ie, !pgdat_balanced)?If it isn't, it would be nop regardless of putting
cond_reshedatvmscan.c. Because, although we complete zone balancing, kswapd doesn't sleep aspgdat_balancereturns wrong result. And at last VM callsbalance_pgdat. In this case,balance_pgdatreturns without any work as kswap couldn't find zones which have not enough free pages and goto out. kswapd could repeat this work infinitely. So you don't have a chance to callcond_resched.But if your test was with Hanne's patch, I am very curious how come kswapd consumes CPU a lot.
Quoted 3 lines
The
need_resched()insleeping_prematurely()seems to be about the best option. The other option might be just to put acond_resched()inkswapd_try_to_sleep(), but that will really have about the same effect.Quoted 7 lines
Quoted 3 lines
I don't oppose it but before that, I think we have to know why kswapd consumes CPU a lot although we applied Hannes' patch.
Because it's still possible for processes to allocate pages at the same rate kswapd is freeing them leading to a situation where kswapd does not consider the zone balanced for prolonged periods of time.
Quoted 7 lines
We have
cond_reschedinshrink_page_list,shrink_slabandbalance_pgdat. So I think kswapd can be scheduled out although it's scheduled in after a short time as task scheduled also need page reclaim. Although all task in system need reclaim, kswapd cpu 99% consumption is a natural result, I think.
Do I miss something?Lets see;
shrink_page_list()only applies if inactive pages were isolated
which in turn may not happen ifall_unreclaimableis set inshrink_zones(). If for whatver reason,all_unreclaimableis set on all zones, we can miss callingcond_resched().Quoted 8 lines
shrink_slabonly applies if we are reclaiming slab pages. If the first
shrinker returns -1, we do not callcond_resched(). If that first shrinker is dcache and__GFP_FSis not set, direct reclaimers will not shrink at all. However, if there are enough of them running or if one of the other shrinkers is running for a very long time, kswapd could be starved acquiring theshrinker_rwsemand never reaching thecond_resched().Don't we have to move
cond_resched?mm/vmscan.c +4
-3@@ -231,8 +231,10 @@ unsigned long shrink_slab(struct shrink_control *shrink,if (scanned ==0 ) scanned =SWAP_CLUSTER_MAX ; -if (!down_read_trylock (&shrinker_rwsem)) -return 1 ;/* Assume we'll be able to shrink next time */ +if (!down_read_trylock (&shrinker_rwsem)) { + ret =1 ; + goto out;/* Assume we'll be able to shrink next time */ + }list_for_each_entry (shrinker, &shrinker_list, list) {unsigned long long delta;@@ -280,12 +282,14 @@ unsigned long shrink_slab(struct shrink_control *shrink,count_vm_events (SLABS_SCANNED , this_scan); total_scan -= this_scan; -cond_resched (); }mm/vmscan.c unsigned long shrink_slab(struct shrink_control *shrink, · line 295
Quoted 10 lines
mm/vmscan.c +3
-0shrinker->nr += total_scan; +cond_resched (); }up_read (&shrinker_rwsem); +out: +cond_resched ();return ret; }This makes some sense for the exit path but if one or more of the shrinkers takes a very long time without sleeping (extremely long list searches for example) then kswapd will not call
cond_resched()between shrinkers and still consume a lot of CPU.Quoted 18 lines
Quoted 8 lines
balance_pgdat()only callscond_reschedif the zones are not
balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that was reclaiming for order-0 tokswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-entersbalance_pgdat()without ever have calledcond_resched().If kswapd reclaims order-o followed by high order, it would have a chance to call
cond_reschedinshrink_page_list. But if all zones areall_unreclaimableis set,balance_pgdatcould return any work. Okay. It does make sense.
By your scenario, someone wakes up kswapd with higher order, again. So re-entersbalance_pgdatwithout ever have calledcond_resched. But if someone wakes up higher order again, we can't have a chance to callkswapd_try_to_sleep. So your patch effect would be nop, too.Quoted 2 lines
It would be better to put
cond_reschedafterbalance_pgdat?Which will leave kswapd runnable instead of going to sleep but guarantees a scheduling point. Lets see if the problem is that
cond_reschedis being missed although if this was the case then patch 4 would truly be a no-op but Colin has already reported that patch 1 on its own didn't fix his problem. If the problem is sandybridge-specific where kswapd remains runnable and consuming large amounts of CPU in turbo mode then we know that there are othercond_resched()decisions that will need to be revisited.
Quoted 2 lines
Colin or James, would you be willing to test with patch 1 from this series and Minchan's patch below? Thanks.
This works OK fine. Ran 250 test cycles for about 2 hours.
Quoted 30 lines
Quoted 29 lines
mm/vmscan.c +1
-0@@ -2753,6 +2753,7 @@ static int kswapd(void *p)if (!ret) {trace_mm_vmscan_kswapd_wake (pgdat->node_id, order); order =balance_pgdat (pgdat, order, &classzone_idx); +cond_resched (); } }return 0 ;Quoted 10 lines
While it appears unlikely, there are bad conditions which can result in
cond_resched()being avoided.--
Mel Gorman
SUSE LabsSignature · 2 lines
Kind regards, Minchan Kim
PATCH 1/1 mm: vmscan: Correctly check if reclaimer should schedule during shrink_slab
It has been reported on some laptops that kswapd is consuming large amounts of CPU and not being scheduled when SLUB is enabled during large amounts of file copying. It is expected that this is due to kswapd missing every cond_resched() point because;
shrink_page_list() calls cond_resched() if inactive pages were isolated
which in turn may not happen if all_unreclaimable is set in shrink_zones(). If for whatver reason, all_unreclaimable is set on all zones, we can miss calling cond_resched().
balance_pgdat() only calls cond_resched if the zones are not
balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that it was reclaiming for order-0 to kswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-enters balance_pgdat() without ever calling cond_resched().
shrink_slab only calls cond_resched() if we are reclaiming slab
pages. If there are a large number of direct reclaimers, the shrinker_rwsem can be contended and prevent kswapd calling cond_resched().
This patch modifies the shrink_slab() case. If the semaphore is contended, the caller will still check cond_resched(). After each successful call into a shrinker, the check for cond_resched() is still necessary in case one shrinker call is particularly slow.
This patch replaces mm-vmscan-if-kswapd-has-been-running-too-long-allow-it-to-sleep.patch in -mm.
[[email protected]: Preserve call to cond_resched after each call into shrinker]
From: Minchan Kim <[email protected]>
--- mm/vmscan.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
mm/vmscan.c +7 -2
@@ -230,8 +230,11 @@ unsigned long shrink_slab(unsigned long scanned, gfp_t gfp_mask,if (scanned ==0 ) scanned =SWAP_CLUSTER_MAX ; -if (!down_read_trylock (&shrinker_rwsem)) -return 1 ;/* Assume we'll be able to shrink next time */ +if (!down_read_trylock (&shrinker_rwsem)) { +/* Assume we'll be able to shrink next time */ + ret =1 ; + goto out; + }list_for_each_entry (shrinker, &shrinker_list, list) {unsigned long long delta;
@@ -282,6 +285,8 @@ unsigned long shrink_slab(unsigned long scanned, gfp_t gfp_mask, shrinker->nr += total_scan; }up_read (&shrinker_rwsem); +out: +cond_resched ();return ret; }
Re: [PATCH] mm: vmscan: Correctly check if reclaimer should schedule during shrink_slab
Quoted 28 lines · Mel Gorman
It has been reported on some laptops that kswapd is consuming large amounts of CPU and not being scheduled when SLUB is enabled during large amounts of file copying. It is expected that this is due to kswapd missing every
cond_resched()point because;
shrink_page_list()callscond_resched()if inactive pages were isolated
which in turn may not happen ifall_unreclaimableis set inshrink_zones(). If for whatver reason,all_unreclaimableis set on all zones, we can miss callingcond_resched().
balance_pgdat()only callscond_reschedif the zones are not
balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that it was reclaiming for order-0 tokswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-entersbalance_pgdat()without ever callingcond_resched().
shrink_slabonly callscond_resched()if we are reclaiming slab
pages. If there are a large number of direct reclaimers, theshrinker_rwsemcan be contended and prevent kswapd callingcond_resched().This patch modifies the
shrink_slab()case. If the semaphore is contended, the caller will still checkcond_resched(). After each successful call into a shrinker, the check forcond_resched()is still necessary in case one shrinker call is particularly slow.
Quoted 7 lines
This patch replaces mm-vmscan-if-kswapd-has-been-running-too-long-allow-it-to-sleep.patch in -mm.
[[email protected]: Preserve call to
cond_reschedafter each call into shrinker]
From: Minchan Kim<[email protected]>Signed-off-by Mel Gorman<[email protected]>
Looks good to me.
Re: [PATCH] mm: vmscan: Correctly check if reclaimer should schedule during shrink_slab
In reply to Mel GormanQuoted 28 lines · Mel Gorman
It has been reported on some laptops that kswapd is consuming large amounts of CPU and not being scheduled when SLUB is enabled during large amounts of file copying. It is expected that this is due to kswapd missing every
cond_resched()point because;
shrink_page_list()callscond_resched()if inactive pages were isolated which in turn may not happen ifall_unreclaimableis set inshrink_zones(). If for whatver reason,all_unreclaimableis set on all zones, we can miss callingcond_resched().
balance_pgdat()only callscond_reschedif the zones are not balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that it was reclaiming for order-0 tokswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-entersbalance_pgdat()without ever callingcond_resched().
shrink_slabonly callscond_resched()if we are reclaiming slab pages. If there are a large number of direct reclaimers, theshrinker_rwsemcan be contended and prevent kswapd callingcond_resched().This patch modifies the
shrink_slab()case. If the semaphore is contended, the caller will still checkcond_resched(). After each successful call into a shrinker, the check forcond_resched()is still necessary in case one shrinker call is particularly slow.
Quoted 6 lines
This patch replaces mm-vmscan-if-kswapd-has-been-running-too-long-allow-it-to-sleep.patch in -mm.
[[email protected]: Preserve call to
cond_reschedafter each call into shrinker]
From: Minchan Kim <[email protected]>
Quoted 1 line
Signed-off-by Mel Gorman <[email protected]>
Signature · 6 lines
Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mm: vmscan: Correctly check if reclaimer should schedule during shrink_slab
In reply to Mel GormanHi Colin.
Sorry for bothering you. :(
I hope this test is last.
We(Mel, KOSAKI and me) finalized opinion.
Could you test below patch with patch[1/4] of Mel's series(ie, !pgdat_balanced of sleeping_prematurely)?
If it is successful, we will try to merge this version instead of various cond_resched sprinkling version.
Quoted 67 lines · Mel Gorman
It has been reported on some laptops that kswapd is consuming large amounts of CPU and not being scheduled when SLUB is enabled during large amounts of file copying. It is expected that this is due to kswapd missing every
cond_resched()point because;
shrink_page_list()callscond_resched()if inactive pages were isolated which in turn may not happen ifall_unreclaimableis set inshrink_zones(). If for whatver reason,all_unreclaimableis set on all zones, we can miss callingcond_resched().
balance_pgdat()only callscond_reschedif the zones are not balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that it was reclaiming for order-0 tokswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-entersbalance_pgdat()without ever callingcond_resched().
shrink_slabonly callscond_resched()if we are reclaiming slab pages. If there are a large number of direct reclaimers, theshrinker_rwsemcan be contended and prevent kswapd callingcond_resched().This patch modifies the
shrink_slab()case. If the semaphore is contended, the caller will still checkcond_resched(). After each successful call into a shrinker, the check forcond_resched()is still necessary in case one shrinker call is particularly slow.This patch replaces mm-vmscan-if-kswapd-has-been-running-too-long-allow-it-to-sleep.patch in -mm.
[[email protected]: Preserve call to
cond_reschedafter each call into shrinker]
From: Minchan Kim <[email protected]>Signed-off-by Mel Gorman <[email protected]>---
mm/vmscan.c | 9 +++++++--1 files changed, 7 insertions(+), 2 deletions(-)mm/vmscan.c +7
-2@@ -230,8 +230,11 @@ unsigned long shrink_slab(unsigned long scanned, gfp_t gfp_mask, if (scanned == 0) scanned = SWAP_CLUSTER_MAX; - if (!down_read_trylock(&shrinker_rwsem)) - return 1; /* Assume we'll be able to shrink next time */ + if (!down_read_trylock(&shrinker_rwsem)) { + /* Assume we'll be able to shrink next time */ + ret = 1; + goto out; + } list_for_each_entry(shrinker, &shrinker_list, list) { unsigned long long delta;@@ -282,6 +285,8 @@ unsigned long shrink_slab(unsigned long scanned, gfp_t gfp_mask, shrinker->nr += total_scan; } up_read(&shrinker_rwsem); +out: + cond_resched(); return ret; }
Signature · 6 lines
Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mm: vmscan: Correctly check if reclaimer should schedule during shrink_slab
Quoted 3 lines · Minchan Kim
Hi Colin.
Sorry for bothering you. :(
No problem at all, I've very happy to re-test.
Quoted 8 lines
I hope this test is last.
We(Mel, KOSAKI and me)finalized opinion.Could you test below patch with patch[1/4] of Mel's
series(ie, !pgdat_balanced of sleeping_prematurely)?
If it is successful, we will try to merge this version instead of variouscond_reschedsprinkling version.
tested with the patch below + patch[1/4] of Mel's series. 300 cycles, 2.5 hrs of soak testing: works OK.
Colin
Quoted 73 lines · Mel Gorman
Quoted 67 lines
It has been reported on some laptops that kswapd is consuming large amounts of CPU and not being scheduled when SLUB is enabled during large amounts of file copying. It is expected that this is due to kswapd missing every
cond_resched()point because;
shrink_page_list()callscond_resched()if inactive pages were isolated
which in turn may not happen ifall_unreclaimableis set inshrink_zones(). If for whatver reason,all_unreclaimableis set on all zones, we can miss callingcond_resched().
balance_pgdat()only callscond_reschedif the zones are not
balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that it was reclaiming for order-0 tokswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-entersbalance_pgdat()without ever callingcond_resched().
shrink_slabonly callscond_resched()if we are reclaiming slab
pages. If there are a large number of direct reclaimers, theshrinker_rwsemcan be contended and prevent kswapd callingcond_resched().This patch modifies the
shrink_slab()case. If the semaphore is contended, the caller will still checkcond_resched(). After each successful call into a shrinker, the check forcond_resched()is still necessary in case one shrinker call is particularly slow.This patch replaces mm-vmscan-if-kswapd-has-been-running-too-long-allow-it-to-sleep.patch in -mm.
[[email protected]: Preserve call to
cond_reschedafter each call into shrinker]
From: Minchan Kim <[email protected]>Signed-off-by Mel Gorman <[email protected]>--- mm/vmscan.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)mm/vmscan.c +7
-2@@ -230,8 +230,11 @@ unsigned long shrink_slab(unsigned long scanned, gfp_t gfp_mask,if (scanned ==0 ) scanned =SWAP_CLUSTER_MAX ; -if (!down_read_trylock (&shrinker_rwsem)) -return 1 ;/* Assume we'll be able to shrink next time */ +if (!down_read_trylock (&shrinker_rwsem)) { +/* Assume we'll be able to shrink next time */ + ret =1 ; + goto out; + }list_for_each_entry (shrinker, &shrinker_list, list) {unsigned long long delta;@@ -282,6 +285,8 @@ unsigned long shrink_slab(unsigned long scanned, gfp_t gfp_mask, shrinker->nr += total_scan; }up_read (&shrinker_rwsem); +out: +cond_resched ();return ret; }
Re: [PATCH] mm: vmscan: Correctly check if reclaimer should schedule during shrink_slab
Quoted 16 lines · Colin Ian King
Quoted 3 lines · Minchan Kim
Hi Colin.
Sorry for bothering you. :(
No problem at all, I've very happy to re-test.
Quoted 8 lines
I hope this test is last.
We(Mel, KOSAKI and me)finalized opinion.Could you test below patch with patch[1/4] of Mel's
series(ie, !pgdat_balanced of sleeping_prematurely)?
If it is successful, we will try to merge this version instead of variouscond_reschedsprinkling version.
Quoted 4 lines
tested with the patch below + patch[1/4] of Mel's series. 300 cycles, 2.5 hrs of soak testing: works OK.
Colin
Thanks, Colin.
We are approaching the conclusion for your help. :)
Mel, KOSAKI.
I will ask test to Andrew Lutomirski.
If he doesn't have a problem, let's go, then.
Signature · 6 lines
Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
In reply to Colin Ian KingHello Colin,
Quoted 174 lines · Colin Ian King
Quoted 173 lines · Mel Gorman
Quoted 126 lines · Minchan Kim
Quoted 91 lines · Mel Gorman
Quoted 75 lines · Minchan Kim
Quoted 66 lines · Mel Gorman
Quoted 65 lines · Minchan Kim
Quoted 40 lines · James Bottomley
Quoted 18 lines · KOSAKI Motohiro
Quoted 17 lines · Mel Gorman
Under constant allocation pressure, kswapd can be in the situation where
sleeping_prematurely()will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.Signed-off-by Mel Gorman<[email protected]>---
mm/vmscan.c| 4 ++++1 files changed, 4 insertions(+), 0 deletions(-)mm/vmscan.c +0
-0@@ -2251,6 +2251,10 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, unsigned long balanced = 0; bool all_zones_ok = true;Quoted 8 lines
mm/vmscan.c static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, · line 2257
Quoted 4 lines
mm/vmscan.c +4
-0+ /* If kswapd has been running too long, just sleep */ + if (need_resched()) + return false; +Hmm... I don't like this patch so much. because this code does
- don't sleep if kswapd got context switch at
shrink_inactive_listThis isn't entirely true: need_resched() will be false, so we'll follow the normal path for determining whether to sleep or not, in effect leaving the current behaviour unchanged.
Quoted 1 line
- sleep if kswapd didn't
This also isn't entirely true: whether
need_resched()is true at this point depends on a whole lot more that whether we did a context switch inshrink_inactive. It mostly depends on how long we've been running without giving up the CPU. Generally that will mean we've been round the shrinker loop hundreds to thousands of times without sleeping.Quoted 5 lines
Quoted 1 line
It seems to be semi random behavior.
Well, we have to do something. Chris Mason first suspected the hang was a kswapd rescheduling problem a while ago. We tried putting
cond_rescheds()in several places in the vmscan code, but to no avail.Is it a result of test with patch of
Hannes(ie, !pgdat_balanced)?If it isn't, it would be nop regardless of putting
cond_reshedatvmscan.c. Because, although we complete zone balancing, kswapd doesn't sleep aspgdat_balancereturns wrong result. And at last VM callsbalance_pgdat. In this case,balance_pgdatreturns without any work as kswap couldn't find zones which have not enough free pages and goto out. kswapd could repeat this work infinitely. So you don't have a chance to callcond_resched.But if your test was with Hanne's patch, I am very curious how come kswapd consumes CPU a lot.
Quoted 3 lines
The
need_resched()insleeping_prematurely()seems to be about the best option. The other option might be just to put acond_resched()inkswapd_try_to_sleep(), but that will really have about the same effect.Quoted 7 lines
Quoted 3 lines
I don't oppose it but before that, I think we have to know why kswapd consumes CPU a lot although we applied Hannes' patch.
Because it's still possible for processes to allocate pages at the same rate kswapd is freeing them leading to a situation where kswapd does not consider the zone balanced for prolonged periods of time.
Quoted 7 lines
We have
cond_reschedinshrink_page_list,shrink_slabandbalance_pgdat. So I think kswapd can be scheduled out although it's scheduled in after a short time as task scheduled also need page reclaim. Although all task in system need reclaim, kswapd cpu 99% consumption is a natural result, I think.
Do I miss something?Lets see;
shrink_page_list()only applies if inactive pages were isolated which in turn may not happen ifall_unreclaimableis set inshrink_zones(). If for whatver reason,all_unreclaimableis set on all zones, we can miss callingcond_resched().Quoted 8 lines
shrink_slabonly applies if we are reclaiming slab pages. If the first shrinker returns -1, we do not callcond_resched(). If that first shrinker is dcache and__GFP_FSis not set, direct reclaimers will not shrink at all. However, if there are enough of them running or if one of the other shrinkers is running for a very long time, kswapd could be starved acquiring theshrinker_rwsemand never reaching thecond_resched().Don't we have to move
cond_resched?mm/vmscan.c +4
-3@@ -231,8 +231,10 @@ unsigned long shrink_slab(struct shrink_control *shrink, if (scanned == 0) scanned = SWAP_CLUSTER_MAX; - if (!down_read_trylock(&shrinker_rwsem)) - return 1; /* Assume we'll be able to shrink next time */ + if (!down_read_trylock(&shrinker_rwsem)) { + ret = 1; + goto out; /* Assume we'll be able to shrink next time */ + } list_for_each_entry(shrinker, &shrinker_list, list) { unsigned long long delta;@@ -280,12 +282,14 @@ unsigned long shrink_slab(struct shrink_control *shrink, count_vm_events(SLABS_SCANNED, this_scan); total_scan -= this_scan; - cond_resched(); }mm/vmscan.c unsigned long shrink_slab(struct shrink_control *shrink, · line 295
Quoted 10 lines
mm/vmscan.c +3
-0shrinker->nr += total_scan; + cond_resched(); } up_read(&shrinker_rwsem); +out: + cond_resched(); return ret; }This makes some sense for the exit path but if one or more of the shrinkers takes a very long time without sleeping (extremely long list searches for example) then kswapd will not call
cond_resched()between shrinkers and still consume a lot of CPU.Quoted 18 lines
Quoted 8 lines
balance_pgdat()only callscond_reschedif the zones are not balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that was reclaiming for order-0 tokswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-entersbalance_pgdat()without ever have calledcond_resched().If kswapd reclaims order-o followed by high order, it would have a chance to call
cond_reschedinshrink_page_list. But if all zones areall_unreclaimableis set,balance_pgdatcould return any work. Okay. It does make sense.
By your scenario, someone wakes up kswapd with higher order, again. So re-entersbalance_pgdatwithout ever have calledcond_resched. But if someone wakes up higher order again, we can't have a chance to callkswapd_try_to_sleep. So your patch effect would be nop, too.Quoted 2 lines
It would be better to put
cond_reschedafterbalance_pgdat?Which will leave kswapd runnable instead of going to sleep but guarantees a scheduling point. Lets see if the problem is that
cond_reschedis being missed although if this was the case then patch 4 would truly be a no-op but Colin has already reported that patch 1 on its own didn't fix his problem. If the problem is sandybridge-specific where kswapd remains runnable and consuming large amounts of CPU in turbo mode then we know that there are othercond_resched()decisions that will need to be revisited.
Quoted 4 lines
Quoted 2 lines
Colin or James, would you be willing to test with patch 1 from this series and Minchan's patch below? Thanks.
This works OK fine. Ran 250 test cycles for about 2 hours.
Thanks for the testing!.
I would like to know exact patch for you to apply. My modification of inserting cond_resched is two.
1) shrink_slab function
2) kswapd right after balance_pgdat.
1) or 2) ?
Or
Both?
Thanks
Signature · 9 lines
Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 187 lines · Minchan Kim
Hello Colin,
Quoted 174 lines · Colin Ian King
Quoted 173 lines · Mel Gorman
Quoted 126 lines · Minchan Kim
Quoted 91 lines · Mel Gorman
Quoted 75 lines · Minchan Kim
Quoted 66 lines · Mel Gorman
Quoted 65 lines · Minchan Kim
Quoted 40 lines · James Bottomley
Quoted 18 lines · KOSAKI Motohiro
Quoted 17 lines · Mel Gorman
Under constant allocation pressure, kswapd can be in the situation where
sleeping_prematurely()will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.Signed-off-by Mel Gorman<[email protected]>--- mm/vmscan.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)mm/vmscan.c +0
-0@@ -2251,6 +2251,10 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining,unsigned long balanced =0 ;bool all_zones_ok = true;Quoted 8 lines
mm/vmscan.c static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, · line 2257
Quoted 4 lines
mm/vmscan.c +4
-0+/* If kswapd has been running too long, just sleep */ +if (need_resched ()) +return false; +Hmm... I don't like this patch so much. because this code does
- don't sleep if kswapd got context switch at
shrink_inactive_listThis isn't entirely true:
need_resched()will be false, so we'll follow the normal path for determining whether to sleep or not, in effect leaving the current behaviour unchanged.Quoted 1 line
- sleep if kswapd didn't
This also isn't entirely true: whether
need_resched()is true at this point depends on a whole lot more that whether we did a context switch inshrink_inactive. It mostly depends on how long we've been running without giving up the CPU. Generally that will mean we've been round the shrinker loop hundreds to thousands of times without sleeping.Quoted 5 lines
Quoted 1 line
It seems to be semi random behavior.
Well, we have to do something. Chris Mason first suspected the hang was a kswapd rescheduling problem a while ago. We tried putting
cond_rescheds()in several places in the vmscan code, but to no avail.Is it a result of test with patch of
Hannes(ie, !pgdat_balanced)?If it isn't, it would be nop regardless of putting
cond_reshedatvmscan.c. Because, although we complete zone balancing, kswapd doesn't sleep aspgdat_balancereturns wrong result. And at last VM callsbalance_pgdat. In this case,balance_pgdatreturns without any work as kswap couldn't find zones which have not enough free pages and goto out. kswapd could repeat this work infinitely. So you don't have a chance to callcond_resched.But if your test was with Hanne's patch, I am very curious how come kswapd consumes CPU a lot.
Quoted 3 lines
The
need_resched()insleeping_prematurely()seems to be about the best option. The other option might be just to put acond_resched()inkswapd_try_to_sleep(), but that will really have about the same effect.Quoted 7 lines
Quoted 3 lines
I don't oppose it but before that, I think we have to know why kswapd consumes CPU a lot although we applied Hannes' patch.
Because it's still possible for processes to allocate pages at the same rate kswapd is freeing them leading to a situation where kswapd does not consider the zone balanced for prolonged periods of time.
Quoted 7 lines
We have
cond_reschedinshrink_page_list,shrink_slabandbalance_pgdat. So I think kswapd can be scheduled out although it's scheduled in after a short time as task scheduled also need page reclaim. Although all task in system need reclaim, kswapd cpu 99% consumption is a natural result, I think.
Do I miss something?Lets see;
shrink_page_list()only applies if inactive pages were isolated
which in turn may not happen ifall_unreclaimableis set inshrink_zones(). If for whatver reason,all_unreclaimableis set on all zones, we can miss callingcond_resched().Quoted 8 lines
shrink_slabonly applies if we are reclaiming slab pages. If the first
shrinker returns -1, we do not callcond_resched(). If that first shrinker is dcache and__GFP_FSis not set, direct reclaimers will not shrink at all. However, if there are enough of them running or if one of the other shrinkers is running for a very long time, kswapd could be starved acquiring theshrinker_rwsemand never reaching thecond_resched().Don't we have to move
cond_resched?mm/vmscan.c +4
-3@@ -231,8 +231,10 @@ unsigned long shrink_slab(struct shrink_control *shrink,if (scanned ==0 ) scanned =SWAP_CLUSTER_MAX ; -if (!down_read_trylock (&shrinker_rwsem)) -return 1 ;/* Assume we'll be able to shrink next time */ +if (!down_read_trylock (&shrinker_rwsem)) { + ret =1 ; + goto out;/* Assume we'll be able to shrink next time */ + }list_for_each_entry (shrinker, &shrinker_list, list) {unsigned long long delta;@@ -280,12 +282,14 @@ unsigned long shrink_slab(struct shrink_control *shrink,count_vm_events (SLABS_SCANNED , this_scan); total_scan -= this_scan; -cond_resched (); }mm/vmscan.c unsigned long shrink_slab(struct shrink_control *shrink, · line 295
Quoted 10 lines
mm/vmscan.c +3
-0shrinker->nr += total_scan; +cond_resched (); }up_read (&shrinker_rwsem); +out: +cond_resched ();return ret; }This makes some sense for the exit path but if one or more of the shrinkers takes a very long time without sleeping (extremely long list searches for example) then kswapd will not call
cond_resched()between shrinkers and still consume a lot of CPU.Quoted 18 lines
Quoted 8 lines
balance_pgdat()only callscond_reschedif the zones are not
balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that was reclaiming for order-0 tokswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-entersbalance_pgdat()without ever have calledcond_resched().If kswapd reclaims order-o followed by high order, it would have a chance to call
cond_reschedinshrink_page_list. But if all zones areall_unreclaimableis set,balance_pgdatcould return any work. Okay. It does make sense.
By your scenario, someone wakes up kswapd with higher order, again. So re-entersbalance_pgdatwithout ever have calledcond_resched. But if someone wakes up higher order again, we can't have a chance to callkswapd_try_to_sleep. So your patch effect would be nop, too.Quoted 2 lines
It would be better to put
cond_reschedafterbalance_pgdat?Which will leave kswapd runnable instead of going to sleep but guarantees a scheduling point. Lets see if the problem is that
cond_reschedis being missed although if this was the case then patch 4 would truly be a no-op but Colin has already reported that patch 1 on its own didn't fix his problem. If the problem is sandybridge-specific where kswapd remains runnable and consuming large amounts of CPU in turbo mode then we know that there are othercond_resched()decisions that will need to be revisited.Quoted 4 lines
Quoted 2 lines
Colin or James, would you be willing to test with patch 1 from this series and Minchan's patch below? Thanks.
This works OK fine. Ran 250 test cycles for about 2 hours.
Thanks for the testing!.
I would like to know exact patch for you to apply. My modification of insertingcond_reschedis two.
Quoted 7 lines
1)
shrink_slabfunction
2) kswapd right afterbalance_pgdat.1) or 2) ?
Or
Both?
I just followed Mel's request, so, patch 1 from the series and *just* the following:
mm/vmscan.c static int kswapd(void *p) · line 2754
Quoted 10 lines
Colin or James, would you be willing to test with patch 1 from this series and Minchan's patch below? Thanks.
mm/vmscan.c +0
-0@@ -2753,6 +2753,7 @@ static int kswapd(void *p)if (!ret) { trace_mm_vmscan_kswapd_wake(pgdat->node_id,
order);
mm/vmscan.c static int kswapd(void *p) · line 2755
Quoted 1 line
mm/vmscan.c +0
-0order = balance_pgdat(pgdat, order,
&classzone_idx);
mm/vmscan.c static int kswapd(void *p) · line 2759
Quoted 4 lines
mm/vmscan.c +1
-0+cond_resched (); } }return 0 ;
Colin
Quoted 1 line
Thanks
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
In reply to Mel GormanQuoted 72 lines · Mel Gorman
Quoted 25 lines · Minchan Kim
Don't we have to move
cond_resched?mm/vmscan.c +4
-3@@ -231,8 +231,10 @@ unsigned long shrink_slab(struct shrink_control *shrink,if (scanned ==0 ) scanned =SWAP_CLUSTER_MAX ; -if (!down_read_trylock (&shrinker_rwsem)) -return 1 ;/* Assume we'll be able to shrink next time */ +if (!down_read_trylock (&shrinker_rwsem)) { + ret =1 ; + goto out;/* Assume we'll be able to shrink next time */ + }list_for_each_entry (shrinker, &shrinker_list, list) {unsigned long long delta;@@ -280,12 +282,14 @@ unsigned long shrink_slab(struct shrink_control *shrink,count_vm_events (SLABS_SCANNED , this_scan); total_scan -= this_scan; -cond_resched (); }mm/vmscan.c unsigned long shrink_slab(struct shrink_control *shrink, · line 295
Quoted 10 lines
mm/vmscan.c +3
-0shrinker->nr += total_scan; +cond_resched (); }up_read (&shrinker_rwsem); +out: +cond_resched ();return ret; }This makes some sense for the exit path but if one or more of the shrinkers takes a very long time without sleeping (extremely long list searches for example) then kswapd will not call
cond_resched()between shrinkers and still consume a lot of CPU.Quoted 18 lines
Quoted 8 lines
balance_pgdat()only callscond_reschedif the zones are not
balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that was reclaiming for order-0 tokswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-entersbalance_pgdat()without ever have calledcond_resched().If kswapd reclaims order-o followed by high order, it would have a chance to call
cond_reschedinshrink_page_list. But if all zones areall_unreclaimableis set,balance_pgdatcould return any work. Okay. It does make sense.
By your scenario, someone wakes up kswapd with higher order, again. So re-entersbalance_pgdatwithout ever have calledcond_resched. But if someone wakes up higher order again, we can't have a chance to callkswapd_try_to_sleep. So your patch effect would be nop, too.Quoted 2 lines
It would be better to put
cond_reschedafterbalance_pgdat?Which will leave kswapd runnable instead of going to sleep but guarantees a scheduling point. Lets see if the problem is that
cond_reschedis being missed although if this was the case then patch 4 would truly be a no-op but Colin has already reported that patch 1 on its own didn't fix his problem. If the problem is sandybridge-specific where kswapd remains runnable and consuming large amounts of CPU in turbo mode then we know that there are othercond_resched()decisions that will need to be revisited.
Quoted 2 lines
Colin or James, would you be willing to test with patch 1 from this series and Minchan's patch below? Thanks.
Yes, but unfortunately I'm on the road at the moment. I won't get back to the laptop showing the problem until late on Tuesday (24th). If it works for Colin, I'd assume it's OK.
James
Quoted 29 lines
Quoted 28 lines
mm/vmscan.c +1
-0@@ -2753,6 +2753,7 @@ static int kswapd(void *p)if (!ret) {trace_mm_vmscan_kswapd_wake (pgdat->node_id, order); order =balance_pgdat (pgdat, order, &classzone_idx); +cond_resched (); } }return 0 ;Quoted 10 lines
While it appears unlikely, there are bad conditions which can result in
cond_resched()being avoided.--
Mel Gorman
SUSE LabsSignature · 2 lines
Kind regards, Minchan Kim
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
In reply to Minchan KimQuoted 15 lines
It would be better to put
cond_reschedafterbalance_pgdat?mm/vmscan.c +1
-0@@ -2753,6 +2753,7 @@ static int kswapd(void *p)if (!ret) {trace_mm_vmscan_kswapd_wake (pgdat->node_id, order); order =balance_pgdat (pgdat, order,&classzone_idx); +cond_resched (); } }return 0 ;
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
Quoted 1 line
While it appears unlikely, there are bad conditions which can result
in
cond_resched()being avoided.
Every reclaim priority decreasing or every shrink_zone() calling makes more fine grained preemption. I think.
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 17 lines · KOSAKI Motohiro
Quoted 17 lines
It would be better to put
cond_reschedafterbalance_pgdat?mm/vmscan.c +1
-0@@ -2753,6 +2753,7 @@ static int kswapd(void *p) if (!ret) { trace_mm_vmscan_kswapd_wake(pgdat->node_id, order); order = balance_pgdat(pgdat, order,&classzone_idx); + cond_resched(); }}
return 0;
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line
While it appears unlikely, there are bad conditions which can result
in
cond_resched()being avoided.Every reclaim priority decreasing or every
shrink_zone()calling makes more fine grained preemption. I think.
It could be.
But in direct reclaim case, I have a concern about losing pages reclaimed to other tasks by preemption.
Hmm,, anyway, we also needs test.
Hmm,, how long should we bother them(Colins and James)? First of all, Let's fix one just between us and ask test to them and send the last patch to akpm.
1. shrink_slab
2. right after balance_pgdat
3. shrink_zone
4. reclaim priority decreasing routine.
Now, I vote 1) and 2).
Mel, KOSAKI?
Signature · 6 lines
Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 10 lines
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line
While it appears unlikely, there are bad conditions which can result
in
cond_resched()being avoided.Every reclaim priority decreasing or every
shrink_zone()calling makes more fine grained preemption. I think.It could be.
But in direct reclaim case, I have a concern about losing pages reclaimed to other tasks by preemption.
Nope, I proposed to add cond_resched() into balance_pgdat().
Quoted 5 lines
Hmm,, anyway, we also needs test.
Hmm,, how long should we bother them(Colins and James)? First of all, Let's fix one just between us and ask test to them and send the last patch to akpm.
Quoted 8 lines
1.
shrink_slab
2. right afterbalance_pgdat
3.shrink_zone
4. reclaim priority decreasing routine.Now, I vote 1) and 2).
Mel, KOSAKI?
I think following patch makes enough preemption.
Thanks.
From e7d88be1916184ea7c93a6f2746b15c7a32d1973 Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <[email protected]>
Date: Wed, 18 May 2011 15:00:39 +0900
Subject: [PATCH] vmscan: balance_pgdat() call cond_resched() unconditionally
Under constant allocation pressure, kswapd can be in the situation where sleeping_prematurely() will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.
--- mm/vmscan.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
mm/vmscan.c +1 -2
@@ -2449,6 +2449,7 @@ loop_again: sc.nr_reclaimed += reclaim_state->reclaimed_slab; total_scanned += sc.nr_scanned; +cond_resched ();if (zone->all_unreclaimable)continue ; if (nr_slab == 0 &&
@@ -2518,8 +2519,6 @@ out:* for the node to be balanced */ if (!(all_zones_ok || (order &&pgdat_balanced (pgdat, balanced, *classzone_idx)))) { -cond_resched (); -try_to_freeze ();/*
Signature · 11 lines
1.7.3.1 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
In reply to Minchan KimQuoted 35 lines · Minchan Kim
Quoted 17 lines · KOSAKI Motohiro
Quoted 17 lines
It would be better to put
cond_reschedafterbalance_pgdat?mm/vmscan.c +1
-0@@ -2753,6 +2753,7 @@ static int kswapd(void *p) if (!ret) { trace_mm_vmscan_kswapd_wake(pgdat->node_id, order); order = balance_pgdat(pgdat, order,&classzone_idx); + cond_resched(); }}
return 0;Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line
While it appears unlikely, there are bad conditions which can result
in
cond_resched()being avoided.Every reclaim priority decreasing or every
shrink_zone()calling makes more fine grained preemption. I think.It could be.
But in direct reclaim case, I have a concern about losing pages reclaimed to other tasks by preemption.Hmm,, anyway, we also needs test.
Hmm,, how long should we bother them(Colins and James)? First of all, Let's fix one just between us and ask test to them and send the last patch to akpm.
Quoted 7 lines
1.
shrink_slab
2. right afterbalance_pgdat
3.shrink_zone
4. reclaim priority decreasing routine.Now, I vote 1) and 2).
I've already submitted a pair of patches for option 1. I don't think option 2 gains us anything. I think it's more likely we should worry about all_unreclaimable being set when shrink_slab is returning 0 and we are encountering so many dirty pages that pages_scanned is high enough.
Signature · 6 lines
Mel Gorman SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 41 lines · Mel Gorman
Quoted 40 lines · Minchan Kim
Quoted 17 lines · KOSAKI Motohiro
Quoted 17 lines
It would be better to put
cond_reschedafterbalance_pgdat?mm/vmscan.c +1
-0@@ -2753,6 +2753,7 @@ static int kswapd(void *p) if (!ret) { trace_mm_vmscan_kswapd_wake(pgdat->node_id, order); order = balance_pgdat(pgdat, order,&classzone_idx); + cond_resched(); }}
return 0;Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line
While it appears unlikely, there are bad conditions which can result
in
cond_resched()being avoided.Every reclaim priority decreasing or every
shrink_zone()calling makes more fine grained preemption. I think.It could be.
But in direct reclaim case, I have a concern about losing pages reclaimed to other tasks by preemption.Hmm,, anyway, we also needs test.
Hmm,, how long should we bother them(Colins and James)? First of all, Let's fix one just between us and ask test to them and send the last patch to akpm.1.
shrink_slab
2. right afterbalance_pgdat
3.shrink_zone
4. reclaim priority decreasing routine.
Quoted 7 lines
Quoted 2 lines
Now, I vote 1) and 2).
I've already submitted a pair of patches for option 1. I don't think option 2 gains us anything. I think it's more likely we should worry about
all_unreclaimablebeing set whenshrink_slabis returning 0 and we are encountering so many dirty pages thatpages_scannedis high enough.
Okay.
Colin reported he had no problem with patch 1 in this series and mine(ie, just cond_resched right after balance_pgdat call without no patch of shrink_slab).
If Colin's test is successful, I don't insist on mine.
(I don't want to drag on for days :( )
If KOSAKI agree, let's ask the test to Colin and confirm our last test.
KOSAKI. Could you post a your opinion?
Signature · 9 lines
Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 11 lines
Quoted 4 lines
I've already submitted a pair of patches for option 1. I don't think option 2 gains us anything. I think it's more likely we should worry about
all_unreclaimablebeing set whenshrink_slabis returning 0 and we are encountering so many dirty pages thatpages_scannedis high enough.Okay.
Colin reported he had no problem with patch 1 in this series and
mine(ie, just cond_resched right after balance_pgdat call without no patch of shrink_slab).
Quoted 5 lines
If Colin's test is successful, I don't insist on mine.
(I don't want to drag on for days :( )
If KOSAKI agree, let's ask the test to Colin and confirm our last test.KOSAKI. Could you post a your opinion?
Yeah.
I also don't have any motivation to ignore Colin's test result.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
In reply to Mel GormanQuoted 7 lines
Lets see;
shrink_page_list()only applies if inactive pages were isolated
which in turn may not happen ifall_unreclaimableis set inshrink_zones(). If for whatver reason,all_unreclaimableis set on all zones, we can miss callingcond_resched().
Quoted 8 lines
shrink_slabonly applies if we are reclaiming slab pages. If the first
shrinker returns -1, we do not callcond_resched(). If that first shrinker is dcache and__GFP_FSis not set, direct reclaimers will not shrink at all. However, if there are enough of them running or if one of the other shrinkers is running for a very long time, kswapd could be starved acquiring theshrinker_rwsemand never reaching thecond_resched().
OK.
Quoted 8 lines
balance_pgdat()only callscond_reschedif the zones are not
balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that was reclaiming for order-0 tokswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-entersbalance_pgdat()without ever have calledcond_resched().
Then, Shouldn't balance_pgdat() call cond_resched() unconditionally? The problem is NOT 100% cpu consumption. if kswapd will sleep, other processes need to reclaim old pages. The problem is, kswapd doesn't invoke context switch and other tasks hang-up.
Quoted 3 lines
While it appears unlikely, there are bad conditions which can result in
cond_resched()being avoided.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
Quoted 28 lines · KOSAKI Motohiro
Quoted 7 lines
Lets see;
shrink_page_list()only applies if inactive pages were isolated
which in turn may not happen ifall_unreclaimableis set inshrink_zones(). If for whatver reason,all_unreclaimableis set on all zones, we can miss callingcond_resched().Quoted 8 lines
shrink_slabonly applies if we are reclaiming slab pages. If the first
shrinker returns -1, we do not callcond_resched(). If that first shrinker is dcache and__GFP_FSis not set, direct reclaimers will not shrink at all. However, if there are enough of them running or if one of the other shrinkers is running for a very long time, kswapd could be starved acquiring theshrinker_rwsemand never reaching thecond_resched().OK.
Quoted 8 lines
balance_pgdat()only callscond_reschedif the zones are not
balanced. For a high-order allocation that is balanced, it checks order-0 again. During that window, order-0 might have become unbalanced so it loops again for order-0 and returns that was reclaiming for order-0 tokswapd(). It can then find that a caller has rewoken kswapd for a high-order and re-entersbalance_pgdat()without ever have calledcond_resched().
Quoted 5 lines
Then, Shouldn't
balance_pgdat()callcond_resched()unconditionally? The problem is NOT 100% cpu consumption. if kswapd will sleep, other processes need to reclaim old pages. The problem is, kswapd doesn't invoke context switch and other tasks hang-up.
Which the shrink_slab patch does (either version). What's the gain from sprinkling more cond_resched() around? If you think there is, submit another pair of patches (include patch 1 from this series) but I'm not seeing the advantage myself.
Quoted 6 lines
Quoted 4 lines
While it appears unlikely, there are bad conditions which can result in
cond_resched()being avoided.
Signature · 9 lines
Mel Gorman SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
In reply to KOSAKI MotohiroQuoted 25 lines · KOSAKI Motohiro
Quoted 14 lines · Mel Gorman
Under constant allocation pressure, kswapd can be in the situation where
sleeping_prematurely()will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.Signed-off-by Mel Gorman<[email protected]>--- mm/vmscan.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)mm/vmscan.c +0
-0mm/vmscan.c static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining, · line 2257
Quoted 7 lines
mm/vmscan.c +4
-0@@ -2251,6 +2251,10 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining,unsigned long balanced =0 ;bool all_zones_ok = true; +/* If kswapd has been running too long, just sleep */ +if (need_resched ()) +return false; +Hmm... I don't like this patch so much. because this code does
Quoted 5 lines
- don't sleep if kswapd got context switch at
shrink_inactive_list
- sleep if kswapd didn'tIt seems to be semi random behavior.
It's possible to keep kswapd awake simply by allocating fast enough that the watermarks are never balanced making kswapd appear to consume 100% of CPU. This check causes kswapd to sleep in this case. The processes doing the allocations will enter direct reclaim and probably stall while processes that are not allocating will get some CPU time.
Signature · 2 lines
Mel Gorman SUSE Labs
Re: [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep
In reply to Mel GormanQuoted 5 lines · Mel Gorman
Under constant allocation pressure, kswapd can be in the situation where
sleeping_prematurely()will always return true even if kswapd has been running a long time. Check if kswapd needs to be scheduled.Signed-off-by Mel Gorman<[email protected]>
Signature · 8 lines
All rights reversed -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 25 lines · Mel Gorman
Changelog since V1
o kswapd should sleep ifneed_resched
o Remove__GFP_REPEATfrom GFP flags when speculatively using high orders so direct/compaction exits earlier
o Remove__GFP_NORETRYfor correctness
o Correct logic insleeping_prematurely
o Leave SLUB using the defaultslub_max_orderThere are a few reports of people experiencing hangs when copying large amounts of data with kswapd using a large amount of CPU which appear to be due to recent reclaim changes.
SLUB using high orders is the trigger but not the root cause as SLUB has been using high orders for a while. The following four patches aim to fix the problems in reclaim while reducing the cost for SLUB using those high orders.
Patch 1 corrects logic introduced by commit [
1741c877: mm:
kswapd: keep kswapd awake for high-order allocations until a percentage of the node is balanced] to allow kswapd to go to sleep when balanced for high orders.Patch 2 prevents kswapd waking up in response to SLUBs speculative
use of high orders.
Quoted 7 lines
Patch 3 further reduces the cost by prevent SLUB entering direct
compaction or reclaim paths on the grounds that falling back to order-0 should be cheaper.Patch 4 notes that even when kswapd is failing to keep up with
allocation requests, it should still go to sleep when its quota has expired to prevent it spinning.
This all works fine for me ... three untar runs and no kswapd hangs or pegging the CPU at 99% ... in fact, kswapd rarely gets over 20%
This isn't as good as the kswapd sleeping_prematurely() throttling patch. For total CPU time on a three 90GB untar run, it's about 64s of CPU time with your patch rather than 6s, but that's vastly better than the 15 minutes of CPU time kswapd was taking even under PREEMPT.
James
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 30 lines · James Bottomley
Quoted 29 lines · Mel Gorman
Changelog since V1
o kswapd should sleep ifneed_resched
o Remove__GFP_REPEATfrom GFP flags when speculatively using high orders so direct/compaction exits earlier
o Remove__GFP_NORETRYfor correctness
o Correct logic insleeping_prematurely
o Leave SLUB using the defaultslub_max_orderThere are a few reports of people experiencing hangs when copying large amounts of data with kswapd using a large amount of CPU which appear to be due to recent reclaim changes.
SLUB using high orders is the trigger but not the root cause as SLUB has been using high orders for a while. The following four patches aim to fix the problems in reclaim while reducing the cost for SLUB using those high orders.
Patch 1 corrects logic introduced by commit [
1741c877: mm:
kswapd: keep kswapd awake for high-order allocations until a percentage of the node is balanced] to allow kswapd to go to sleep when balanced for high orders.Patch 2 prevents kswapd waking up in response to SLUBs speculative
use of high orders.Patch 3 further reduces the cost by prevent SLUB entering direct
compaction or reclaim paths on the grounds that falling back to order-0 should be cheaper.
Quoted 7 lines
Quoted 3 lines
Patch 4 notes that even when kswapd is failing to keep up with
allocation requests, it should still go to sleep when its quota has expired to prevent it spinning.This all works fine for me ... three untar runs and no kswapd hangs or pegging the CPU at 99% ... in fact, kswapd rarely gets over 20%
Good stuff, thanks.
Quoted 5 lines
This isn't as good as the kswapd
sleeping_prematurely()throttling patch. For total CPU time on a three 90GB untar run, it's about 64s of CPU time with your patch rather than 6s, but that's vastly better than the 15 minutes of CPU time kswapd was taking even under PREEMPT.
The throttling patch is unfortunately a bit hand-wavy based on number of times it's entered and time passed. It'll be even harder to debug problems related to this in the future particularly as it's using global information (a static) for kswapd (per-node which could be worse in the future depending on what memcg do).
However, as you are testing against stable, can you also apply this patch? [2876592f: mm: vmscan: stop reclaim/compaction earlier due to insufficient progress if !__GFP_REPEAT]. It makes a difference as to when reclaimers give up on high-orders and go to sleep.
Signature · 2 lines
Mel Gorman SUSE Labs
Quoted 9 lines · Mel Gorman
SLUB using high orders is the trigger but not the root cause as SLUB has been using high orders for a while. The following four patches aim to fix the problems in reclaim while reducing the cost for SLUB using those high orders.
Patch 1 corrects logic introduced by commit [
1741c877: mm:
kswapd: keep kswapd awake for high-order allocations until a percentage of the node is balanced] to allow kswapd to go to sleep when balanced for high orders.
The above looks good.
Quoted 2 lines
Patch 2 prevents kswapd waking up in response to SLUBs speculative
use of high orders.
Not sure if that is necessary since it seems that we triggered kswapd before? Why not continue to do it? Once kswapd has enough higher order pages kswapd should no longer be triggered right?
Quoted 3 lines
Patch 3 further reduces the cost by prevent SLUB entering direct
compaction or reclaim paths on the grounds that falling back to order-0 should be cheaper.
Its cheaper for reclaim path true but more expensive in terms of SLUBs management costs of the data and it also increases the memory wasted. A higher order means denser packing of objects less page management overhead. Fallback is not for free. Reasonable effort should be made to allocate the page order requested.
Quoted 3 lines
Patch 4 notes that even when kswapd is failing to keep up with
allocation requests, it should still go to sleep when its quota has expired to prevent it spinning.
Looks good too.
Overall, it looks like the compaction logic and the modifications to reclaim introduced recently with the intend to increase the amount of physically contiguous memory is not working as expected.
SLUBs chance of getting higher order pages should be *increasing* as a result of these changes. The above looks like the chances are decreasing now.
This is a matter of future concern. The metadata management overhead in the kernel is continually increasing since memory sizes keep growing and we typically manage memory in 4k chunks. Through large allocation sizes we can reduce that management overhead but we can only do this if we have an effective way of defragmenting memory to get longer contiguous chunks that can be managed to a single page struct.
Please make sure that compaction and related measures really work properly.
The patches suggest that the recent modifications are not improving the situation.
Quoted 7 lines · Christoph Lameter
Quoted 5 lines · Mel Gorman
SLUB using high orders is the trigger but not the root cause as SLUB has been using high orders for a while. The following four patches aim to fix the problems in reclaim while reducing the cost for SLUB using those high orders.
Quoted 7 lines
Quoted 4 lines
Patch 1 corrects logic introduced by commit [
1741c877: mm:
kswapd: keep kswapd awake for high-order allocations until a percentage of the node is balanced] to allow kswapd to go to sleep when balanced for high orders.The above looks good.
Ok.
Quoted 7 lines
Quoted 2 lines
Patch 2 prevents kswapd waking up in response to SLUBs speculative
use of high orders.Not sure if that is necessary since it seems that we triggered kswapd before? Why not continue to do it? Once kswapd has enough higher order pages kswapd should no longer be triggered right?
Because kswapd waking up isn't cheap and we are reclaiming pages just so SLUB may get high-order pages in the future. As it's for PAGE_ORDER_COSTLY_ORDER, we are not entering lumpy reclaim and just selecting a few random order-0 pages which may or may not help. There is very little control of how many pages are getting freed if kswapd is being woken frequently.
Quoted 6 lines
Quoted 3 lines
Patch 3 further reduces the cost by prevent SLUB entering direct
compaction or reclaim paths on the grounds that falling back to order-0 should be cheaper.Its cheaper for reclaim path true but more expensive in terms of SLUBs management costs of the data and it also increases the memory wasted.
Surely the reclaim cost exceeds SLUB management cost?
Quoted 3 lines
A
higher order means denser packing of objects less page management overhead. Fallback is not for free.
Neither is reclaiming a large bunch of pages. Worse, reclaiming pages so SLUB gets a high-order means it's likely to be stealing MIGRATE_MOVABLE blocks which eventually gives diminishing returns but may not be noticeable for weeks. From a fragmentation perspective, it's better if SLUB uses order-0 allocations when memory is low so that SLUB pages continue to get packed into as few MIGRATE_UNMOVABLE and MIGRATE_UNRECLAIMABLE blocks as possible.
Quoted 7 lines
Reasonable effort should be made to
allocate the page order requested.Quoted 3 lines
Patch 4 notes that even when kswapd is failing to keep up with
allocation requests, it should still go to sleep when its quota has expired to prevent it spinning.
Quoted 6 lines
Looks good too.
Overall, it looks like the compaction logic and the modifications to reclaim introduced recently with the intend to increase the amount of physically contiguous memory is not working as expected.
The reclaim and kswapd damage was unintended and this is my fault but reclaim/compaction still makes a lot more sense than lumpy reclaim. Testing showed it disrupted the system a lot less and allocated high-order pages faster with fewer pages reclaimed.
Quoted 4 lines
SLUBs chance of getting higher order pages should be *increasing* as a result of these changes. The above looks like the chances are decreasing now.
Patches 2 and 3 may mean that SLUB gets fewer high order pages when memory is low and it's depending on high-order pages to be naturally freed by SLUB as it recycles slabs of old objects. On the flip-side, fewer pages will be reclaimed. I'd expect the latter option is cheaper overall.
Quoted 9 lines
This is a matter of future concern. The metadata management overhead in the kernel is continually increasing since memory sizes keep growing and we typically manage memory in 4k chunks. Through large allocation sizes we can reduce that management overhead but we can only do this if we have an effective way of defragmenting memory to get longer contiguous chunks that can be managed to a single page struct.
Please make sure that compaction and related measures really work properly.
Local testing still shows them to be behaving as expected but then again, I haven't reproduced the simple problem reported by Chris and James despite using a few different laptops and two different low-end servers.
Quoted 3 lines
The patches suggest that the recent modifications are not improving the situation.
Signature · 2 lines
Mel Gorman SUSE Labs
Quoted 74 lines · Mel Gorman
Changelog since V1
o kswapd should sleep ifneed_resched
o Remove__GFP_REPEATfrom GFP flags when speculatively using high orders so direct/compaction exits earlier
o Remove__GFP_NORETRYfor correctness
o Correct logic insleeping_prematurely
o Leave SLUB using the defaultslub_max_orderThere are a few reports of people experiencing hangs when copying large amounts of data with kswapd using a large amount of CPU which appear to be due to recent reclaim changes.
SLUB using high orders is the trigger but not the root cause as SLUB has been using high orders for a while. The following four patches aim to fix the problems in reclaim while reducing the cost for SLUB using those high orders.
Patch 1 corrects logic introduced by commit [
1741c877: mm:
kswapd: keep kswapd awake for high-order allocations until a percentage of the node is balanced] to allow kswapd to go to sleep when balanced for high orders.Patch 2 prevents kswapd waking up in response to SLUBs speculative
use of high orders.Patch 3 further reduces the cost by prevent SLUB entering direct
compaction or reclaim paths on the grounds that falling back to order-0 should be cheaper.Patch 4 notes that even when kswapd is failing to keep up with
allocation requests, it should still go to sleep when its quota has expired to prevent it spinning.My own data on this is not great. I haven't really been able to reproduce the same problem locally.
The test case is simple. "download tar" wgets a large tar file and stores it locally. "unpack" is expanding it (15 times physical RAM in this case) and "delete source dirs" is the tarfile being deleted again. I also experimented with having the tar copied numerous times and into deeper directories to increase the size but the results were not particularly interesting so I left it as one tar.
In the background, applications are being launched to time to vaguely simulate activity on the desktop and to measure how long it takes applications to start.
Test server, 4 CPU threads,
x86_64, 2G of RAM, no PREEMPT, no COMPACTION, X running LARGE COPY AND UNTAR
vanilla fixprematurely kswapd-nowwake slub-noexstep kswapdsleep
download tar 95 ( 0.00%) 94 ( 1.06%) 94 ( 1.06%) 94 ( 1.06%) 94 ( 1.06%)
unpack tar 654 ( 0.00%) 649 ( 0.77%) 655 (-0.15%) 589 (11.04%) 598 ( 9.36%)
copy source files 0 ( 0.00%) 0 ( 0.00%) 0 ( 0.00%) 0 ( 0.00%) 0 ( 0.00%)
delete source dirs 327 ( 0.00%) 334 (-2.10%) 318 ( 2.83%) 325 ( 0.62%) 320 ( 2.19%)
MMTests Statistics: durationUser/Sys Time Running Test (seconds) 1139.7 1142.55 1149.78 1109.32 1113.26 Total Elapsed Time (seconds) 1341.59 1342.45 1324.90 1271.02 1247.35MMTests Statistics: application launch
evolution-wait30 mean 34.92 34.96 34.92 34.92 35.08 gnome-terminal-find mean 7.96 7.96 8.76 7.80 7.96 iceweasel-table mean 7.93 7.81 7.73 7.65 7.88evolution-wait30 stddev 0.96 1.22 1.27 1.20 1.15 gnome-terminal-find stddev 3.02 3.09 3.51 2.99 3.02 iceweasel-table stddev 1.05 0.90 1.09 1.11 1.11Having SLUB avoid expensive steps in reclaim improves performance by quite a bit with the overall test completing 1.5 minutes faster. Application launch times were not really affected but it's not something my test machine was suffering from in the first place so it's not really conclusive. The kswapd patches also did not appear to help but again, the test machine wasn't suffering that problem.
Quoted 2 lines
These patches are against 2.6.39-rc7. Again, testing would be appreciated.
These patches solve the problem for me. I've been soak testing the file copy test
for 3.5 hours with nearly 400 test cycles and observed no lockups at all
- rock solid. From my observations from the output from vmstat the system is behaving sanely.
Thanks for finding a solution - much appreciated!
Quoted 11 lines
Documentation/vm/slub.txt | 2 +- mm/page_alloc.c | 3 ++- mm/slub.c | 5 +++-- 3 files changed, 6 insertions(+), 4 deletions(-)mm/page_alloc.c | 3 ++- mm/slub.c | 3 ++- mm/vmscan.c | 6 +++++- 3 files changed, 9 insertions(+), 3 deletions(-)
Quoted 78 lines · Colin Ian King
Quoted 74 lines · Mel Gorman
Changelog since V1
o kswapd should sleep ifneed_resched
o Remove__GFP_REPEATfrom GFP flags when speculatively using high orders so direct/compaction exits earlier
o Remove__GFP_NORETRYfor correctness
o Correct logic insleeping_prematurely
o Leave SLUB using the defaultslub_max_orderThere are a few reports of people experiencing hangs when copying large amounts of data with kswapd using a large amount of CPU which appear to be due to recent reclaim changes.
SLUB using high orders is the trigger but not the root cause as SLUB has been using high orders for a while. The following four patches aim to fix the problems in reclaim while reducing the cost for SLUB using those high orders.
Patch 1 corrects logic introduced by commit [
1741c877: mm:
kswapd: keep kswapd awake for high-order allocations until a percentage of the node is balanced] to allow kswapd to go to sleep when balanced for high orders.Patch 2 prevents kswapd waking up in response to SLUBs speculative
use of high orders.Patch 3 further reduces the cost by prevent SLUB entering direct
compaction or reclaim paths on the grounds that falling back to order-0 should be cheaper.Patch 4 notes that even when kswapd is failing to keep up with
allocation requests, it should still go to sleep when its quota has expired to prevent it spinning.My own data on this is not great. I haven't really been able to reproduce the same problem locally.
The test case is simple. "download tar" wgets a large tar file and stores it locally. "unpack" is expanding it (15 times physical RAM in this case) and "delete source dirs" is the tarfile being deleted again. I also experimented with having the tar copied numerous times and into deeper directories to increase the size but the results were not particularly interesting so I left it as one tar.
In the background, applications are being launched to time to vaguely simulate activity on the desktop and to measure how long it takes applications to start.
Test server, 4 CPU threads,
x86_64, 2G of RAM, no PREEMPT, no COMPACTION, X running LARGE COPY AND UNTAR
vanilla fixprematurely kswapd-nowwake slub-noexstep kswapdsleep
download tar 95 ( 0.00%) 94 ( 1.06%) 94 ( 1.06%) 94 ( 1.06%) 94 ( 1.06%)
unpack tar 654 ( 0.00%) 649 ( 0.77%) 655 (-0.15%) 589 (11.04%) 598 ( 9.36%)
copy source files 0 ( 0.00%) 0 ( 0.00%) 0 ( 0.00%) 0 ( 0.00%) 0 ( 0.00%)
delete source dirs 327 ( 0.00%) 334 (-2.10%) 318 ( 2.83%) 325 ( 0.62%) 320 ( 2.19%)
MMTests Statistics: durationUser/Sys Time Running Test (seconds) 1139.7 1142.55 1149.78 1109.32 1113.26 Total Elapsed Time (seconds) 1341.59 1342.45 1324.90 1271.02 1247.35MMTests Statistics: application launch
evolution-wait30 mean 34.92 34.96 34.92 34.92 35.08 gnome-terminal-find mean 7.96 7.96 8.76 7.80 7.96 iceweasel-table mean 7.93 7.81 7.73 7.65 7.88evolution-wait30 stddev 0.96 1.22 1.27 1.20 1.15 gnome-terminal-find stddev 3.02 3.09 3.51 2.99 3.02 iceweasel-table stddev 1.05 0.90 1.09 1.11 1.11Having SLUB avoid expensive steps in reclaim improves performance by quite a bit with the overall test completing 1.5 minutes faster. Application launch times were not really affected but it's not something my test machine was suffering from in the first place so it's not really conclusive. The kswapd patches also did not appear to help but again, the test machine wasn't suffering that problem.
Quoted 2 lines
These patches are against 2.6.39-rc7. Again, testing would be appreciated.
Quoted 7 lines
These patches solve the problem for me. I've been soak testing the file copy test
for 3.5 hours with nearly 400 test cycles and observed no lockups at all
- rock solid. From my observations from the output from vmstat the system is behaving sanely.
Thanks for finding a solution - much appreciated!
Can you tell me if just patches 1 and 4 fix the problem please? It'd be good to know if this was only a reclaim-related problem. Thanks.
Signature · 9 lines
Mel Gorman SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 87 lines · Mel Gorman
Quoted 78 lines · Colin Ian King
Quoted 74 lines · Mel Gorman
Changelog since V1
o kswapd should sleep ifneed_resched
o Remove__GFP_REPEATfrom GFP flags when speculatively using high orders so direct/compaction exits earlier
o Remove__GFP_NORETRYfor correctness
o Correct logic insleeping_prematurely
o Leave SLUB using the defaultslub_max_orderThere are a few reports of people experiencing hangs when copying large amounts of data with kswapd using a large amount of CPU which appear to be due to recent reclaim changes.
SLUB using high orders is the trigger but not the root cause as SLUB has been using high orders for a while. The following four patches aim to fix the problems in reclaim while reducing the cost for SLUB using those high orders.
Patch 1 corrects logic introduced by commit [
1741c877: mm:
kswapd: keep kswapd awake for high-order allocations until a percentage of the node is balanced] to allow kswapd to go to sleep when balanced for high orders.Patch 2 prevents kswapd waking up in response to SLUBs speculative
use of high orders.Patch 3 further reduces the cost by prevent SLUB entering direct
compaction or reclaim paths on the grounds that falling back to order-0 should be cheaper.Patch 4 notes that even when kswapd is failing to keep up with
allocation requests, it should still go to sleep when its quota has expired to prevent it spinning.My own data on this is not great. I haven't really been able to reproduce the same problem locally.
The test case is simple. "download tar" wgets a large tar file and stores it locally. "unpack" is expanding it (15 times physical RAM in this case) and "delete source dirs" is the tarfile being deleted again. I also experimented with having the tar copied numerous times and into deeper directories to increase the size but the results were not particularly interesting so I left it as one tar.
In the background, applications are being launched to time to vaguely simulate activity on the desktop and to measure how long it takes applications to start.
Test server, 4 CPU threads,
x86_64, 2G of RAM, no PREEMPT, no COMPACTION, X running LARGE COPY AND UNTAR
vanilla fixprematurely kswapd-nowwake slub-noexstep kswapdsleep
download tar 95 ( 0.00%) 94 ( 1.06%) 94 ( 1.06%) 94 ( 1.06%) 94 ( 1.06%)
unpack tar 654 ( 0.00%) 649 ( 0.77%) 655 (-0.15%) 589 (11.04%) 598 ( 9.36%)
copy source files 0 ( 0.00%) 0 ( 0.00%) 0 ( 0.00%) 0 ( 0.00%) 0 ( 0.00%)
delete source dirs 327 ( 0.00%) 334 (-2.10%) 318 ( 2.83%) 325 ( 0.62%) 320 ( 2.19%)
MMTests Statistics: durationUser/Sys Time Running Test (seconds) 1139.7 1142.55 1149.78 1109.32 1113.26 Total Elapsed Time (seconds) 1341.59 1342.45 1324.90 1271.02 1247.35MMTests Statistics: application launch
evolution-wait30 mean 34.92 34.96 34.92 34.92 35.08 gnome-terminal-find mean 7.96 7.96 8.76 7.80 7.96 iceweasel-table mean 7.93 7.81 7.73 7.65 7.88evolution-wait30 stddev 0.96 1.22 1.27 1.20 1.15 gnome-terminal-find stddev 3.02 3.09 3.51 2.99 3.02 iceweasel-table stddev 1.05 0.90 1.09 1.11 1.11Having SLUB avoid expensive steps in reclaim improves performance by quite a bit with the overall test completing 1.5 minutes faster. Application launch times were not really affected but it's not something my test machine was suffering from in the first place so it's not really conclusive. The kswapd patches also did not appear to help but again, the test machine wasn't suffering that problem.
Quoted 2 lines
These patches are against 2.6.39-rc7. Again, testing would be appreciated.
Quoted 7 lines
These patches solve the problem for me. I've been soak testing the file copy test
for 3.5 hours with nearly 400 test cycles and observed no lockups at all
- rock solid. From my observations from the output from vmstat the system is behaving sanely.
Thanks for finding a solution - much appreciated!
Quoted 2 lines
Can you tell me if just patches 1 and 4 fix the problem please? It'd be good to know if this was only a reclaim-related problem. Thanks.
Hi Mel,
Soak tested just patches 1 + 4 and works fine. Did 250 cycles for ~2 hours, no lockups, and the output from vmstat looked sane.
Colin