Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1252780 > unrolled thread
| Started by | Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> |
|---|---|
| First post | 2015-10-21 14:30 +0200 |
| Last post | 2015-10-22 17:40 +0200 |
| Articles | 12 on this page of 52 — 5 participants |
Back to article view | Back to linux.kernel
[PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2015-10-21 14:30 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Michal Hocko <mhocko@kernel.org> - 2015-10-21 15:10 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Christoph Lameter <cl@linux.com> - 2015-10-21 16:30 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Michal Hocko <mhocko@kernel.org> - 2015-10-21 16:40 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Christoph Lameter <cl@linux.com> - 2015-10-21 16:50 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Michal Hocko <mhocko@kernel.org> - 2015-10-21 17:00 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2015-10-21 17:40 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Christoph Lameter <cl@linux.com> - 2015-10-21 19:20 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2015-10-22 13:40 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Christoph Lameter <cl@linux.com> - 2015-10-22 15:40 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-22 16:20 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-22 16:30 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-22 16:30 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Christoph Lameter <cl@linux.com> - 2015-10-22 16:30 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-22 16:40 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Christoph Lameter <cl@linux.com> - 2015-10-22 16:50 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-22 17:20 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-23 06:30 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Christoph Lameter <cl@linux.com> - 2015-10-22 16:30 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Christoph Lameter <cl@linux.com> - 2015-10-22 16:30 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Michal Hocko <mhocko@kernel.org> - 2015-10-22 17:10 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-22 17:20 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Christoph Lameter <cl@linux.com> - 2015-10-22 17:40 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Michal Hocko <mhocko@kernel.org> - 2015-10-23 10:40 +0200
Make vmstat deferrable again (was Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks) Christoph Lameter <cl@linux.com> - 2015-10-23 13:50 +0200
Re: Make vmstat deferrable again (was Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks) Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2015-10-23 14:10 +0200
Re: Make vmstat deferrable again (was Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks) Christoph Lameter <cl@linux.com> - 2015-10-23 16:20 +0200
Re: Make vmstat deferrable again (was Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks) Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2015-10-23 17:00 +0200
Re: Make vmstat deferrable again (was Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks) Christoph Lameter <cl@linux.com> - 2015-10-23 18:20 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-22 17:40 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Michal Hocko <mhocko@kernel.org> - 2015-10-22 17:50 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-22 20:50 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable()checks Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2015-10-22 23:50 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable()checks Tejun Heo <htejun@gmail.com> - 2015-10-23 00:50 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable()checks Michal Hocko <mhocko@kernel.org> - 2015-10-23 10:40 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable()checks Tejun Heo <htejun@gmail.com> - 2015-10-23 12:40 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Michal Hocko <mhocko@kernel.org> - 2015-10-23 10:40 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-23 12:40 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Michal Hocko <mhocko@kernel.org> - 2015-10-23 13:20 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2015-10-23 14:30 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-23 20:30 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2015-10-25 12:00 +0100
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-25 23:50 +0100
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Michal Hocko <mhocko@kernel.org> - 2015-10-27 10:30 +0100
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-27 12:00 +0100
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Michal Hocko <mhocko@kernel.org> - 2015-10-27 13:10 +0100
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-23 20:30 +0200
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Michal Hocko <mhocko@kernel.org> - 2015-10-27 10:20 +0100
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Tejun Heo <htejun@gmail.com> - 2015-10-27 12:00 +0100
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable()checks Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2015-10-27 12:10 +0100
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable()checks Tejun Heo <htejun@gmail.com> - 2015-10-27 12:40 +0100
Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks Michal Hocko <mhocko@kernel.org> - 2015-10-22 17:40 +0200
Page 3 of 3 — ← Prev page 1 2 [3]
| From | Tejun Heo <htejun@gmail.com> |
|---|---|
| Date | 2015-10-23 20:30 +0200 |
| Subject | Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks |
| Message-ID | <qmPgf-112-39@gated-at.bofh.it> |
| In reply to | #1254541 |
Hello, Tetsuo. On Fri, Oct 23, 2015 at 09:25:11PM +0900, Tetsuo Handa wrote: > WQ_MEM_RECLAIM only guarantees that a "struct task_struct" is preallocated > in order to avoid failing to allocate it on demand due to a GFP_KERNEL > allocation? Is this correct? > > WQ_CPU_INTENSIVE only guarantees that work items don't participate in > concurrency management in order to avoid failing to wake up a "struct > task_struct" which will process the work items? Is this correct? CPU_INTENSIVE avoids the tail end of concurrency management. The previous HIGHPRI or the posted IMMEDIATE avoids the head end. > Is Michal's question "does it make sense to use WQ_MEM_RECLAIM without > WQ_CPU_INTENSIVE"? In other words, any "struct task_struct" which calls > rescuer_thread() must imply WQ_CPU_INTENSIVE in order to avoid failing to > wake up due to being participated in concurrency management? If this is an actual problem, a better approach would be something which detects the stall condition and kicks off the next work item but if we do that I think I'd still trigger a warning there. I don't know. Don't go busy waiting in kernel. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> |
|---|---|
| Date | 2015-10-25 12:00 +0100 |
| Message-ID | <qnrbR-1Jw-33@gated-at.bofh.it> |
| In reply to | #1254862 |
Tejun Heo wrote:
> If this is an actual problem, a better approach would be something
> which detects the stall condition and kicks off the next work item but
> if we do that I think I'd still trigger a warning there. I don't
> know. Don't go busy waiting in kernel.
Busy waiting in kernel refers several cases.
(1) Wait for something with interrupts disabled.
(2) Wait for something with interrupts enabled but
without calling cond_resched() etc.
(3) Wait for something with interrupts enabled and
with calling cond_resched() etc.
(4) Wait for something with interrupts enabled and
with calling schedule_timeout() etc.
Kernel code tries to minimize (1). Kernel code does (2) if they are
not allowed to sleep. But kernel code is allowed to do (3) if they
are allowed to sleep, as long as cond_resched() is sometimes called.
And currently page allocator does (3). But kernel code invoked via
workqueue is expected to do (4) than (3).
This means that any kernel code which invokes a __GFP_WAIT allocation
might fail to do (4) when invoked via workqueue, regardless of flags
passed to alloc_workqueue()?
Michal Hocko wrote:
> On Fri 23-10-15 06:42:43, Tetsuo Handa wrote:
> > Tejun Heo wrote:
> > > On Thu, Oct 22, 2015 at 05:49:22PM +0200, Michal Hocko wrote:
> > > > I am confused. What makes rescuer to not run? Nothing seems to be
> > > > hogging CPUs, we are just out of workers which are loopin in the
> > > > allocator but that is preemptible context.
> > >
> > > It's concurrency management. Workqueue thinks that the pool is making
> > > positive forward progress and doesn't schedule anything else for
> > > execution while that work item is burning cpu cycles.
> >
> > Then, isn't below change easier to backport which will also alleviate
> > needlessly burning CPU cycles?
>
> This is quite obscure. If the vmstat_update fix needs workqueue tweaks
> as well then I would vote for your original patch which is clear,
> straightforward and easy to backport.
I think that inserting a short sleep into page allocator is better
because the vmstat_update fix will not require workqueue tweaks if
we sleep inside page allocator. Also, from the point of view of
protecting page allocator from going unresponsive when hundreds of tasks
started busy-waiting at __alloc_pages_slowpath() because we can observe
that XXX value in the "MemAlloc-Info: XXX stalling task," line grows
when we are unable to make forward progress.
----------------------------------------
>From a2f34850c26b5bb124d44983f5a2020b51249d53 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Sun, 25 Oct 2015 19:42:15 +0900
Subject: [PATCH] mm,page_alloc: Insert an uninterruptible sleep before
retrying.
Since "struct zone"->vm_stat[] is array of atomic_long_t, an effort
to reduce frequency of updating values in vm_stat[] is made by using
per cpu variables "struct per_cpu_pageset"->vm_stat_diff[].
Values in vm_stat_diff[] are merged into vm_stat[] periodically
using vmstat_update workqueue item (struct delayed_work vmstat_work).
When a task attempted to allocate memory and reached direct reclaim
path, shrink_zones() checks whether there are reclaimable pages by
calling zone_reclaimable(). zone_reclaimable() makes decision based
on values in vm_stat[] by calling zone_page_state(). This is usually
fine because values in vm_stat_diff[] are expected to be merged into
vm_stat[] shortly.
But workqueue and page allocator have different assumptions.
(A) The workqueue defers processing of other items unless currently
in-flight item enters into !TASK_RUNNING state.
(B) The page allocator never enters into !TASK_RUNNING state if there
is nothing to reclaim. (The page allocator calls cond_resched()
via wait_iff_congested(), but cond_resched() does not make the
task enter into !TASK_RUNNING state.)
Therefore, if a workqueue item which is processed before vmstat_update
item is processed got stuck inside memory allocation request, values in
vm_stat_diff[] cannot be merged into vm_stat[].
As a result, zone_reclaimable() continues using outdated vm_stat[] values
and the task which is doing direct reclaim path thinks that there are
still reclaimable pages and therefore continues looping.
The consequence is a silent livelock (hang up without any kernel messages)
because the OOM killer will not be invoked. We can hit such livelock by
e.g. disk_events_workfn workqueue item doing memory allocation from
bio_copy_kern().
----------------------------------------
[ 255.054205] kworker/3:1 R running task 0 45 2 0x00000008
[ 255.056063] Workqueue: events_freezable_power_ disk_events_workfn
[ 255.057715] ffff88007f805680 ffff88007c55f6d0 ffffffff8116463d ffff88007c55f758
[ 255.059705] ffff88007f82b870 ffff88007c55f6e0 ffffffff811646be ffff88007c55f710
[ 255.061694] ffffffff811bdaf0 ffff88007f82b870 0000000000000400 0000000000000000
[ 255.063690] Call Trace:
[ 255.064664] [<ffffffff8116463d>] ? __list_lru_count_one.isra.4+0x1d/0x80
[ 255.066428] [<ffffffff811646be>] ? list_lru_count_one+0x1e/0x20
[ 255.068063] [<ffffffff811bdaf0>] ? super_cache_count+0x50/0xd0
[ 255.069666] [<ffffffff8114ecf6>] ? shrink_slab.part.38+0xf6/0x2a0
[ 255.071313] [<ffffffff81151f78>] ? shrink_zone+0x2c8/0x2e0
[ 255.072845] [<ffffffff81152316>] ? do_try_to_free_pages+0x156/0x6d0
[ 255.074527] [<ffffffff810bc6b6>] ? mark_held_locks+0x66/0x90
[ 255.076085] [<ffffffff816ca797>] ? _raw_spin_unlock_irq+0x27/0x40
[ 255.077727] [<ffffffff810bc7d9>] ? trace_hardirqs_on_caller+0xf9/0x1c0
[ 255.079451] [<ffffffff81152924>] ? try_to_free_pages+0x94/0xc0
[ 255.081045] [<ffffffff81145b4a>] ? __alloc_pages_nodemask+0x72a/0xdb0
[ 255.082761] [<ffffffff8118cd06>] ? alloc_pages_current+0x96/0x1b0
[ 255.084407] [<ffffffff8133985d>] ? bio_alloc_bioset+0x20d/0x2d0
[ 255.086032] [<ffffffff8133aba4>] ? bio_copy_kern+0xc4/0x180
[ 255.087584] [<ffffffff81344f20>] ? blk_rq_map_kern+0x70/0x130
[ 255.089161] [<ffffffff814a334d>] ? scsi_execute+0x12d/0x160
[ 255.090696] [<ffffffff814a3474>] ? scsi_execute_req_flags+0x84/0xf0
[ 255.092466] [<ffffffff814b55f2>] ? sr_check_events+0xb2/0x2a0
[ 255.094042] [<ffffffff814c3223>] ? cdrom_check_events+0x13/0x30
[ 255.095634] [<ffffffff814b5a35>] ? sr_block_check_events+0x25/0x30
[ 255.097278] [<ffffffff813501fb>] ? disk_check_events+0x5b/0x150
[ 255.098865] [<ffffffff81350307>] ? disk_events_workfn+0x17/0x20
[ 255.100451] [<ffffffff810890b5>] ? process_one_work+0x1a5/0x420
[ 255.102046] [<ffffffff81089051>] ? process_one_work+0x141/0x420
[ 255.103625] [<ffffffff8108944b>] ? worker_thread+0x11b/0x490
[ 255.105159] [<ffffffff816c4e95>] ? __schedule+0x315/0xac0
[ 255.106643] [<ffffffff81089330>] ? process_one_work+0x420/0x420
[ 255.108217] [<ffffffff8108f4e9>] ? kthread+0xf9/0x110
[ 255.109634] [<ffffffff8108f3f0>] ? kthread_create_on_node+0x230/0x230
[ 255.111307] [<ffffffff816cb35f>] ? ret_from_fork+0x3f/0x70
[ 255.112785] [<ffffffff8108f3f0>] ? kthread_create_on_node+0x230/0x230
(...snipped...)
[ 273.930846] Showing busy workqueues and worker pools:
[ 273.932299] workqueue events: flags=0x0
[ 273.933465] pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=4/256
[ 273.935120] pending: vmpressure_work_fn, vmstat_shepherd, vmstat_update, vmw_fb_dirty_flush [vmwgfx]
[ 273.937489] workqueue events_freezable: flags=0x4
[ 273.938795] pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=1/256
[ 273.940446] pending: vmballoon_work [vmw_balloon]
[ 273.941973] workqueue events_power_efficient: flags=0x80
[ 273.943491] pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=1/256
[ 273.945167] pending: check_lifetime
[ 273.946422] workqueue events_freezable_power_: flags=0x84
[ 273.947890] pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=1/256
[ 273.949579] in-flight: 45:disk_events_workfn
[ 273.951103] workqueue ipv6_addrconf: flags=0x8
[ 273.952447] pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=1/1
[ 273.954121] pending: addrconf_verify_work
[ 273.955541] workqueue xfs-reclaim/sda1: flags=0x4
[ 273.957036] pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=1/256
[ 273.958847] pending: xfs_reclaim_worker
[ 273.960392] pool 6: cpus=3 node=0 flags=0x0 nice=0 workers=3 idle: 186 26
----------------------------------------
Three approaches are proposed for fixing this silent livelock problem.
(1) Use zone_page_state_snapshot() instead of zone_page_state()
when doing zone_reclaimable() checks. This approach is clear,
straightforward and easy to backport. So far I cannot reproduce
this livelock using this change. But there might be more locations
which should use zone_page_state_snapshot().
(2) Use a dedicated workqueue for vmstat_update item which is guaranteed
to be processed immediately. So far I cannot reproduce this livelock
using a dedicated workqueue created with WQ_MEM_RECLAIM|WQ_HIGHPRI
(patch proposed by Christoph Lameter). But according to Tejun Heo,
if we want to guarantee that nobody can reproduce this livelock, we
need to modify workqueue API because commit 3270476a6c0c ("workqueue:
reimplement WQ_HIGHPRI using a separate worker_pool") which went to
Linux 3.6 lost the guarantee.
(3) Use a !TASK_RUNNING sleep inside page allocator side. This approach
is easy to backport. So far I cannot reproduce this livelock using
this approach. And I think that nobody can reproduce this livelock
because this changes the page allocator to obey the workqueue's
expectations. Even if we leave this livelock problem aside, not
entering into !TASK_RUNNING state for too long is an exclusive
occupation of workqueue which will make other items in the workqueue
needlessly deferred. We don't need to defer other items which do not
invoke a __GFP_WAIT allocation.
This patch does approach (3), by inserting an uninterruptible sleep into
page allocator side before retrying, in order to make sure that other
workqueue items (especially vmstat_update item) are given a chance to be
processed.
Although a different problem, by using approach (3), we can alleviate
needlessly burning CPU cycles even when we hit OOM-killer livelock problem
(hang up after the OOM-killer messages are printed because the OOM victim
cannot terminate due to dependency).
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
mm/oom_kill.c | 8 +-------
mm/page_alloc.c | 19 +++++++++++++++++--
2 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index d13a339..877b5a5 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -722,15 +722,9 @@ bool out_of_memory(struct oom_control *oc)
dump_header(oc, NULL, NULL);
panic("Out of memory and no killable processes...\n");
}
- if (p && p != (void *)-1UL) {
+ if (p && p != (void *)-1UL)
oom_kill_process(oc, p, points, totalpages, NULL,
"Out of memory");
- /*
- * Give the killed process a good chance to exit before trying
- * to allocate memory again.
- */
- schedule_timeout_killable(1);
- }
return true;
}
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3687f4c..047ebda 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2726,7 +2726,6 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
*/
if (!mutex_trylock(&oom_lock)) {
*did_some_progress = 1;
- schedule_timeout_uninterruptible(1);
return NULL;
}
@@ -3385,6 +3384,15 @@ retry:
((gfp_mask & __GFP_REPEAT) && pages_reclaimed < (1 << order))) {
/* Wait for some write requests to complete then retry */
wait_iff_congested(ac->preferred_zone, BLK_RW_ASYNC, HZ/50);
+ /*
+ * Give other workqueue items (especially vmstat_update item)
+ * a chance to be processed. There is no need to wait if I was
+ * chosen by the OOM killer, for I will leave this function
+ * using ALLOC_NO_WATERMARKS. But I need to wait even if I have
+ * SIGKILL pending, for I can't leave this function.
+ */
+ if (!test_thread_flag(TIF_MEMDIE))
+ schedule_timeout_uninterruptible(1);
goto retry;
}
@@ -3394,8 +3402,15 @@ retry:
goto got_pg;
/* Retry as long as the OOM killer is making progress */
- if (did_some_progress)
+ if (did_some_progress) {
+ /*
+ * Give the OOM victim a chance to leave this function
+ * before trying to allocate memory again.
+ */
+ if (!test_thread_flag(TIF_MEMDIE))
+ schedule_timeout_uninterruptible(1);
goto retry;
+ }
noretry:
/*
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tejun Heo <htejun@gmail.com> |
|---|---|
| Date | 2015-10-25 23:50 +0100 |
| Subject | Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks |
| Message-ID | <qnCgW-hK-19@gated-at.bofh.it> |
| In reply to | #1255361 |
Hello, On Sun, Oct 25, 2015 at 07:52:59PM +0900, Tetsuo Handa wrote: ... > This means that any kernel code which invokes a __GFP_WAIT allocation > might fail to do (4) when invoked via workqueue, regardless of flags > passed to alloc_workqueue()? Sounds that way and yeah (3) should technically be okay and that's why HIGHPRI was implemented the way it was at the beginning; however, in practice, this is the first time it's noticeable in all the years. I think it comes down to the fact that there just aren't many places which need such looping behavior and even in those places it's often very undesirable to busy-loop while not making forward-progress (and if forward-progress is being made, it won't be indefinite). > I think that inserting a short sleep into page allocator is better > because the vmstat_update fix will not require workqueue tweaks if > we sleep inside page allocator. Also, from the point of view of > protecting page allocator from going unresponsive when hundreds of tasks > started busy-waiting at __alloc_pages_slowpath() because we can observe > that XXX value in the "MemAlloc-Info: XXX stalling task," line grows > when we are unable to make forward progress. This looks good to me too; however, it still needs a dedicated workqueue with WQ_MEM_RECLAIM set. That deadlock probably is very unlikely as the side effect of vmstat failing to execute due to worker exhaustion is more memory reclaim but it still is theoretically possible and it could just be that it happens at low enough frequency that it hasn't been reported yet. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2015-10-27 10:30 +0100 |
| Subject | Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks |
| Message-ID | <qo8JR-3sW-11@gated-at.bofh.it> |
| In reply to | #1255361 |
On Sun 25-10-15 19:52:59, Tetsuo Handa wrote:
[...]
> Three approaches are proposed for fixing this silent livelock problem.
>
> (1) Use zone_page_state_snapshot() instead of zone_page_state()
> when doing zone_reclaimable() checks. This approach is clear,
> straightforward and easy to backport. So far I cannot reproduce
> this livelock using this change. But there might be more locations
> which should use zone_page_state_snapshot().
>
> (2) Use a dedicated workqueue for vmstat_update item which is guaranteed
> to be processed immediately. So far I cannot reproduce this livelock
> using a dedicated workqueue created with WQ_MEM_RECLAIM|WQ_HIGHPRI
> (patch proposed by Christoph Lameter). But according to Tejun Heo,
> if we want to guarantee that nobody can reproduce this livelock, we
> need to modify workqueue API because commit 3270476a6c0c ("workqueue:
> reimplement WQ_HIGHPRI using a separate worker_pool") which went to
> Linux 3.6 lost the guarantee.
>
> (3) Use a !TASK_RUNNING sleep inside page allocator side. This approach
> is easy to backport. So far I cannot reproduce this livelock using
> this approach. And I think that nobody can reproduce this livelock
> because this changes the page allocator to obey the workqueue's
> expectations. Even if we leave this livelock problem aside, not
> entering into !TASK_RUNNING state for too long is an exclusive
> occupation of workqueue which will make other items in the workqueue
> needlessly deferred. We don't need to defer other items which do not
> invoke a __GFP_WAIT allocation.
>
> This patch does approach (3), by inserting an uninterruptible sleep into
> page allocator side before retrying, in order to make sure that other
> workqueue items (especially vmstat_update item) are given a chance to be
> processed.
>
> Although a different problem, by using approach (3), we can alleviate
> needlessly burning CPU cycles even when we hit OOM-killer livelock problem
> (hang up after the OOM-killer messages are printed because the OOM victim
> cannot terminate due to dependency).
I really dislike this approach. Waiting without having an event to
wait for is just too ugly. I think 1) is easiest to backport to
stable kernels without causing any other regressions. 2) is the way
to move forward for next kernels and we should really think whether
WQ_MEM_RECLAIM should imply also WQ_HIGHPRI by default. If there is a
general consensus that there are legitimate WQ_MEM_RECLAIM users which
can do without the other flag then I am perfectly OK to use it for
vmstat and oom sysrq dedicated workqueues.
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
[...]
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tejun Heo <htejun@gmail.com> |
|---|---|
| Date | 2015-10-27 12:00 +0100 |
| Subject | Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks |
| Message-ID | <qoa8W-4bL-5@gated-at.bofh.it> |
| In reply to | #1256609 |
On Tue, Oct 27, 2015 at 10:22:31AM +0100, Michal Hocko wrote: ... > stable kernels without causing any other regressions. 2) is the way > to move forward for next kernels and we should really think whether > WQ_MEM_RECLAIM should imply also WQ_HIGHPRI by default. If there is a > general consensus that there are legitimate WQ_MEM_RECLAIM users which > can do without the other flag then I am perfectly OK to use it for > vmstat and oom sysrq dedicated workqueues. I don't think flagging these things is a good approach. These are too easy to miss. If this is a problem which needs to be solved, which I'm not convined it is at this point, the right thing to do would be doing stall detection and kicking the next work item automatically. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2015-10-27 13:10 +0100 |
| Subject | Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks |
| Message-ID | <qobeH-53s-21@gated-at.bofh.it> |
| In reply to | #1256693 |
On Tue 27-10-15 19:55:06, Tejun Heo wrote: > On Tue, Oct 27, 2015 at 10:22:31AM +0100, Michal Hocko wrote: > ... > > stable kernels without causing any other regressions. 2) is the way > > to move forward for next kernels and we should really think whether > > WQ_MEM_RECLAIM should imply also WQ_HIGHPRI by default. If there is a > > general consensus that there are legitimate WQ_MEM_RECLAIM users which > > can do without the other flag then I am perfectly OK to use it for > > vmstat and oom sysrq dedicated workqueues. > > I don't think flagging these things is a good approach. These are too > easy to miss. If this is a problem which needs to be solved, which > I'm not convined it is at this point, the right thing to do would be > doing stall detection and kicking the next work item automatically. To be honest, I do not really care whether this gets "fixed" in the stall detection code or by making WQ_MEM_RECLAIM to flag a special behavior implicitly. All I would like to see is to have a guarantee that such workqueues are not staying behind just because all current workers are in the allocator. Adding artificial schedule_timeouts in the allocator is a fragile way to work around the issue. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tejun Heo <htejun@gmail.com> |
|---|---|
| Date | 2015-10-23 20:30 +0200 |
| Subject | Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks |
| Message-ID | <qmPgg-112-67@gated-at.bofh.it> |
| In reply to | #1254490 |
Hello, On Fri, Oct 23, 2015 at 01:11:45PM +0200, Michal Hocko wrote: > > The problem here is not lack > > of execution resource but concurrency management misunderstanding the > > situation. > > And this sounds like a bug to me. I don't know. I can be argued either way, the other direction being a kernel thread going RUNNING non-stop is buggy. Given how this has been a complete non-issue for all the years, I'm not sure how useful plugging this is. > Don't we have some IO related paths which would suffer from the same > problem. I haven't checked all the WQ_MEM_RECLAIM users but from the > name I would expect they _do_ participate in the reclaim and so they > should be able to make a progress. Now if your new IMMEDIATE flag will Seriously, nobody goes full-on RUNNING. > guarantee that then I would argue that it should be implicit for > WQ_MEM_RECLAIM otherwise we always risk a similar situation. What would > be a counter argument for doing that? Not serving any actual purpose and degrading execution behavior. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2015-10-27 10:20 +0100 |
| Subject | Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks |
| Message-ID | <qo8Aa-3pT-9@gated-at.bofh.it> |
| In reply to | #1254878 |
On Sat 24-10-15 03:21:09, Tejun Heo wrote: > Hello, > > On Fri, Oct 23, 2015 at 01:11:45PM +0200, Michal Hocko wrote: > > > The problem here is not lack > > > of execution resource but concurrency management misunderstanding the > > > situation. > > > > And this sounds like a bug to me. > > I don't know. I can be argued either way, the other direction being a > kernel thread going RUNNING non-stop is buggy. Given how this has > been a complete non-issue for all the years, I'm not sure how useful > plugging this is. Well, I guess we haven't noticed because this is a pathological case. It also triggers OOM livelocks which were not reported in the past either. You do not reach this state normally unless you rely _want_ to kill your machine And vmstat is not the only instance. E.g. sysrq oom trigger is known to stay behind in similar cases. It should be changed to a dedicated WQ_MEM_RECLAIM wq and it would require runnable item guarantee as well. > > Don't we have some IO related paths which would suffer from the same > > problem. I haven't checked all the WQ_MEM_RECLAIM users but from the > > name I would expect they _do_ participate in the reclaim and so they > > should be able to make a progress. Now if your new IMMEDIATE flag will > > Seriously, nobody goes full-on RUNNING. Looping with cond_resched seems like general pattern in the kernel when there is no clear source to wait for. We have io_schedule when we know we should wait for IO (in case of congestion) but this is not necessarily the case - as you can see here. What should we wait for? A short nap without actually waiting on anything sounds like a dirty workaround to me. > > guarantee that then I would argue that it should be implicit for > > WQ_MEM_RECLAIM otherwise we always risk a similar situation. What would > > be a counter argument for doing that? > > Not serving any actual purpose and degrading execution behavior. I dunno, I am not familiar with WQ internals to see the risks but to me it sounds like WQ_MEM_RECLAIM gives an incorrect impression of safety wrt. memory pressure and as demonstrated it doesn't do that. Even if you consider cond_resched behavior of the page allocator as bug we should be able to handle this gracefully. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tejun Heo <htejun@gmail.com> |
|---|---|
| Date | 2015-10-27 12:00 +0100 |
| Subject | Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks |
| Message-ID | <qoa8W-4bL-11@gated-at.bofh.it> |
| In reply to | #1256598 |
Hello, Michal. On Tue, Oct 27, 2015 at 10:16:03AM +0100, Michal Hocko wrote: > > Seriously, nobody goes full-on RUNNING. > > Looping with cond_resched seems like general pattern in the kernel when > there is no clear source to wait for. We have io_schedule when we know > we should wait for IO (in case of congestion) but this is not necessarily > the case - as you can see here. What should we wait for? A short nap > without actually waiting on anything sounds like a dirty workaround to > me. It's one thing to do cond_resched() in long loops to avoid long priority inversions and another to indefinitely loop without making any difference. > > > guarantee that then I would argue that it should be implicit for > > > WQ_MEM_RECLAIM otherwise we always risk a similar situation. What would > > > be a counter argument for doing that? > > > > Not serving any actual purpose and degrading execution behavior. > > I dunno, I am not familiar with WQ internals to see the risks but to me > it sounds like WQ_MEM_RECLAIM gives an incorrect impression of safety > wrt. memory pressure and as demonstrated it doesn't do that. Even if you It generally does. This is an extremely rare corner case where infinite loop w/o forward progress is introduce w/o the user being outright buggy. > consider cond_resched behavior of the page allocator as bug we should be > able to handle this gracefully. We can argue this back and forth forever but we'll either need to special case it (be it short sleep or a special flag) or implement a rather complex detection logic which will likely involve some level of complexity and is dubious in its practical usefulness. It's a trade-off and given the circumstances adding short sleep looks like a reasonable one to me. If this is more common, we definitely wanna go for automatic detection. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> |
|---|---|
| Date | 2015-10-27 12:10 +0100 |
| Subject | Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable()checks |
| Message-ID | <qoaiB-4ul-9@gated-at.bofh.it> |
| In reply to | #1256598 |
Michal Hocko wrote: > > On Fri, Oct 23, 2015 at 01:11:45PM +0200, Michal Hocko wrote: > > > > The problem here is not lack > > > > of execution resource but concurrency management misunderstanding the > > > > situation. > > > > > > And this sounds like a bug to me. > > > > I don't know. I can be argued either way, the other direction being a > > kernel thread going RUNNING non-stop is buggy. Given how this has > > been a complete non-issue for all the years, I'm not sure how useful > > plugging this is. > > Well, I guess we haven't noticed because this is a pathological case. It > also triggers OOM livelocks which were not reported in the past either. > You do not reach this state normally unless you rely _want_ to kill your > machine I don't think we can say this is a pathological case. Customers' serves might have hit this state. We have no code for warning this state. > > And vmstat is not the only instance. E.g. sysrq oom trigger is known > to stay behind in similar cases. It should be changed to a dedicated > WQ_MEM_RECLAIM wq and it would require runnable item guarantee as well. > Well, this seems to be the cause of SysRq-f being unresponsive... http://lkml.kernel.org/r/201411231349.CAG78628.VFQFOtOSFJMOLH@I-love.SAKURA.ne.jp Picking up from http://lkml.kernel.org/r/201506112212.JAG26531.FLSVFMOQJOtOHF@I-love.SAKURA.ne.jp ---------- [ 515.536393] Showing busy workqueues and worker pools: [ 515.538185] workqueue events: flags=0x0 [ 515.539758] pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=8/256 [ 515.541872] pending: vmpressure_work_fn, console_callback, vmstat_update, flush_to_ldisc, push_to_pool, moom_callback, sysrq_reinject_alt_sysrq, fb_deferred_io_work [ 515.546684] workqueue events_power_efficient: flags=0x80 [ 515.548589] pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=2/256 [ 515.550829] pending: neigh_periodic_work, check_lifetime [ 515.552884] workqueue events_freezable_power_: flags=0x84 [ 515.554742] pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=1/256 [ 515.556846] in-flight: 3837:disk_events_workfn [ 515.558665] workqueue writeback: flags=0x4e [ 515.560291] pwq 16: cpus=0-7 flags=0x4 nice=0 active=2/256 [ 515.562271] in-flight: 3812:bdi_writeback_workfn bdi_writeback_workfn [ 515.564544] workqueue xfs-data/sda1: flags=0xc [ 515.566265] pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=4/256 [ 515.568359] in-flight: 374(RESCUER):xfs_end_io, 3759:xfs_end_io, 26:xfs_end_io, 3836:xfs_end_io [ 515.571018] pwq 2: cpus=1 node=0 flags=0x0 nice=0 active=1/256 [ 515.573113] in-flight: 179:xfs_end_io [ 515.574782] pool 2: cpus=1 node=0 flags=0x0 nice=0 workers=4 idle: 3790 237 3820 [ 515.577230] pool 6: cpus=3 node=0 flags=0x0 nice=0 workers=5 manager: 219 [ 515.579488] pool 16: cpus=0-7 flags=0x4 nice=0 workers=3 idle: 356 357 ---------- We want immediate execution guarantee for not only vmstat_update and moom_callback but also vmstat_shepherd and console_callback? > > > Don't we have some IO related paths which would suffer from the same > > > problem. I haven't checked all the WQ_MEM_RECLAIM users but from the > > > name I would expect they _do_ participate in the reclaim and so they > > > should be able to make a progress. Now if your new IMMEDIATE flag will > > > > Seriously, nobody goes full-on RUNNING. > > Looping with cond_resched seems like general pattern in the kernel when > there is no clear source to wait for. We have io_schedule when we know > we should wait for IO (in case of congestion) but this is not necessarily > the case - as you can see here. What should we wait for? A short nap > without actually waiting on anything sounds like a dirty workaround to > me. Can't we have a waitqueue like http://lkml.kernel.org/r/201510142121.IDE86954.SOVOFFQOFMJHtL@I-love.SAKURA.ne.jp ? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tejun Heo <htejun@gmail.com> |
|---|---|
| Date | 2015-10-27 12:40 +0100 |
| Subject | Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable()checks |
| Message-ID | <qoaLE-4E6-3@gated-at.bofh.it> |
| In reply to | #1256697 |
On Tue, Oct 27, 2015 at 08:07:38PM +0900, Tetsuo Handa wrote: > Can't we have a waitqueue like > http://lkml.kernel.org/r/201510142121.IDE86954.SOVOFFQOFMJHtL@I-love.SAKURA.ne.jp ? There's no reason to complicate it. It wouldn't buy anything meaningful. Can we please stop trying to solve a non-existent problem? -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2015-10-22 17:40 +0200 |
| Subject | Re: [PATCH] mm,vmscan: Use accurate values for zone_reclaimable() checks |
| Message-ID | <qmq8a-6GG-21@gated-at.bofh.it> |
| In reply to | #1253900 |
On Fri 23-10-15 00:15:28, Tejun Heo wrote: > On Thu, Oct 22, 2015 at 05:06:23PM +0200, Michal Hocko wrote: > > Do I get it right that if vmstat_update has its own workqueue with > > WQ_MEM_RECLAIM then there is a _guarantee_ that the rescuer will always > > be able to process vmstat_update work from the requested CPU? > > Yeah. Thanks for the confirmation. > > That should be sufficient because vmstat_update doesn't sleep on > > allocation. I agree that this would be a more appropriate fix. > > The problem seems to be reclaim path busy looping waiting for > vmstat_update and workqueue thinking that the work item must be making > forward-progress and thus not starting the next work item. But that shouldn't happen because the allocation path does cond_resched even when nothing is really reclaimable (e.g. wait_iff_congested from __alloc_pages_slowpath). -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Page 3 of 3 — ← Prev page 1 2 [3]
Back to top | Article view | linux.kernel
csiph-web