Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1440722 > unrolled thread
| Started by | Mikulas Patocka <mpatocka@redhat.com> |
|---|---|
| First post | 2016-07-11 17:50 +0200 |
| Last post | 2016-07-14 02:10 +0200 |
| Articles | 20 on this page of 60 — 8 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-11 17:50 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-12 08:50 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-13 01:50 +0200
Re: System freezes after OOM Jerome Marchand <jmarchan@redhat.com> - 2016-07-13 10:40 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-13 13:20 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-13 16:30 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-13 13:20 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-13 15:00 +0200
Re: System freezes after OOM Milan Broz <gmazyland@gmail.com> - 2016-07-13 15:50 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-13 17:40 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-14 11:10 +0200
Re: System freezes after OOM Milan Broz <gmazyland@gmail.com> - 2016-07-14 11:50 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-13 17:10 +0200
Re: [dm-devel] System freezes after OOM Ondrej Kozina <okozina@redhat.com> - 2016-07-14 13:00 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-14 15:00 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-14 16:10 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-14 17:00 +0200
Re: System freezes after OOM Ondrej Kozina <okozina@redhat.com> - 2016-07-14 17:30 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-14 19:40 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-15 10:40 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-15 14:20 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-15 14:30 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-15 19:10 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-18 09:30 +0200
Re: System freezes after OOM Ondrej Kozina <okozina@redhat.com> - 2016-07-14 17:30 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-14 17:40 +0200
Re: System freezes after OOM Ondrej Kozina <okozina@redhat.com> - 2016-07-14 19:10 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-14 19:40 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-14 19:40 +0200
Re: System freezes after OOM Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-07-15 13:50 +0200
Re: System freezes after OOM Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-07-13 15:30 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-13 15:50 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-13 16:30 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-13 17:00 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-13 17:20 +0200
Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-14 02:00 +0200
Re: System freezes after OOM Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-07-14 13:10 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-14 14:30 +0200
Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-14 22:30 +0200
Re: System freezes after OOM Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-07-14 23:50 +0200
Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-15 00:10 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-15 13:30 +0200
Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-15 23:30 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-14 14:30 +0200
Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-14 22:30 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-15 13:30 +0200
Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-15 23:30 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-15 23:50 +0200
Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-16 00:00 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-16 02:00 +0200
Re: System freezes after OOM Johannes Weiner <hannes@cmpxchg.org> - 2016-07-18 17:20 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-14 17:30 +0200
Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-14 22:40 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-15 09:30 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-15 10:30 +0200
Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-15 14:10 +0200
Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-15 23:50 +0200
Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-18 09:40 +0200
Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-18 23:10 +0200
Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-14 02:10 +0200
Page 1 of 3 [1] 2 3 Next page →
| From | Mikulas Patocka <mpatocka@redhat.com> |
|---|---|
| Date | 2016-07-11 17:50 +0200 |
| Subject | Re: System freezes after OOM |
| Message-ID | <rTLD3-xi-5@gated-at.bofh.it> |
On Mon, 11 Jul 2016, Ondrej Kozina wrote: > On 07/11/2016 01:55 PM, Jerome Marchand wrote: > > On 07/11/2016 01:03 PM, Stanislav Kozina wrote: > > > Hi Jerome, > > > > > > On upstream mailing lists there have been reports of freezing systems > > > due to OOM. Ondra (on CC) managed to reproduce this inhouse, he'd like > > > someone with mm skills to look at the problem since he doesn't > > > understand why OOM comes into play when >90% of 2GB swap are still free. > > > > > > Could you please take a look? It's following this email on upstream: > > > https://lkml.org/lkml/2016/5/5/356 > > > > > > Thanks! > > > -Stanislav > > > > Hi Ondrej, > > > > I can see [1] that there are several atomic memory allocation failures > > before the OOM kill, several of them are in memory reclaim path, which > > prevents it to free memory. > > Normally the linux mm try to keep enough memory free at all time to > > satisfy atomic allocation (cf. /proc/sys/vm/min_free_kbytes). Have you > > try to increase that value? > > It would be useful to understand why the reserve for atomic allocations > > runs out. There might be a burst of atomic allocations that deplete the > > reserve. What kind of workload is that? > > > > Jerome > > > > [1]: > > https://okozina.fedorapeople.org/bugs/swap_on_dmcrypt/vmlog-1462458369-00000/sample-00011/dmesg > > > > Hi Jerome, > > first let thank you for looking into it! About the workload it's nothing > special. I've started gcc build of a project in C++ in 3-4 threads so that I'd > waste all physical memory to trigger it. I can build some simple utility to > allocate memory in predefined chunks in some loop if it'd of any help. It was > really quite simple to trigger this. > > On a /proc/sys/vm/min_free_kbytes value. Let me try it... > > Thanks Ondra > > PS: Adding Mikulas on CC'ed (dm-crypt upstream) in case he has anything to > add. That allocation warning in wb_start_writeback was already silenced by the commit 78ebc2f7146156f488083c9e5a7ded9d5c38c58b. The warning in drivers/virtio/virtio_ring.c:alloc_indirect could be silenced as well (the driver does fallback in case of allocation failure, so this failure can't result in loss of functionality). The general problem is that the memory allocator does 16 retries to allocate a page and then triggers the OOM killer (and it doesn't take into account how much swap space is free or how many dirty pages were really swapped out while it waited). So, it could prematurely trigger OOM killer on any slow swapping device (including dm-crypt). Michal Hocko reworked the OOM killer in the patch 0a0337e0d1d134465778a16f5cbea95086e8e9e0, but it still has the flaw that it triggers OOM if there is plenty of free swap space free. Michal, would you accept a change to the OOM killer, to prevent it from triggerring when there is free swap space? Mikulas
[toc] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-07-12 08:50 +0200 |
| Message-ID | <rTZG1-1jg-7@gated-at.bofh.it> |
| In reply to | #1440722 |
On Mon 11-07-16 11:43:02, Mikulas Patocka wrote: [...] > The general problem is that the memory allocator does 16 retries to > allocate a page and then triggers the OOM killer (and it doesn't take into > account how much swap space is free or how many dirty pages were really > swapped out while it waited). Well, that is not how it works exactly. We retry as long as there is a reclaim progress (at least one page freed) back off only if the reclaimable memory can exceed watermks which is scaled down in 16 retries. The overal size of free swap is not really that important if we cannot swap out like here due to complete memory reserves depletion: https://okozina.fedorapeople.org/bugs/swap_on_dmcrypt/vmlog-1462458369-00000/sample-00011/dmesg: [ 90.491276] Node 0 DMA free:0kB min:60kB low:72kB high:84kB active_anon:4096kB inactive_anon:4636kB active_file:212kB inactive_file:280kB unevictable:488kB isolated(anon):0kB isolated(file):0kB present:15992kB managed:15908kB mlocked:488kB dirty:276kB writeback:4636kB mapped:476kB shmem:12kB slab_reclaimable:204kB slab_unreclaimable:4700kB kernel_stack:48kB pagetables:120kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:61132 all_unreclaimable? yes [ 90.491283] lowmem_reserve[]: 0 977 977 977 [ 90.491286] Node 0 DMA32 free:0kB min:3828kB low:4824kB high:5820kB active_anon:423820kB inactive_anon:424916kB active_file:17996kB inactive_file:21800kB unevictable:20724kB isolated(anon):384kB isolated(file):0kB present:1032184kB managed:1001260kB mlocked:20724kB dirty:25236kB writeback:49972kB mapped:23076kB shmem:1364kB slab_reclaimable:13796kB slab_unreclaimable:43008kB kernel_stack:2816kB pagetables:7320kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:5635400 all_unreclaimable? yes Look at the amount of free memory. It is completely depleted. So it smells like a process which has access to memory reserves has consumed all of it. I suspect a __GFP_MEMALLOC resp. PF_MEMALLOC from softirq context user which went off the leash. > So, it could prematurely trigger OOM killer on any slow swapping device > (including dm-crypt). Michal Hocko reworked the OOM killer in the patch > 0a0337e0d1d134465778a16f5cbea95086e8e9e0, but it still has the flaw that > it triggers OOM if there is plenty of free swap space free. > > Michal, would you accept a change to the OOM killer, to prevent it from > triggerring when there is free swap space? No this doesn't sound like a proper solution. The current decision logic, as explained above relies on the feedback from the reclaim. A free swap space doesn't really mean we can make a forward progress. -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Mikulas Patocka <mpatocka@redhat.com> |
|---|---|
| Date | 2016-07-13 01:50 +0200 |
| Message-ID | <rUfB7-3og-9@gated-at.bofh.it> |
| In reply to | #1441087 |
The problem of swapping to dm-crypt is this. The free memory goes low, kswapd decides that some page should be swapped out. However, when you swap to an ecrypted device, writeback of each page requires another page to hold the encrypted data. dm-crypt uses mempools for all its structures and pages, so that it can make forward progress even if there is no memory free. However, the mempool code first allocates from general memory allocator and resorts to the mempool only if the memory is below limit. So every attempt to swap out some page allocates another page. As long as swapping is in progress, the free memory is below the limit (because the swapping activity itself consumes any memory over the limit). And that triggered the OOM killer prematurely. On Tue, 12 Jul 2016, Michal Hocko wrote: > On Mon 11-07-16 11:43:02, Mikulas Patocka wrote: > [...] > > The general problem is that the memory allocator does 16 retries to > > allocate a page and then triggers the OOM killer (and it doesn't take into > > account how much swap space is free or how many dirty pages were really > > swapped out while it waited). > > Well, that is not how it works exactly. We retry as long as there is a > reclaim progress (at least one page freed) back off only if the > reclaimable memory can exceed watermks which is scaled down in 16 > retries. The overal size of free swap is not really that important if we > cannot swap out like here due to complete memory reserves depletion: > https://okozina.fedorapeople.org/bugs/swap_on_dmcrypt/vmlog-1462458369-00000/sample-00011/dmesg: > [ 90.491276] Node 0 DMA free:0kB min:60kB low:72kB high:84kB active_anon:4096kB inactive_anon:4636kB active_file:212kB inactive_file:280kB unevictable:488kB isolated(anon):0kB isolated(file):0kB present:15992kB managed:15908kB mlocked:488kB dirty:276kB writeback:4636kB mapped:476kB shmem:12kB slab_reclaimable:204kB slab_unreclaimable:4700kB kernel_stack:48kB pagetables:120kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:61132 all_unreclaimable? yes > [ 90.491283] lowmem_reserve[]: 0 977 977 977 > [ 90.491286] Node 0 DMA32 free:0kB min:3828kB low:4824kB high:5820kB active_anon:423820kB inactive_anon:424916kB active_file:17996kB inactive_file:21800kB unevictable:20724kB isolated(anon):384kB isolated(file):0kB present:1032184kB managed:1001260kB mlocked:20724kB dirty:25236kB writeback:49972kB mapped:23076kB shmem:1364kB slab_reclaimable:13796kB slab_unreclaimable:43008kB kernel_stack:2816kB pagetables:7320kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:5635400 all_unreclaimable? yes > > Look at the amount of free memory. It is completely depleted. So it > smells like a process which has access to memory reserves has consumed > all of it. I suspect a __GFP_MEMALLOC resp. PF_MEMALLOC from softirq > context user which went off the leash. It is caused by the commit f9054c70d28bc214b2857cf8db8269f4f45a5e23. Prior to this commit, mempool allocations set __GFP_NOMEMALLOC, so they never exhausted reserved memory. With this commit, mempool allocations drop __GFP_NOMEMALLOC, so they can dig deeper (if the process has PF_MEMALLOC, they can bypass all limits). But swapping should proceed even if there is no memory free. There is a comment "TODO: this could cause a theoretical memory reclaim deadlock in the swap out path." in the function add_to_swap - but apart from that, swap should proceed even with no available memory, as long as all the drivers in the block layer use mempools. > > So, it could prematurely trigger OOM killer on any slow swapping device > > (including dm-crypt). Michal Hocko reworked the OOM killer in the patch > > 0a0337e0d1d134465778a16f5cbea95086e8e9e0, but it still has the flaw that > > it triggers OOM if there is plenty of free swap space free. > > > > Michal, would you accept a change to the OOM killer, to prevent it from > > triggerring when there is free swap space? > > No this doesn't sound like a proper solution. The current decision > logic, as explained above relies on the feedback from the reclaim. A > free swap space doesn't really mean we can make a forward progress. I'm interested - why would you need to trigger the OOM killer if there is free swap space? The only possibility is that all the memory is filled with unswappable kernel pages - but that condition could be detected if there is unusually low number of anonymous and cache pages. Besides that - in what situation is triggering the OOM killer with free swap desired? > -- > Michal Hocko > SUSE Labs > The kernel 4.7-rc almost deadlocks in another way. The machine got stuck and the following stacktrace was obtained when swapping to dm-crypt. We can see that dm-crypt does a mempool allocation. But the mempool allocation somehow falls into throttle_vm_writeout. There, it waits for 0.1 seconds. So, as a result, the dm-crypt worker thread ends up processing requests at an unusually slow rate of 10 requests per second and it results in the machine being stuck (it would proabably recover if we waited for extreme amount of time). [ 345.352536] kworker/u4:0 D ffff88003df7f438 10488 6 2 0x00000000 [ 345.352536] Workqueue: kcryptd kcryptd_crypt [dm_crypt] [ 345.352536] ffff88003df7f438 ffff88003e5d0380 ffff88003e5d0380 ffff88003e5d8e80 [ 345.352536] ffff88003dfb3240 ffff88003df73240 ffff88003df80000 ffff88003df7f470 [ 345.352536] ffff88003e5d0380 ffff88003e5d0380 ffff88003df7f828 ffff88003df7f450 [ 345.352536] Call Trace: [ 345.352536] [<ffffffff818d466c>] schedule+0x3c/0x90 [ 345.352536] [<ffffffff818d96a8>] schedule_timeout+0x1d8/0x360 [ 345.352536] [<ffffffff81135e40>] ? detach_if_pending+0x1c0/0x1c0 [ 345.352536] [<ffffffff811407c3>] ? ktime_get+0xb3/0x150 [ 345.352536] [<ffffffff811958cf>] ? __delayacct_blkio_start+0x1f/0x30 [ 345.352536] [<ffffffff818d39e4>] io_schedule_timeout+0xa4/0x110 [ 345.352536] [<ffffffff8121d886>] congestion_wait+0x86/0x1f0 [ 345.352536] [<ffffffff810fdf40>] ? prepare_to_wait_event+0xf0/0xf0 [ 345.352536] [<ffffffff812061d4>] throttle_vm_writeout+0x44/0xd0 [ 345.352536] [<ffffffff81211533>] shrink_zone_memcg+0x613/0x720 [ 345.352536] [<ffffffff81211720>] shrink_zone+0xe0/0x300 [ 345.352536] [<ffffffff81211aed>] do_try_to_free_pages+0x1ad/0x450 [ 345.352536] [<ffffffff81211e7f>] try_to_free_pages+0xef/0x300 [ 345.352536] [<ffffffff811fef19>] __alloc_pages_nodemask+0x879/0x1210 [ 345.352536] [<ffffffff810e8080>] ? sched_clock_cpu+0x90/0xc0 [ 345.352536] [<ffffffff8125a8d1>] alloc_pages_current+0xa1/0x1f0 [ 345.352536] [<ffffffff81265ef5>] ? new_slab+0x3f5/0x6a0 [ 345.352536] [<ffffffff81265dd7>] new_slab+0x2d7/0x6a0 [ 345.352536] [<ffffffff810e7f87>] ? sched_clock_local+0x17/0x80 [ 345.352536] [<ffffffff812678cb>] ___slab_alloc+0x3fb/0x5c0 [ 345.352536] [<ffffffff811f71bd>] ? mempool_alloc_slab+0x1d/0x30 [ 345.352536] [<ffffffff810e7f87>] ? sched_clock_local+0x17/0x80 [ 345.352536] [<ffffffff811f71bd>] ? mempool_alloc_slab+0x1d/0x30 [ 345.352536] [<ffffffff81267ae1>] __slab_alloc+0x51/0x90 [ 345.352536] [<ffffffff811f71bd>] ? mempool_alloc_slab+0x1d/0x30 [ 345.352536] [<ffffffff81267d9b>] kmem_cache_alloc+0x27b/0x310 [ 345.352536] [<ffffffff811f71bd>] mempool_alloc_slab+0x1d/0x30 [ 345.352536] [<ffffffff811f6f11>] mempool_alloc+0x91/0x230 [ 345.352536] [<ffffffff8141a02d>] bio_alloc_bioset+0xbd/0x260 [ 345.352536] [<ffffffffc02f1a54>] kcryptd_crypt+0x114/0x3b0 [dm_crypt] [ 345.352536] [<ffffffff810cc312>] process_one_work+0x242/0x700 [ 345.352536] [<ffffffff810cc28a>] ? process_one_work+0x1ba/0x700 [ 345.352536] [<ffffffff810cc81e>] worker_thread+0x4e/0x490 [ 345.352536] [<ffffffff810cc7d0>] ? process_one_work+0x700/0x700 [ 345.352536] [<ffffffff810d3c01>] kthread+0x101/0x120 [ 345.352536] [<ffffffff8110b9f5>] ? trace_hardirqs_on_caller+0xf5/0x1b0 [ 345.352536] [<ffffffff818db1af>] ret_from_fork+0x1f/0x40 [ 345.352536] [<ffffffff810d3b00>] ? kthread_create_on_node+0x250/0x250
[toc] | [prev] | [next] | [standalone]
| From | Jerome Marchand <jmarchan@redhat.com> |
|---|---|
| Date | 2016-07-13 10:40 +0200 |
| Message-ID | <rUnS2-vb-17@gated-at.bofh.it> |
| In reply to | #1441921 |
[Multipart message — attachments visible in raw view] — view raw
On 07/13/2016 01:44 AM, Mikulas Patocka wrote: > The problem of swapping to dm-crypt is this. > > The free memory goes low, kswapd decides that some page should be swapped > out. However, when you swap to an ecrypted device, writeback of each page > requires another page to hold the encrypted data. dm-crypt uses mempools > for all its structures and pages, so that it can make forward progress > even if there is no memory free. However, the mempool code first allocates > from general memory allocator and resorts to the mempool only if the > memory is below limit. > > So every attempt to swap out some page allocates another page. > > As long as swapping is in progress, the free memory is below the limit > (because the swapping activity itself consumes any memory over the limit). > And that triggered the OOM killer prematurely. There is a quite recent sysctl vm knob that I believe can help in this case: watermark_scale_factor. If you increase this value, kswapd will start paging out earlier, when there might still be enough free memory. Ondrej, have you tried to increase /proc/sys/vm/watermark_scale_factor? Jerome > > > On Tue, 12 Jul 2016, Michal Hocko wrote: > >> On Mon 11-07-16 11:43:02, Mikulas Patocka wrote: >> [...] >>> The general problem is that the memory allocator does 16 retries to >>> allocate a page and then triggers the OOM killer (and it doesn't take into >>> account how much swap space is free or how many dirty pages were really >>> swapped out while it waited). >> >> Well, that is not how it works exactly. We retry as long as there is a >> reclaim progress (at least one page freed) back off only if the >> reclaimable memory can exceed watermks which is scaled down in 16 >> retries. The overal size of free swap is not really that important if we >> cannot swap out like here due to complete memory reserves depletion: >> https://okozina.fedorapeople.org/bugs/swap_on_dmcrypt/vmlog-1462458369-00000/sample-00011/dmesg: >> [ 90.491276] Node 0 DMA free:0kB min:60kB low:72kB high:84kB active_anon:4096kB inactive_anon:4636kB active_file:212kB inactive_file:280kB unevictable:488kB isolated(anon):0kB isolated(file):0kB present:15992kB managed:15908kB mlocked:488kB dirty:276kB writeback:4636kB mapped:476kB shmem:12kB slab_reclaimable:204kB slab_unreclaimable:4700kB kernel_stack:48kB pagetables:120kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:61132 all_unreclaimable? yes >> [ 90.491283] lowmem_reserve[]: 0 977 977 977 >> [ 90.491286] Node 0 DMA32 free:0kB min:3828kB low:4824kB high:5820kB active_anon:423820kB inactive_anon:424916kB active_file:17996kB inactive_file:21800kB unevictable:20724kB isolated(anon):384kB isolated(file):0kB present:1032184kB managed:1001260kB mlocked:20724kB dirty:25236kB writeback:49972kB mapped:23076kB shmem:1364kB slab_reclaimable:13796kB slab_unreclaimable:43008kB kernel_stack:2816kB pagetables:7320kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:5635400 all_unreclaimable? yes >> >> Look at the amount of free memory. It is completely depleted. So it >> smells like a process which has access to memory reserves has consumed >> all of it. I suspect a __GFP_MEMALLOC resp. PF_MEMALLOC from softirq >> context user which went off the leash. > > It is caused by the commit f9054c70d28bc214b2857cf8db8269f4f45a5e23. Prior > to this commit, mempool allocations set __GFP_NOMEMALLOC, so they never > exhausted reserved memory. With this commit, mempool allocations drop > __GFP_NOMEMALLOC, so they can dig deeper (if the process has PF_MEMALLOC, > they can bypass all limits). > > But swapping should proceed even if there is no memory free. There is a > comment "TODO: this could cause a theoretical memory reclaim deadlock in > the swap out path." in the function add_to_swap - but apart from that, > swap should proceed even with no available memory, as long as all the > drivers in the block layer use mempools. > >>> So, it could prematurely trigger OOM killer on any slow swapping device >>> (including dm-crypt). Michal Hocko reworked the OOM killer in the patch >>> 0a0337e0d1d134465778a16f5cbea95086e8e9e0, but it still has the flaw that >>> it triggers OOM if there is plenty of free swap space free. >>> >>> Michal, would you accept a change to the OOM killer, to prevent it from >>> triggerring when there is free swap space? >> >> No this doesn't sound like a proper solution. The current decision >> logic, as explained above relies on the feedback from the reclaim. A >> free swap space doesn't really mean we can make a forward progress. > > I'm interested - why would you need to trigger the OOM killer if there is > free swap space? > > The only possibility is that all the memory is filled with unswappable > kernel pages - but that condition could be detected if there is unusually > low number of anonymous and cache pages. Besides that - in what situation > is triggering the OOM killer with free swap desired? > >> -- >> Michal Hocko >> SUSE Labs >> > > The kernel 4.7-rc almost deadlocks in another way. The machine got stuck > and the following stacktrace was obtained when swapping to dm-crypt. > > We can see that dm-crypt does a mempool allocation. But the mempool > allocation somehow falls into throttle_vm_writeout. There, it waits for > 0.1 seconds. So, as a result, the dm-crypt worker thread ends up > processing requests at an unusually slow rate of 10 requests per second > and it results in the machine being stuck (it would proabably recover if > we waited for extreme amount of time). > > [ 345.352536] kworker/u4:0 D ffff88003df7f438 10488 6 2 0x00000000 > [ 345.352536] Workqueue: kcryptd kcryptd_crypt [dm_crypt] > [ 345.352536] ffff88003df7f438 ffff88003e5d0380 ffff88003e5d0380 ffff88003e5d8e80 > [ 345.352536] ffff88003dfb3240 ffff88003df73240 ffff88003df80000 ffff88003df7f470 > [ 345.352536] ffff88003e5d0380 ffff88003e5d0380 ffff88003df7f828 ffff88003df7f450 > [ 345.352536] Call Trace: > [ 345.352536] [<ffffffff818d466c>] schedule+0x3c/0x90 > [ 345.352536] [<ffffffff818d96a8>] schedule_timeout+0x1d8/0x360 > [ 345.352536] [<ffffffff81135e40>] ? detach_if_pending+0x1c0/0x1c0 > [ 345.352536] [<ffffffff811407c3>] ? ktime_get+0xb3/0x150 > [ 345.352536] [<ffffffff811958cf>] ? __delayacct_blkio_start+0x1f/0x3 0 > [ 345.352536] [<ffffffff818d39e4>] io_schedule_timeout+0xa4/0x110 > [ 345.352536] [<ffffffff8121d886>] congestion_wait+0x86/0x1f0 > [ 345.352536] [<ffffffff810fdf40>] ? prepare_to_wait_event+0xf0/0xf0 > [ 345.352536] [<ffffffff812061d4>] throttle_vm_writeout+0x44/0xd0 > [ 345.352536] [<ffffffff81211533>] shrink_zone_memcg+0x613/0x720 > [ 345.352536] [<ffffffff81211720>] shrink_zone+0xe0/0x300 > [ 345.352536] [<ffffffff81211aed>] do_try_to_free_pages+0x1ad/0x450 > [ 345.352536] [<ffffffff81211e7f>] try_to_free_pages+0xef/0x300 > [ 345.352536] [<ffffffff811fef19>] __alloc_pages_nodemask+0x879/0x121 0 > [ 345.352536] [<ffffffff810e8080>] ? sched_clock_cpu+0x90/0xc0 > [ 345.352536] [<ffffffff8125a8d1>] alloc_pages_current+0xa1/0x1f0 > [ 345.352536] [<ffffffff81265ef5>] ? new_slab+0x3f5/0x6a0 > [ 345.352536] [<ffffffff81265dd7>] new_slab+0x2d7/0x6a0 > [ 345.352536] [<ffffffff810e7f87>] ? sched_clock_local+0x17/0x80 > [ 345.352536] [<ffffffff812678cb>] ___slab_alloc+0x3fb/0x5c0 > [ 345.352536] [<ffffffff811f71bd>] ? mempool_alloc_slab+0x1d/0x30 > [ 345.352536] [<ffffffff810e7f87>] ? sched_clock_local+0x17/0x80 > [ 345.352536] [<ffffffff811f71bd>] ? mempool_alloc_slab+0x1d/0x30 > [ 345.352536] [<ffffffff81267ae1>] __slab_alloc+0x51/0x90 > [ 345.352536] [<ffffffff811f71bd>] ? mempool_alloc_slab+0x1d/0x30 > [ 345.352536] [<ffffffff81267d9b>] kmem_cache_alloc+0x27b/0x310 > [ 345.352536] [<ffffffff811f71bd>] mempool_alloc_slab+0x1d/0x30 > [ 345.352536] [<ffffffff811f6f11>] mempool_alloc+0x91/0x230 > [ 345.352536] [<ffffffff8141a02d>] bio_alloc_bioset+0xbd/0x260 > [ 345.352536] [<ffffffffc02f1a54>] kcryptd_crypt+0x114/0x3b0 [dm_crypt] > [ 345.352536] [<ffffffff810cc312>] process_one_work+0x242/0x700 > [ 345.352536] [<ffffffff810cc28a>] ? process_one_work+0x1ba/0x700 > [ 345.352536] [<ffffffff810cc81e>] worker_thread+0x4e/0x490 > [ 345.352536] [<ffffffff810cc7d0>] ? process_one_work+0x700/0x700 > [ 345.352536] [<ffffffff810d3c01>] kthread+0x101/0x120 > [ 345.352536] [<ffffffff8110b9f5>] ? trace_hardirqs_on_caller+0xf5/0x1b0 > [ 345.352536] [<ffffffff818db1af>] ret_from_fork+0x1f/0x40 > [ 345.352536] [<ffffffff810d3b00>] ? kthread_create_on_node+0x250/0x250 >
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-07-13 13:20 +0200 |
| Message-ID | <rUqmS-2l8-13@gated-at.bofh.it> |
| In reply to | #1442173 |
On Wed 13-07-16 10:35:01, Jerome Marchand wrote: > On 07/13/2016 01:44 AM, Mikulas Patocka wrote: > > The problem of swapping to dm-crypt is this. > > > > The free memory goes low, kswapd decides that some page should be swapped > > out. However, when you swap to an ecrypted device, writeback of each page > > requires another page to hold the encrypted data. dm-crypt uses mempools > > for all its structures and pages, so that it can make forward progress > > even if there is no memory free. However, the mempool code first allocates > > from general memory allocator and resorts to the mempool only if the > > memory is below limit. > > > > So every attempt to swap out some page allocates another page. > > > > As long as swapping is in progress, the free memory is below the limit > > (because the swapping activity itself consumes any memory over the limit). > > And that triggered the OOM killer prematurely. > > There is a quite recent sysctl vm knob that I believe can help in this > case: watermark_scale_factor. If you increase this value, kswapd will > start paging out earlier, when there might still be enough free memory. > > Ondrej, have you tried to increase /proc/sys/vm/watermark_scale_factor? I suspect this would just change the timing or the real problem gets hidden. -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Mikulas Patocka <mpatocka@redhat.com> |
|---|---|
| Date | 2016-07-13 16:30 +0200 |
| Message-ID | <rUtkK-4fl-21@gated-at.bofh.it> |
| In reply to | #1442348 |
On Wed, 13 Jul 2016, Michal Hocko wrote: > On Wed 13-07-16 10:35:01, Jerome Marchand wrote: > > On 07/13/2016 01:44 AM, Mikulas Patocka wrote: > > > The problem of swapping to dm-crypt is this. > > > > > > The free memory goes low, kswapd decides that some page should be swapped > > > out. However, when you swap to an ecrypted device, writeback of each page > > > requires another page to hold the encrypted data. dm-crypt uses mempools > > > for all its structures and pages, so that it can make forward progress > > > even if there is no memory free. However, the mempool code first allocates > > > from general memory allocator and resorts to the mempool only if the > > > memory is below limit. > > > > > > So every attempt to swap out some page allocates another page. > > > > > > As long as swapping is in progress, the free memory is below the limit > > > (because the swapping activity itself consumes any memory over the limit). > > > And that triggered the OOM killer prematurely. > > > > There is a quite recent sysctl vm knob that I believe can help in this > > case: watermark_scale_factor. If you increase this value, kswapd will > > start paging out earlier, when there might still be enough free memory. > > > > Ondrej, have you tried to increase /proc/sys/vm/watermark_scale_factor? > > I suspect this would just change the timing or the real problem gets > hidden. I agree - tweaking some limits would just change the probability of the bug without addressing the root cause. We shouldn't tweak anything and just stick to Ondrej's scenario where he reproduced the bug. Mikulas > -- > Michal Hocko > SUSE Labs >
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-07-13 13:20 +0200 |
| Message-ID | <rUqmS-2l8-15@gated-at.bofh.it> |
| In reply to | #1441921 |
On Tue 12-07-16 19:44:11, Mikulas Patocka wrote:
> The problem of swapping to dm-crypt is this.
>
> The free memory goes low, kswapd decides that some page should be swapped
> out. However, when you swap to an ecrypted device, writeback of each page
> requires another page to hold the encrypted data. dm-crypt uses mempools
> for all its structures and pages, so that it can make forward progress
> even if there is no memory free. However, the mempool code first allocates
> from general memory allocator and resorts to the mempool only if the
> memory is below limit.
OK, thanks for the clarification. I guess the core part happens in
crypt_alloc_buffer, right?
> So every attempt to swap out some page allocates another page.
>
> As long as swapping is in progress, the free memory is below the limit
> (because the swapping activity itself consumes any memory over the limit).
> And that triggered the OOM killer prematurely.
I am not sure I understand the last part. Are you saing that we trigger
OOM because the initiated swapout will not be able to finish the IO thus
release the page in time?
The oom detection checks waits for an ongoing writeout if there is no
reclaim progress and at least half of the reclaimable memory is either
dirty or under writeback. Pages under swaout are marked as under
writeback AFAIR. The writeout path (dm-crypt worker in this case) should
be able to allocate a memory from the mempool, hand over to the crypt
layer and finish the IO. Is it possible this might take a lot of time?
> On Tue, 12 Jul 2016, Michal Hocko wrote:
>
> > On Mon 11-07-16 11:43:02, Mikulas Patocka wrote:
> > [...]
> > > The general problem is that the memory allocator does 16 retries to
> > > allocate a page and then triggers the OOM killer (and it doesn't take into
> > > account how much swap space is free or how many dirty pages were really
> > > swapped out while it waited).
> >
> > Well, that is not how it works exactly. We retry as long as there is a
> > reclaim progress (at least one page freed) back off only if the
> > reclaimable memory can exceed watermks which is scaled down in 16
> > retries. The overal size of free swap is not really that important if we
> > cannot swap out like here due to complete memory reserves depletion:
> > https://okozina.fedorapeople.org/bugs/swap_on_dmcrypt/vmlog-1462458369-00000/sample-00011/dmesg:
> > [ 90.491276] Node 0 DMA free:0kB min:60kB low:72kB high:84kB active_anon:4096kB inactive_anon:4636kB active_file:212kB inactive_file:280kB unevictable:488kB isolated(anon):0kB isolated(file):0kB present:15992kB managed:15908kB mlocked:488kB dirty:276kB writeback:4636kB mapped:476kB shmem:12kB slab_reclaimable:204kB slab_unreclaimable:4700kB kernel_stack:48kB pagetables:120kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:61132 all_unreclaimable? yes
> > [ 90.491283] lowmem_reserve[]: 0 977 977 977
> > [ 90.491286] Node 0 DMA32 free:0kB min:3828kB low:4824kB high:5820kB active_anon:423820kB inactive_anon:424916kB active_file:17996kB inactive_file:21800kB unevictable:20724kB isolated(anon):384kB isolated(file):0kB present:1032184kB managed:1001260kB mlocked:20724kB dirty:25236kB writeback:49972kB mapped:23076kB shmem:1364kB slab_reclaimable:13796kB slab_unreclaimable:43008kB kernel_stack:2816kB pagetables:7320kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:5635400 all_unreclaimable? yes
> >
> > Look at the amount of free memory. It is completely depleted. So it
> > smells like a process which has access to memory reserves has consumed
> > all of it. I suspect a __GFP_MEMALLOC resp. PF_MEMALLOC from softirq
> > context user which went off the leash.
>
> It is caused by the commit f9054c70d28bc214b2857cf8db8269f4f45a5e23. Prior
> to this commit, mempool allocations set __GFP_NOMEMALLOC, so they never
> exhausted reserved memory. With this commit, mempool allocations drop
> __GFP_NOMEMALLOC, so they can dig deeper (if the process has PF_MEMALLOC,
> they can bypass all limits).
Hmm, but the patch allows access to the memory reserves only when the
pool is empty. And even then the caller would have to request access to
reserves explicitly either by __GFP_NOMEMALLOC or PF_MEMALLOC. That
doesn't seem to be the case for the dm-crypt, though. Or do you suspect
that some other mempool user might be doing so?
> But swapping should proceed even if there is no memory free. There is a
> comment "TODO: this could cause a theoretical memory reclaim deadlock in
> the swap out path." in the function add_to_swap - but apart from that,
> swap should proceed even with no available memory, as long as all the
> drivers in the block layer use mempools.
>
> > > So, it could prematurely trigger OOM killer on any slow swapping device
> > > (including dm-crypt). Michal Hocko reworked the OOM killer in the patch
> > > 0a0337e0d1d134465778a16f5cbea95086e8e9e0, but it still has the flaw that
> > > it triggers OOM if there is plenty of free swap space free.
> > >
> > > Michal, would you accept a change to the OOM killer, to prevent it from
> > > triggerring when there is free swap space?
> >
> > No this doesn't sound like a proper solution. The current decision
> > logic, as explained above relies on the feedback from the reclaim. A
> > free swap space doesn't really mean we can make a forward progress.
>
> I'm interested - why would you need to trigger the OOM killer if there is
> free swap space?
Let me clarify. If there is a swapable memory then we shouldn't trigger
the OOM killer normally of course. And that should be the case with the
current implementation. We just rely on the swapout making some progress
and back off only if that is not the case after several attempts with a
throttling based on the writeback counters. Checking the available swap
space doesn't guarantee a forward progress, though. If the swap out is
stuck for some reason then it should be safer to trigger to OOM rather
than wait or trash for ever (or an excessive amount of time).
Now, I can see that the retry logic might need some tuning for complex
setups like dm-crypt swap partitions because the progress might be much
slower there. But I would like the understand what is the worst estimate
for the swapout path with all the roadblocks on the way for this setup
before we can think of a proper retry logic tuning.
> The only possibility is that all the memory is filled with unswappable
> kernel pages - but that condition could be detected if there is unusually
> low number of anonymous and cache pages. Besides that - in what situation
> is triggering the OOM killer with free swap desired?
I hope the above has explained that.
> The kernel 4.7-rc almost deadlocks in another way. The machine got stuck
> and the following stacktrace was obtained when swapping to dm-crypt.
>
> We can see that dm-crypt does a mempool allocation. But the mempool
> allocation somehow falls into throttle_vm_writeout. There, it waits for
> 0.1 seconds. So, as a result, the dm-crypt worker thread ends up
> processing requests at an unusually slow rate of 10 requests per second
> and it results in the machine being stuck (it would proabably recover if
> we waited for extreme amount of time).
Hmm, that throttling is there since ever basically. I do not see what
would have changed that recently, but I haven't looked too close to be
honest.
I agree that throttling a flusher (which this worker definitely is)
doesn't look like a correct thing to do. We have PF_LESS_THROTTLE for
this kind of things. So maybe the right thing to do is to use this flag
for the dm_crypt worker:
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 4f3cb3554944..0b806810efab 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1392,11 +1392,14 @@ static void kcryptd_async_done(struct crypto_async_request *async_req,
static void kcryptd_crypt(struct work_struct *work)
{
struct dm_crypt_io *io = container_of(work, struct dm_crypt_io, work);
+ unsigned int pflags = current->flags;
+ current->flags |= PF_LESS_THROTTLE;
if (bio_data_dir(io->base_bio) == READ)
kcryptd_crypt_read_convert(io);
else
kcryptd_crypt_write_convert(io);
+ tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
}
static void kcryptd_queue_crypt(struct dm_crypt_io *io)
>
> [ 345.352536] kworker/u4:0 D ffff88003df7f438 10488 6 2 0x00000000
> [ 345.352536] Workqueue: kcryptd kcryptd_crypt [dm_crypt]
> [ 345.352536] ffff88003df7f438 ffff88003e5d0380 ffff88003e5d0380 ffff88003e5d8e80
> [ 345.352536] ffff88003dfb3240 ffff88003df73240 ffff88003df80000 ffff88003df7f470
> [ 345.352536] ffff88003e5d0380 ffff88003e5d0380 ffff88003df7f828 ffff88003df7f450
> [ 345.352536] Call Trace:
> [ 345.352536] [<ffffffff818d466c>] schedule+0x3c/0x90
> [ 345.352536] [<ffffffff818d96a8>] schedule_timeout+0x1d8/0x360
> [ 345.352536] [<ffffffff81135e40>] ? detach_if_pending+0x1c0/0x1c0
> [ 345.352536] [<ffffffff811407c3>] ? ktime_get+0xb3/0x150
> [ 345.352536] [<ffffffff811958cf>] ? __delayacct_blkio_start+0x1f/0x30
> [ 345.352536] [<ffffffff818d39e4>] io_schedule_timeout+0xa4/0x110
> [ 345.352536] [<ffffffff8121d886>] congestion_wait+0x86/0x1f0
> [ 345.352536] [<ffffffff810fdf40>] ? prepare_to_wait_event+0xf0/0xf0
> [ 345.352536] [<ffffffff812061d4>] throttle_vm_writeout+0x44/0xd0
> [ 345.352536] [<ffffffff81211533>] shrink_zone_memcg+0x613/0x720
> [ 345.352536] [<ffffffff81211720>] shrink_zone+0xe0/0x300
> [ 345.352536] [<ffffffff81211aed>] do_try_to_free_pages+0x1ad/0x450
> [ 345.352536] [<ffffffff81211e7f>] try_to_free_pages+0xef/0x300
> [ 345.352536] [<ffffffff811fef19>] __alloc_pages_nodemask+0x879/0x1210
> [ 345.352536] [<ffffffff810e8080>] ? sched_clock_cpu+0x90/0xc0
> [ 345.352536] [<ffffffff8125a8d1>] alloc_pages_current+0xa1/0x1f0
> [ 345.352536] [<ffffffff81265ef5>] ? new_slab+0x3f5/0x6a0
> [ 345.352536] [<ffffffff81265dd7>] new_slab+0x2d7/0x6a0
> [ 345.352536] [<ffffffff810e7f87>] ? sched_clock_local+0x17/0x80
> [ 345.352536] [<ffffffff812678cb>] ___slab_alloc+0x3fb/0x5c0
> [ 345.352536] [<ffffffff811f71bd>] ? mempool_alloc_slab+0x1d/0x30
> [ 345.352536] [<ffffffff810e7f87>] ? sched_clock_local+0x17/0x80
> [ 345.352536] [<ffffffff811f71bd>] ? mempool_alloc_slab+0x1d/0x30
> [ 345.352536] [<ffffffff81267ae1>] __slab_alloc+0x51/0x90
> [ 345.352536] [<ffffffff811f71bd>] ? mempool_alloc_slab+0x1d/0x30
> [ 345.352536] [<ffffffff81267d9b>] kmem_cache_alloc+0x27b/0x310
> [ 345.352536] [<ffffffff811f71bd>] mempool_alloc_slab+0x1d/0x30
> [ 345.352536] [<ffffffff811f6f11>] mempool_alloc+0x91/0x230
> [ 345.352536] [<ffffffff8141a02d>] bio_alloc_bioset+0xbd/0x260
> [ 345.352536] [<ffffffffc02f1a54>] kcryptd_crypt+0x114/0x3b0 [dm_crypt]
> [ 345.352536] [<ffffffff810cc312>] process_one_work+0x242/0x700
> [ 345.352536] [<ffffffff810cc28a>] ? process_one_work+0x1ba/0x700
> [ 345.352536] [<ffffffff810cc81e>] worker_thread+0x4e/0x490
> [ 345.352536] [<ffffffff810cc7d0>] ? process_one_work+0x700/0x700
> [ 345.352536] [<ffffffff810d3c01>] kthread+0x101/0x120
> [ 345.352536] [<ffffffff8110b9f5>] ? trace_hardirqs_on_caller+0xf5/0x1b0
> [ 345.352536] [<ffffffff818db1af>] ret_from_fork+0x1f/0x40
> [ 345.352536] [<ffffffff810d3b00>] ? kthread_create_on_node+0x250/0x250
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-07-13 15:00 +0200 |
| Message-ID | <rUrVE-3bS-33@gated-at.bofh.it> |
| In reply to | #1442349 |
On Wed 13-07-16 13:10:06, Michal Hocko wrote: > On Tue 12-07-16 19:44:11, Mikulas Patocka wrote: [...] > > As long as swapping is in progress, the free memory is below the limit > > (because the swapping activity itself consumes any memory over the limit). > > And that triggered the OOM killer prematurely. > > I am not sure I understand the last part. Are you saing that we trigger > OOM because the initiated swapout will not be able to finish the IO thus > release the page in time? > > The oom detection checks waits for an ongoing writeout if there is no > reclaim progress and at least half of the reclaimable memory is either > dirty or under writeback. Pages under swaout are marked as under > writeback AFAIR. The writeout path (dm-crypt worker in this case) should > be able to allocate a memory from the mempool, hand over to the crypt > layer and finish the IO. Is it possible this might take a lot of time? I am not familiar with the crypto API but from what I understood from crypt_convert the encryption is done asynchronously. Then I got lost in the indirection. Who is completing the request and from what kind of context? Is it possible it wouldn't be runable for a long time? -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Milan Broz <gmazyland@gmail.com> |
|---|---|
| Date | 2016-07-13 15:50 +0200 |
| Message-ID | <rUsI2-3Kr-19@gated-at.bofh.it> |
| In reply to | #1442424 |
On 07/13/2016 02:50 PM, Michal Hocko wrote: > On Wed 13-07-16 13:10:06, Michal Hocko wrote: >> On Tue 12-07-16 19:44:11, Mikulas Patocka wrote: > [...] >>> As long as swapping is in progress, the free memory is below the limit >>> (because the swapping activity itself consumes any memory over the limit). >>> And that triggered the OOM killer prematurely. >> >> I am not sure I understand the last part. Are you saing that we trigger >> OOM because the initiated swapout will not be able to finish the IO thus >> release the page in time? >> >> The oom detection checks waits for an ongoing writeout if there is no >> reclaim progress and at least half of the reclaimable memory is either >> dirty or under writeback. Pages under swaout are marked as under >> writeback AFAIR. The writeout path (dm-crypt worker in this case) should >> be able to allocate a memory from the mempool, hand over to the crypt >> layer and finish the IO. Is it possible this might take a lot of time? > > I am not familiar with the crypto API but from what I understood from > crypt_convert the encryption is done asynchronously. Then I got lost in > the indirection. Who is completing the request and from what kind of > context? Is it possible it wouldn't be runable for a long time? If you mean crypt_convert in dm-crypt, then it can do asynchronous completion but usually (with AES-NI ans sw implementations) it run the operation completely synchronously. Asynchronous processing is quite rare, usually only on some specific hardware crypto accelerators. Once the encryption is finished, the cloned bio is sent to the block layer for processing. (There is also some magic with sorting writes but Mikulas knows this better.) Milan p.s. I added cc to dm-devel, some dmcrypt people reads only this list.
[toc] | [prev] | [next] | [standalone]
| From | Mikulas Patocka <mpatocka@redhat.com> |
|---|---|
| Date | 2016-07-13 17:40 +0200 |
| Message-ID | <rUuqt-4Vy-17@gated-at.bofh.it> |
| In reply to | #1442472 |
On Wed, 13 Jul 2016, Milan Broz wrote: > On 07/13/2016 02:50 PM, Michal Hocko wrote: > > On Wed 13-07-16 13:10:06, Michal Hocko wrote: > >> On Tue 12-07-16 19:44:11, Mikulas Patocka wrote: > > [...] > >>> As long as swapping is in progress, the free memory is below the limit > >>> (because the swapping activity itself consumes any memory over the limit). > >>> And that triggered the OOM killer prematurely. > >> > >> I am not sure I understand the last part. Are you saing that we trigger > >> OOM because the initiated swapout will not be able to finish the IO thus > >> release the page in time? > >> > >> The oom detection checks waits for an ongoing writeout if there is no > >> reclaim progress and at least half of the reclaimable memory is either > >> dirty or under writeback. Pages under swaout are marked as under > >> writeback AFAIR. The writeout path (dm-crypt worker in this case) should > >> be able to allocate a memory from the mempool, hand over to the crypt > >> layer and finish the IO. Is it possible this might take a lot of time? > > > > I am not familiar with the crypto API but from what I understood from > > crypt_convert the encryption is done asynchronously. Then I got lost in > > the indirection. Who is completing the request and from what kind of > > context? Is it possible it wouldn't be runable for a long time? > > If you mean crypt_convert in dm-crypt, then it can do asynchronous completion > but usually (with AES-NI ans sw implementations) it run the operation completely > synchronously. > Asynchronous processing is quite rare, usually only on some specific hardware > crypto accelerators. > > Once the encryption is finished, the cloned bio is sent to the block > layer for processing. > (There is also some magic with sorting writes but Mikulas knows this better.) dm-crypt receives requests in crypt_map, then it distributes write requests to multiple encryption threads. Encryption is done usually synchronously; asynchronous completion is used only when using some PCI cards that accelerate encryption. When encryption finishes, the encrypted pages are submitted to a thread dmcrypt_write that sorts the requests using rbtree and submits them. The block layer has a deficiency that it cannot merge adjacent requests submitted by the different threads. If we submitted requests directly from encryption threads, lack of merging degraded performance seriously. Mikulas > Milan > p.s. I added cc to dm-devel, some dmcrypt people reads only this list. >
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-07-14 11:10 +0200 |
| Message-ID | <rUKOC-7Jy-25@gated-at.bofh.it> |
| In reply to | #1442557 |
On Wed 13-07-16 11:21:41, Mikulas Patocka wrote: > > > On Wed, 13 Jul 2016, Milan Broz wrote: > > > On 07/13/2016 02:50 PM, Michal Hocko wrote: > > > On Wed 13-07-16 13:10:06, Michal Hocko wrote: > > >> On Tue 12-07-16 19:44:11, Mikulas Patocka wrote: > > > [...] > > >>> As long as swapping is in progress, the free memory is below the limit > > >>> (because the swapping activity itself consumes any memory over the limit). > > >>> And that triggered the OOM killer prematurely. > > >> > > >> I am not sure I understand the last part. Are you saing that we trigger > > >> OOM because the initiated swapout will not be able to finish the IO thus > > >> release the page in time? > > >> > > >> The oom detection checks waits for an ongoing writeout if there is no > > >> reclaim progress and at least half of the reclaimable memory is either > > >> dirty or under writeback. Pages under swaout are marked as under > > >> writeback AFAIR. The writeout path (dm-crypt worker in this case) should > > >> be able to allocate a memory from the mempool, hand over to the crypt > > >> layer and finish the IO. Is it possible this might take a lot of time? > > > > > > I am not familiar with the crypto API but from what I understood from > > > crypt_convert the encryption is done asynchronously. Then I got lost in > > > the indirection. Who is completing the request and from what kind of > > > context? Is it possible it wouldn't be runable for a long time? > > > > If you mean crypt_convert in dm-crypt, then it can do asynchronous completion > > but usually (with AES-NI ans sw implementations) it run the operation completely > > synchronously. > > Asynchronous processing is quite rare, usually only on some specific hardware > > crypto accelerators. > > > > Once the encryption is finished, the cloned bio is sent to the block > > layer for processing. > > (There is also some magic with sorting writes but Mikulas knows this better.) > > dm-crypt receives requests in crypt_map, then it distributes write > requests to multiple encryption threads. Encryption is done usually > synchronously; asynchronous completion is used only when using some PCI > cards that accelerate encryption. When encryption finishes, the encrypted > pages are submitted to a thread dmcrypt_write that sorts the requests > using rbtree and submits them. OK. I was worried that the async context would depend on WQ and a lack of workers could lead to long stalls. Dedicated kernel threads seem sufficient. Thanks for the clarification. -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Milan Broz <gmazyland@gmail.com> |
|---|---|
| Date | 2016-07-14 11:50 +0200 |
| Message-ID | <rULrj-7WZ-7@gated-at.bofh.it> |
| In reply to | #1443304 |
On 07/14/2016 11:09 AM, Michal Hocko wrote: > On Wed 13-07-16 11:21:41, Mikulas Patocka wrote: >> >> >> On Wed, 13 Jul 2016, Milan Broz wrote: >> >>> On 07/13/2016 02:50 PM, Michal Hocko wrote: >>>> On Wed 13-07-16 13:10:06, Michal Hocko wrote: >>>>> On Tue 12-07-16 19:44:11, Mikulas Patocka wrote: >>>> [...] >>>>>> As long as swapping is in progress, the free memory is below the limit >>>>>> (because the swapping activity itself consumes any memory over the limit). >>>>>> And that triggered the OOM killer prematurely. >>>>> >>>>> I am not sure I understand the last part. Are you saing that we trigger >>>>> OOM because the initiated swapout will not be able to finish the IO thus >>>>> release the page in time? >>>>> >>>>> The oom detection checks waits for an ongoing writeout if there is no >>>>> reclaim progress and at least half of the reclaimable memory is either >>>>> dirty or under writeback. Pages under swaout are marked as under >>>>> writeback AFAIR. The writeout path (dm-crypt worker in this case) should >>>>> be able to allocate a memory from the mempool, hand over to the crypt >>>>> layer and finish the IO. Is it possible this might take a lot of time? >>>> >>>> I am not familiar with the crypto API but from what I understood from >>>> crypt_convert the encryption is done asynchronously. Then I got lost in >>>> the indirection. Who is completing the request and from what kind of >>>> context? Is it possible it wouldn't be runable for a long time? >>> >>> If you mean crypt_convert in dm-crypt, then it can do asynchronous completion >>> but usually (with AES-NI ans sw implementations) it run the operation completely >>> synchronously. >>> Asynchronous processing is quite rare, usually only on some specific hardware >>> crypto accelerators. >>> >>> Once the encryption is finished, the cloned bio is sent to the block >>> layer for processing. >>> (There is also some magic with sorting writes but Mikulas knows this better.) >> >> dm-crypt receives requests in crypt_map, then it distributes write >> requests to multiple encryption threads. Encryption is done usually >> synchronously; asynchronous completion is used only when using some PCI >> cards that accelerate encryption. When encryption finishes, the encrypted >> pages are submitted to a thread dmcrypt_write that sorts the requests >> using rbtree and submits them. > > OK. I was worried that the async context would depend on WQ and a lack > of workers could lead to long stalls. Dedicated kernel threads seem > sufficient. Just for the record - if there is a suspicion that some crypto operation causes problem, dmcrypt can use null cipher. This degrades encryption/decryption to just plain memcpy inside crypto API but leaves all workqueues and tooling around the same. (I added it to cryptsetup to easily configure it and it was intended to test dmcrypt non-crypto overherad in fact.) Anyway, thanks for looking into this! Milan
[toc] | [prev] | [next] | [standalone]
| From | Mikulas Patocka <mpatocka@redhat.com> |
|---|---|
| Date | 2016-07-13 17:10 +0200 |
| Message-ID | <rUtXr-4Ju-5@gated-at.bofh.it> |
| In reply to | #1442349 |
On Wed, 13 Jul 2016, Michal Hocko wrote:
> On Tue 12-07-16 19:44:11, Mikulas Patocka wrote:
> > The problem of swapping to dm-crypt is this.
> >
> > The free memory goes low, kswapd decides that some page should be swapped
> > out. However, when you swap to an ecrypted device, writeback of each page
> > requires another page to hold the encrypted data. dm-crypt uses mempools
> > for all its structures and pages, so that it can make forward progress
> > even if there is no memory free. However, the mempool code first allocates
> > from general memory allocator and resorts to the mempool only if the
> > memory is below limit.
>
> OK, thanks for the clarification. I guess the core part happens in
> crypt_alloc_buffer, right?
>
> > So every attempt to swap out some page allocates another page.
> >
> > As long as swapping is in progress, the free memory is below the limit
> > (because the swapping activity itself consumes any memory over the limit).
> > And that triggered the OOM killer prematurely.
>
> I am not sure I understand the last part. Are you saing that we trigger
> OOM because the initiated swapout will not be able to finish the IO thus
> release the page in time?
On kernel 4.6 - premature OOM is triggered just because the free memory
stays below the limit for some time.
On kernel 4.7-rc6 (that contains your OOM patch
0a0337e0d1d134465778a16f5cbea95086e8e9e0), OOM is not triggered, but the
machine slows down to a crawl, because the allocator applies throttling to
the process that is doing the encryption.
> The oom detection checks waits for an ongoing writeout if there is no
> reclaim progress and at least half of the reclaimable memory is either
> dirty or under writeback. Pages under swaout are marked as under
> writeback AFAIR. The writeout path (dm-crypt worker in this case) should
> be able to allocate a memory from the mempool, hand over to the crypt
> layer and finish the IO. Is it possible this might take a lot of time?
See the backtrace below - the dm-crypt worker is making progress, but the
memory allocator deliberatelly stalls it in throttle_vm_writeout.
> > On Tue, 12 Jul 2016, Michal Hocko wrote:
> > >
> > > Look at the amount of free memory. It is completely depleted. So it
> > > smells like a process which has access to memory reserves has consumed
> > > all of it. I suspect a __GFP_MEMALLOC resp. PF_MEMALLOC from softirq
> > > context user which went off the leash.
> >
> > It is caused by the commit f9054c70d28bc214b2857cf8db8269f4f45a5e23. Prior
> > to this commit, mempool allocations set __GFP_NOMEMALLOC, so they never
> > exhausted reserved memory. With this commit, mempool allocations drop
> > __GFP_NOMEMALLOC, so they can dig deeper (if the process has PF_MEMALLOC,
> > they can bypass all limits).
>
> Hmm, but the patch allows access to the memory reserves only when the
> pool is empty. And even then the caller would have to request access to
> reserves explicitly either by __GFP_NOMEMALLOC or PF_MEMALLOC. That
PF_MEMALLOC is set when you enter the block driver when swapping. So, some
of the mempool allocations are done with PF_MEMALLOC.
> doesn't seem to be the case for the dm-crypt, though. Or do you suspect
> that some other mempool user might be doing so?
Bisection showed that that patch triggered the dm-crypt swapping problems.
Without the patch f9054c70d28bc214b2857cf8db8269f4f45a5e23, mempool_alloc
1. allocates memory up to __GFP_NOMEMALLOC limit
2. allocates memory from the mempool reserve
3. waits, until some objects are returned to the mempool
With the patch f9054c70d28bc214b2857cf8db8269f4f45a5e23, mempool_alloc
1. allocates memory up to __GFP_NOMEMALLOC limit
2. allocates memory from the mempool reserve
3. allocates all remaining memory until total exhaustion
4. waits, until some objects are returned to the mempool
> > > No this doesn't sound like a proper solution. The current decision
> > > logic, as explained above relies on the feedback from the reclaim. A
> > > free swap space doesn't really mean we can make a forward progress.
> >
> > I'm interested - why would you need to trigger the OOM killer if there is
> > free swap space?
>
> Let me clarify. If there is a swapable memory then we shouldn't trigger
> the OOM killer normally of course. And that should be the case with the
> current implementation. We just rely on the swapout making some progress
And what does exactly "making some progress" mean? How do you measure it?
> and back off only if that is not the case after several attempts with a
> throttling based on the writeback counters. Checking the available swap
> space doesn't guarantee a forward progress, though. If the swap out is
> stuck for some reason then it should be safer to trigger to OOM rather
> than wait or trash for ever (or an excessive amount of time).
>
> Now, I can see that the retry logic might need some tuning for complex
> setups like dm-crypt swap partitions because the progress might be much
> slower there. But I would like the understand what is the worst estimate
> for the swapout path with all the roadblocks on the way for this setup
> before we can think of a proper retry logic tuning.
For example, you could increment a percpu counter each time writeback of a
page is finished. If the counters stays the same for some pre-determined
period of time, writeback is stuck and you could trigger OOM prematurely.
But the memory management code doesn't do that. So what it really does and
what is the intention behind it?
Another question is - do we really want to try to recover in case of stuck
writeback? If the swap device dies so that it stops processing I/Os, the
system is dead anyway - there is no point in trying to recover it by
killing processes.
The question is if these safeguards against stuck writeback are really
doing more harm than good. Do you have some real use case where you get
stuck writeback and where you need to recover by OOM killing?
This is not the first time I've seen premature OOM. Long time ago, I saw a
case when the admin set /proc/sys/vm/swappiness to a low value (because he
was running some scientific calculations on the machine and he preferred
memory being allocated to those calculations rather than to the cache) -
and the result was premature OOM killing while the machine had plenty of
free swap swace.
> > The kernel 4.7-rc almost deadlocks in another way. The machine got stuck
> > and the following stacktrace was obtained when swapping to dm-crypt.
> >
> > We can see that dm-crypt does a mempool allocation. But the mempool
> > allocation somehow falls into throttle_vm_writeout. There, it waits for
> > 0.1 seconds. So, as a result, the dm-crypt worker thread ends up
> > processing requests at an unusually slow rate of 10 requests per second
> > and it results in the machine being stuck (it would proabably recover if
> > we waited for extreme amount of time).
>
> Hmm, that throttling is there since ever basically. I do not see what
> would have changed that recently, but I haven't looked too close to be
> honest.
>
> I agree that throttling a flusher (which this worker definitely is)
> doesn't look like a correct thing to do. We have PF_LESS_THROTTLE for
> this kind of things. So maybe the right thing to do is to use this flag
> for the dm_crypt worker:
>
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 4f3cb3554944..0b806810efab 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -1392,11 +1392,14 @@ static void kcryptd_async_done(struct crypto_async_request *async_req,
> static void kcryptd_crypt(struct work_struct *work)
> {
> struct dm_crypt_io *io = container_of(work, struct dm_crypt_io, work);
> + unsigned int pflags = current->flags;
>
> + current->flags |= PF_LESS_THROTTLE;
> if (bio_data_dir(io->base_bio) == READ)
> kcryptd_crypt_read_convert(io);
> else
> kcryptd_crypt_write_convert(io);
> + tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
> }
>
> static void kcryptd_queue_crypt(struct dm_crypt_io *io)
^^^ That fixes just one specific case - but there may be other threads
doing mempool allocations in the device mapper subsystem - and you would
need to mark all of them.
I would try the patch below - generally, allocations from the mempool
subsystem should not wait in the memory allocator at all. I don't know if
there are other cases when these allocations can sleep. I'm interested if
it fixes Ondrej's case - or if it uncovers some other sleeping.
An alternate possibility would be to drop the flag __GFP_DIRECT_RECLAIM in
mempool_alloc - so that mempool allocations never sleep in the allocator.
---
mm/page-writeback.c | 8 ++++++++
1 file changed, 8 insertions(+)
Index: linux-4.7-rc7/mm/page-writeback.c
===================================================================
--- linux-4.7-rc7.orig/mm/page-writeback.c 2016-07-12 20:57:53.000000000 +0200
+++ linux-4.7-rc7/mm/page-writeback.c 2016-07-12 20:59:41.000000000 +0200
@@ -1945,6 +1945,14 @@ void throttle_vm_writeout(gfp_t gfp_mask
unsigned long background_thresh;
unsigned long dirty_thresh;
+ /*
+ * If we came here from mempool_alloc, we don't want to wait 0.1s.
+ * We want to fail as soon as possible, so that the allocation is tried
+ * from mempool reserve.
+ */
+ if (unlikely(gfp_mask & __GFP_NORETRY))
+ return;
+
for ( ; ; ) {
global_dirty_limits(&background_thresh, &dirty_thresh);
dirty_thresh = hard_dirty_limit(&global_wb_domain, dirty_thresh);
> >
> > [ 345.352536] kworker/u4:0 D ffff88003df7f438 10488 6 2 0x00000000
> > [ 345.352536] Workqueue: kcryptd kcryptd_crypt [dm_crypt]
> > [ 345.352536] ffff88003df7f438 ffff88003e5d0380 ffff88003e5d0380 ffff88003e5d8e80
> > [ 345.352536] ffff88003dfb3240 ffff88003df73240 ffff88003df80000 ffff88003df7f470
> > [ 345.352536] ffff88003e5d0380 ffff88003e5d0380 ffff88003df7f828 ffff88003df7f450
> > [ 345.352536] Call Trace:
> > [ 345.352536] [<ffffffff818d466c>] schedule+0x3c/0x90
> > [ 345.352536] [<ffffffff818d96a8>] schedule_timeout+0x1d8/0x360
> > [ 345.352536] [<ffffffff81135e40>] ? detach_if_pending+0x1c0/0x1c0
> > [ 345.352536] [<ffffffff811407c3>] ? ktime_get+0xb3/0x150
> > [ 345.352536] [<ffffffff811958cf>] ? __delayacct_blkio_start+0x1f/0x30
> > [ 345.352536] [<ffffffff818d39e4>] io_schedule_timeout+0xa4/0x110
> > [ 345.352536] [<ffffffff8121d886>] congestion_wait+0x86/0x1f0
> > [ 345.352536] [<ffffffff810fdf40>] ? prepare_to_wait_event+0xf0/0xf0
> > [ 345.352536] [<ffffffff812061d4>] throttle_vm_writeout+0x44/0xd0
> > [ 345.352536] [<ffffffff81211533>] shrink_zone_memcg+0x613/0x720
> > [ 345.352536] [<ffffffff81211720>] shrink_zone+0xe0/0x300
> > [ 345.352536] [<ffffffff81211aed>] do_try_to_free_pages+0x1ad/0x450
> > [ 345.352536] [<ffffffff81211e7f>] try_to_free_pages+0xef/0x300
> > [ 345.352536] [<ffffffff811fef19>] __alloc_pages_nodemask+0x879/0x1210
> > [ 345.352536] [<ffffffff810e8080>] ? sched_clock_cpu+0x90/0xc0
> > [ 345.352536] [<ffffffff8125a8d1>] alloc_pages_current+0xa1/0x1f0
> > [ 345.352536] [<ffffffff81265ef5>] ? new_slab+0x3f5/0x6a0
> > [ 345.352536] [<ffffffff81265dd7>] new_slab+0x2d7/0x6a0
> > [ 345.352536] [<ffffffff810e7f87>] ? sched_clock_local+0x17/0x80
> > [ 345.352536] [<ffffffff812678cb>] ___slab_alloc+0x3fb/0x5c0
> > [ 345.352536] [<ffffffff811f71bd>] ? mempool_alloc_slab+0x1d/0x30
> > [ 345.352536] [<ffffffff810e7f87>] ? sched_clock_local+0x17/0x80
> > [ 345.352536] [<ffffffff811f71bd>] ? mempool_alloc_slab+0x1d/0x30
> > [ 345.352536] [<ffffffff81267ae1>] __slab_alloc+0x51/0x90
> > [ 345.352536] [<ffffffff811f71bd>] ? mempool_alloc_slab+0x1d/0x30
> > [ 345.352536] [<ffffffff81267d9b>] kmem_cache_alloc+0x27b/0x310
> > [ 345.352536] [<ffffffff811f71bd>] mempool_alloc_slab+0x1d/0x30
> > [ 345.352536] [<ffffffff811f6f11>] mempool_alloc+0x91/0x230
> > [ 345.352536] [<ffffffff8141a02d>] bio_alloc_bioset+0xbd/0x260
> > [ 345.352536] [<ffffffffc02f1a54>] kcryptd_crypt+0x114/0x3b0 [dm_crypt]
> > [ 345.352536] [<ffffffff810cc312>] process_one_work+0x242/0x700
> > [ 345.352536] [<ffffffff810cc28a>] ? process_one_work+0x1ba/0x700
> > [ 345.352536] [<ffffffff810cc81e>] worker_thread+0x4e/0x490
> > [ 345.352536] [<ffffffff810cc7d0>] ? process_one_work+0x700/0x700
> > [ 345.352536] [<ffffffff810d3c01>] kthread+0x101/0x120
> > [ 345.352536] [<ffffffff8110b9f5>] ? trace_hardirqs_on_caller+0xf5/0x1b0
> > [ 345.352536] [<ffffffff818db1af>] ret_from_fork+0x1f/0x40
> > [ 345.352536] [<ffffffff810d3b00>] ? kthread_create_on_node+0x250/0x250
>
> --
> Michal Hocko
> SUSE Labs
>
[toc] | [prev] | [next] | [standalone]
| From | Ondrej Kozina <okozina@redhat.com> |
|---|---|
| Date | 2016-07-14 13:00 +0200 |
| Subject | Re: [dm-devel] System freezes after OOM |
| Message-ID | <rUMx4-bM-29@gated-at.bofh.it> |
| In reply to | #1442533 |
On 07/13/2016 05:02 PM, Mikulas Patocka wrote:
>
>
> On Wed, 13 Jul 2016, Michal Hocko wrote:
>
>> On Tue 12-07-16 19:44:11, Mikulas Patocka wrote:
>>> The problem of swapping to dm-crypt is this.
>>>
>>> The free memory goes low, kswapd decides that some page should be swapped
>>> out. However, when you swap to an ecrypted device, writeback of each page
>>> requires another page to hold the encrypted data. dm-crypt uses mempools
>>> for all its structures and pages, so that it can make forward progress
>>> even if there is no memory free. However, the mempool code first allocates
>>> from general memory allocator and resorts to the mempool only if the
>>> memory is below limit.
>>
>> OK, thanks for the clarification. I guess the core part happens in
>> crypt_alloc_buffer, right?
>>
>>> So every attempt to swap out some page allocates another page.
>>>
>>> As long as swapping is in progress, the free memory is below the limit
>>> (because the swapping activity itself consumes any memory over the limit).
>>> And that triggered the OOM killer prematurely.
>>
>> I am not sure I understand the last part. Are you saing that we trigger
>> OOM because the initiated swapout will not be able to finish the IO thus
>> release the page in time?
>
> On kernel 4.6 - premature OOM is triggered just because the free memory
> stays below the limit for some time.
>
> On kernel 4.7-rc6 (that contains your OOM patch
> 0a0337e0d1d134465778a16f5cbea95086e8e9e0), OOM is not triggered, but the
> machine slows down to a crawl, because the allocator applies throttling to
> the process that is doing the encryption.
>
>> The oom detection checks waits for an ongoing writeout if there is no
>> reclaim progress and at least half of the reclaimable memory is either
>> dirty or under writeback. Pages under swaout are marked as under
>> writeback AFAIR. The writeout path (dm-crypt worker in this case) should
>> be able to allocate a memory from the mempool, hand over to the crypt
>> layer and finish the IO. Is it possible this might take a lot of time?
>
> See the backtrace below - the dm-crypt worker is making progress, but the
> memory allocator deliberatelly stalls it in throttle_vm_writeout.
>
>>> On Tue, 12 Jul 2016, Michal Hocko wrote:
>>>>
>>>> Look at the amount of free memory. It is completely depleted. So it
>>>> smells like a process which has access to memory reserves has consumed
>>>> all of it. I suspect a __GFP_MEMALLOC resp. PF_MEMALLOC from softirq
>>>> context user which went off the leash.
>>>
>>> It is caused by the commit f9054c70d28bc214b2857cf8db8269f4f45a5e23. Prior
>>> to this commit, mempool allocations set __GFP_NOMEMALLOC, so they never
>>> exhausted reserved memory. With this commit, mempool allocations drop
>>> __GFP_NOMEMALLOC, so they can dig deeper (if the process has PF_MEMALLOC,
>>> they can bypass all limits).
>>
>> Hmm, but the patch allows access to the memory reserves only when the
>> pool is empty. And even then the caller would have to request access to
>> reserves explicitly either by __GFP_NOMEMALLOC or PF_MEMALLOC. That
>
> PF_MEMALLOC is set when you enter the block driver when swapping. So, some
> of the mempool allocations are done with PF_MEMALLOC.
>
>> doesn't seem to be the case for the dm-crypt, though. Or do you suspect
>> that some other mempool user might be doing so?
>
> Bisection showed that that patch triggered the dm-crypt swapping problems.
>
> Without the patch f9054c70d28bc214b2857cf8db8269f4f45a5e23, mempool_alloc
> 1. allocates memory up to __GFP_NOMEMALLOC limit
> 2. allocates memory from the mempool reserve
> 3. waits, until some objects are returned to the mempool
>
> With the patch f9054c70d28bc214b2857cf8db8269f4f45a5e23, mempool_alloc
> 1. allocates memory up to __GFP_NOMEMALLOC limit
> 2. allocates memory from the mempool reserve
> 3. allocates all remaining memory until total exhaustion
> 4. waits, until some objects are returned to the mempool
>
>>>> No this doesn't sound like a proper solution. The current decision
>>>> logic, as explained above relies on the feedback from the reclaim. A
>>>> free swap space doesn't really mean we can make a forward progress.
>>>
>>> I'm interested - why would you need to trigger the OOM killer if there is
>>> free swap space?
>>
>> Let me clarify. If there is a swapable memory then we shouldn't trigger
>> the OOM killer normally of course. And that should be the case with the
>> current implementation. We just rely on the swapout making some progress
>
> And what does exactly "making some progress" mean? How do you measure it?
>
>> and back off only if that is not the case after several attempts with a
>> throttling based on the writeback counters. Checking the available swap
>> space doesn't guarantee a forward progress, though. If the swap out is
>> stuck for some reason then it should be safer to trigger to OOM rather
>> than wait or trash for ever (or an excessive amount of time).
>>
>> Now, I can see that the retry logic might need some tuning for complex
>> setups like dm-crypt swap partitions because the progress might be much
>> slower there. But I would like the understand what is the worst estimate
>> for the swapout path with all the roadblocks on the way for this setup
>> before we can think of a proper retry logic tuning.
>
> For example, you could increment a percpu counter each time writeback of a
> page is finished. If the counters stays the same for some pre-determined
> period of time, writeback is stuck and you could trigger OOM prematurely.
>
> But the memory management code doesn't do that. So what it really does and
> what is the intention behind it?
>
> Another question is - do we really want to try to recover in case of stuck
> writeback? If the swap device dies so that it stops processing I/Os, the
> system is dead anyway - there is no point in trying to recover it by
> killing processes.
>
> The question is if these safeguards against stuck writeback are really
> doing more harm than good. Do you have some real use case where you get
> stuck writeback and where you need to recover by OOM killing?
>
> This is not the first time I've seen premature OOM. Long time ago, I saw a
> case when the admin set /proc/sys/vm/swappiness to a low value (because he
> was running some scientific calculations on the machine and he preferred
> memory being allocated to those calculations rather than to the cache) -
> and the result was premature OOM killing while the machine had plenty of
> free swap swace.
>
>>> The kernel 4.7-rc almost deadlocks in another way. The machine got stuck
>>> and the following stacktrace was obtained when swapping to dm-crypt.
>>>
>>> We can see that dm-crypt does a mempool allocation. But the mempool
>>> allocation somehow falls into throttle_vm_writeout. There, it waits for
>>> 0.1 seconds. So, as a result, the dm-crypt worker thread ends up
>>> processing requests at an unusually slow rate of 10 requests per second
>>> and it results in the machine being stuck (it would proabably recover if
>>> we waited for extreme amount of time).
>>
>> Hmm, that throttling is there since ever basically. I do not see what
>> would have changed that recently, but I haven't looked too close to be
>> honest.
>>
>> I agree that throttling a flusher (which this worker definitely is)
>> doesn't look like a correct thing to do. We have PF_LESS_THROTTLE for
>> this kind of things. So maybe the right thing to do is to use this flag
>> for the dm_crypt worker:
>>
>> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
>> index 4f3cb3554944..0b806810efab 100644
>> --- a/drivers/md/dm-crypt.c
>> +++ b/drivers/md/dm-crypt.c
>> @@ -1392,11 +1392,14 @@ static void kcryptd_async_done(struct crypto_async_request *async_req,
>> static void kcryptd_crypt(struct work_struct *work)
>> {
>> struct dm_crypt_io *io = container_of(work, struct dm_crypt_io, work);
>> + unsigned int pflags = current->flags;
>>
>> + current->flags |= PF_LESS_THROTTLE;
>> if (bio_data_dir(io->base_bio) == READ)
>> kcryptd_crypt_read_convert(io);
>> else
>> kcryptd_crypt_write_convert(io);
>> + tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
>> }
>>
>> static void kcryptd_queue_crypt(struct dm_crypt_io *io)
>
> ^^^ That fixes just one specific case - but there may be other threads
> doing mempool allocations in the device mapper subsystem - and you would
> need to mark all of them.
>
> I would try the patch below - generally, allocations from the mempool
> subsystem should not wait in the memory allocator at all. I don't know if
> there are other cases when these allocations can sleep. I'm interested if
> it fixes Ondrej's case - or if it uncovers some other sleeping.
>
> An alternate possibility would be to drop the flag __GFP_DIRECT_RECLAIM in
> mempool_alloc - so that mempool allocations never sleep in the allocator.
Good news (I hope). With Mikulas's patch below I'm able to run the test
and not get the utility oom_killed. Neither the system livelocks for
dozens minutes as before with pure 4.7.0-rc6. Here's the syslog:
https://okozina.fedorapeople.org/bugs/swap_on_dmcrypt/4.7.0-rc7+/0/4.7.0-rc7+.log
Just for the record the test utility allocates memory. More than
physical ram installed, but much less than total ram including swap.
As you can see the swap fills slowly as expected. I'd not say it's ideal
fix since during the test system not much responsive, but still I'd call
it a progress.
Regards O.
>
> ---
> mm/page-writeback.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> Index: linux-4.7-rc7/mm/page-writeback.c
> ===================================================================
> --- linux-4.7-rc7.orig/mm/page-writeback.c 2016-07-12 20:57:53.000000000 +0200
> +++ linux-4.7-rc7/mm/page-writeback.c 2016-07-12 20:59:41.000000000 +0200
> @@ -1945,6 +1945,14 @@ void throttle_vm_writeout(gfp_t gfp_mask
> unsigned long background_thresh;
> unsigned long dirty_thresh;
>
> + /*
> + * If we came here from mempool_alloc, we don't want to wait 0.1s.
> + * We want to fail as soon as possible, so that the allocation is tried
> + * from mempool reserve.
> + */
> + if (unlikely(gfp_mask & __GFP_NORETRY))
> + return;
> +
> for ( ; ; ) {
> global_dirty_limits(&background_thresh, &dirty_thresh);
> dirty_thresh = hard_dirty_limit(&global_wb_domain, dirty_thresh);
>
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-07-14 15:00 +0200 |
| Message-ID | <rUOpb-1nZ-11@gated-at.bofh.it> |
| In reply to | #1442533 |
On Wed 13-07-16 11:02:15, Mikulas Patocka wrote:
> On Wed, 13 Jul 2016, Michal Hocko wrote:
[...]
We are discussing several topics together so let's focus on this
particlar thing for now
> > > The kernel 4.7-rc almost deadlocks in another way. The machine got stuck
> > > and the following stacktrace was obtained when swapping to dm-crypt.
> > >
> > > We can see that dm-crypt does a mempool allocation. But the mempool
> > > allocation somehow falls into throttle_vm_writeout. There, it waits for
> > > 0.1 seconds. So, as a result, the dm-crypt worker thread ends up
> > > processing requests at an unusually slow rate of 10 requests per second
> > > and it results in the machine being stuck (it would proabably recover if
> > > we waited for extreme amount of time).
> >
> > Hmm, that throttling is there since ever basically. I do not see what
> > would have changed that recently, but I haven't looked too close to be
> > honest.
> >
> > I agree that throttling a flusher (which this worker definitely is)
> > doesn't look like a correct thing to do. We have PF_LESS_THROTTLE for
> > this kind of things. So maybe the right thing to do is to use this flag
> > for the dm_crypt worker:
> >
> > diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> > index 4f3cb3554944..0b806810efab 100644
> > --- a/drivers/md/dm-crypt.c
> > +++ b/drivers/md/dm-crypt.c
> > @@ -1392,11 +1392,14 @@ static void kcryptd_async_done(struct crypto_async_request *async_req,
> > static void kcryptd_crypt(struct work_struct *work)
> > {
> > struct dm_crypt_io *io = container_of(work, struct dm_crypt_io, work);
> > + unsigned int pflags = current->flags;
> >
> > + current->flags |= PF_LESS_THROTTLE;
> > if (bio_data_dir(io->base_bio) == READ)
> > kcryptd_crypt_read_convert(io);
> > else
> > kcryptd_crypt_write_convert(io);
> > + tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
> > }
> >
> > static void kcryptd_queue_crypt(struct dm_crypt_io *io)
>
> ^^^ That fixes just one specific case - but there may be other threads
> doing mempool allocations in the device mapper subsystem - and you would
> need to mark all of them.
Now that I am thinking about it some more. Are there any mempool users
which would actually want to be throttled? I would expect mempool users
are necessary to push IO through and throttle them sounds like a bad
decision in the first place but there might be other mempool users which
could cause issues. Anyway how about setting PF_LESS_THROTTLE
unconditionally inside mempool_alloc? Something like the following:
diff --git a/mm/mempool.c b/mm/mempool.c
index 8f65464da5de..e21fb632983f 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -310,7 +310,8 @@ EXPORT_SYMBOL(mempool_resize);
*/
void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
{
- void *element;
+ unsigned int pflags = current->flags;
+ void *element = NULL;
unsigned long flags;
wait_queue_t wait;
gfp_t gfp_temp;
@@ -327,6 +328,12 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
gfp_temp = gfp_mask & ~(__GFP_DIRECT_RECLAIM|__GFP_IO);
+ /*
+ * Make sure that the allocation doesn't get throttled during the
+ * reclaim
+ */
+ if (gfpflags_allow_blocking(gfp_mask))
+ current->flags |= PF_LESS_THROTTLE;
repeat_alloc:
if (likely(pool->curr_nr)) {
/*
@@ -339,7 +346,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
element = pool->alloc(gfp_temp, pool->pool_data);
if (likely(element != NULL))
- return element;
+ goto out;
spin_lock_irqsave(&pool->lock, flags);
if (likely(pool->curr_nr)) {
@@ -352,7 +359,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
* for debugging.
*/
kmemleak_update_trace(element);
- return element;
+ goto out;
}
/*
@@ -369,7 +376,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
/* We must not sleep if !__GFP_DIRECT_RECLAIM */
if (!(gfp_mask & __GFP_DIRECT_RECLAIM)) {
spin_unlock_irqrestore(&pool->lock, flags);
- return NULL;
+ goto out;
}
/* Let's wait for someone else to return an element to @pool */
@@ -386,6 +393,10 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
finish_wait(&pool->wait, &wait);
goto repeat_alloc;
+out:
+ if (gfpflags_allow_blocking(gfp_mask))
+ tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
+ return element;
}
EXPORT_SYMBOL(mempool_alloc);
> I would try the patch below - generally, allocations from the mempool
> subsystem should not wait in the memory allocator at all. I don't know if
> there are other cases when these allocations can sleep. I'm interested if
> it fixes Ondrej's case - or if it uncovers some other sleeping.
__GFP_NORETRY is used outside of mempool allocator as well and
throttling them sounds like a proper think to do. The primary point of
throttle_vm_writeout is to slow down reclaim so that it doesn't generate
excessive amount of dirty pages. It used to be a bigger deal in the past
when we initiated regular IO from the direct reclaim but we can still
generate swap IO these days. So I would rather come up with a more
robust solution.
> An alternate possibility would be to drop the flag __GFP_DIRECT_RECLAIM in
> mempool_alloc - so that mempool allocations never sleep in the allocator.
Hmm, that would mean that the retry loop would completely rely on kswapd
doing forward progress. But note that kswapd might trigger IO and get
stuck waiting for the FS. GFP_NOIO request might be hopelessly
inefficient on its own but at least we try to reclaim something which
sounds better to me than looping and relying only on kswapd. I do not
see other potential side effects of such a change but my gut feeling
tells me this is not quite right. It works around a problem that is at a
different layer.
> ---
> mm/page-writeback.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> Index: linux-4.7-rc7/mm/page-writeback.c
> ===================================================================
> --- linux-4.7-rc7.orig/mm/page-writeback.c 2016-07-12 20:57:53.000000000 +0200
> +++ linux-4.7-rc7/mm/page-writeback.c 2016-07-12 20:59:41.000000000 +0200
> @@ -1945,6 +1945,14 @@ void throttle_vm_writeout(gfp_t gfp_mask
> unsigned long background_thresh;
> unsigned long dirty_thresh;
>
> + /*
> + * If we came here from mempool_alloc, we don't want to wait 0.1s.
> + * We want to fail as soon as possible, so that the allocation is tried
> + * from mempool reserve.
> + */
> + if (unlikely(gfp_mask & __GFP_NORETRY))
> + return;
> +
> for ( ; ; ) {
> global_dirty_limits(&background_thresh, &dirty_thresh);
> dirty_thresh = hard_dirty_limit(&global_wb_domain, dirty_thresh);
>
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Mikulas Patocka <mpatocka@redhat.com> |
|---|---|
| Date | 2016-07-14 16:10 +0200 |
| Message-ID | <rUPuV-2hx-5@gated-at.bofh.it> |
| In reply to | #1443431 |
On Thu, 14 Jul 2016, Michal Hocko wrote:
> On Wed 13-07-16 11:02:15, Mikulas Patocka wrote:
> > > diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> > > index 4f3cb3554944..0b806810efab 100644
> > > --- a/drivers/md/dm-crypt.c
> > > +++ b/drivers/md/dm-crypt.c
> > > @@ -1392,11 +1392,14 @@ static void kcryptd_async_done(struct crypto_async_request *async_req,
> > > static void kcryptd_crypt(struct work_struct *work)
> > > {
> > > struct dm_crypt_io *io = container_of(work, struct dm_crypt_io, work);
> > > + unsigned int pflags = current->flags;
> > >
> > > + current->flags |= PF_LESS_THROTTLE;
> > > if (bio_data_dir(io->base_bio) == READ)
> > > kcryptd_crypt_read_convert(io);
> > > else
> > > kcryptd_crypt_write_convert(io);
> > > + tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
> > > }
> > >
> > > static void kcryptd_queue_crypt(struct dm_crypt_io *io)
> >
> > ^^^ That fixes just one specific case - but there may be other threads
> > doing mempool allocations in the device mapper subsystem - and you would
> > need to mark all of them.
>
> Now that I am thinking about it some more. Are there any mempool users
> which would actually want to be throttled? I would expect mempool users
> are necessary to push IO through and throttle them sounds like a bad
> decision in the first place but there might be other mempool users which
> could cause issues. Anyway how about setting PF_LESS_THROTTLE
> unconditionally inside mempool_alloc? Something like the following:
>
> diff --git a/mm/mempool.c b/mm/mempool.c
> index 8f65464da5de..e21fb632983f 100644
> --- a/mm/mempool.c
> +++ b/mm/mempool.c
> @@ -310,7 +310,8 @@ EXPORT_SYMBOL(mempool_resize);
> */
> void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> {
> - void *element;
> + unsigned int pflags = current->flags;
> + void *element = NULL;
> unsigned long flags;
> wait_queue_t wait;
> gfp_t gfp_temp;
> @@ -327,6 +328,12 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>
> gfp_temp = gfp_mask & ~(__GFP_DIRECT_RECLAIM|__GFP_IO);
>
> + /*
> + * Make sure that the allocation doesn't get throttled during the
> + * reclaim
> + */
> + if (gfpflags_allow_blocking(gfp_mask))
> + current->flags |= PF_LESS_THROTTLE;
> repeat_alloc:
> if (likely(pool->curr_nr)) {
> /*
> @@ -339,7 +346,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>
> element = pool->alloc(gfp_temp, pool->pool_data);
> if (likely(element != NULL))
> - return element;
> + goto out;
>
> spin_lock_irqsave(&pool->lock, flags);
> if (likely(pool->curr_nr)) {
> @@ -352,7 +359,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> * for debugging.
> */
> kmemleak_update_trace(element);
> - return element;
> + goto out;
> }
>
> /*
> @@ -369,7 +376,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> /* We must not sleep if !__GFP_DIRECT_RECLAIM */
> if (!(gfp_mask & __GFP_DIRECT_RECLAIM)) {
> spin_unlock_irqrestore(&pool->lock, flags);
> - return NULL;
> + goto out;
> }
>
> /* Let's wait for someone else to return an element to @pool */
> @@ -386,6 +393,10 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>
> finish_wait(&pool->wait, &wait);
> goto repeat_alloc;
> +out:
> + if (gfpflags_allow_blocking(gfp_mask))
> + tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
> + return element;
> }
> EXPORT_SYMBOL(mempool_alloc);
>
But it needs other changes to honor the PF_LESS_THROTTLE flag:
static int current_may_throttle(void)
{
return !(current->flags & PF_LESS_THROTTLE) ||
current->backing_dev_info == NULL ||
bdi_write_congested(current->backing_dev_info);
}
--- if you set PF_LESS_THROTTLE, current_may_throttle may still return
true if one of the other conditions is met.
shrink_zone_memcg calls throttle_vm_writeout without checking
PF_LESS_THROTTLE at all.
Mikulas
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-07-14 17:00 +0200 |
| Message-ID | <rUQhj-2yA-19@gated-at.bofh.it> |
| In reply to | #1443474 |
On Thu 14-07-16 10:00:16, Mikulas Patocka wrote:
>
>
> On Thu, 14 Jul 2016, Michal Hocko wrote:
>
> > On Wed 13-07-16 11:02:15, Mikulas Patocka wrote:
>
> > > > diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> > > > index 4f3cb3554944..0b806810efab 100644
> > > > --- a/drivers/md/dm-crypt.c
> > > > +++ b/drivers/md/dm-crypt.c
> > > > @@ -1392,11 +1392,14 @@ static void kcryptd_async_done(struct crypto_async_request *async_req,
> > > > static void kcryptd_crypt(struct work_struct *work)
> > > > {
> > > > struct dm_crypt_io *io = container_of(work, struct dm_crypt_io, work);
> > > > + unsigned int pflags = current->flags;
> > > >
> > > > + current->flags |= PF_LESS_THROTTLE;
> > > > if (bio_data_dir(io->base_bio) == READ)
> > > > kcryptd_crypt_read_convert(io);
> > > > else
> > > > kcryptd_crypt_write_convert(io);
> > > > + tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
> > > > }
> > > >
> > > > static void kcryptd_queue_crypt(struct dm_crypt_io *io)
> > >
> > > ^^^ That fixes just one specific case - but there may be other threads
> > > doing mempool allocations in the device mapper subsystem - and you would
> > > need to mark all of them.
> >
> > Now that I am thinking about it some more. Are there any mempool users
> > which would actually want to be throttled? I would expect mempool users
> > are necessary to push IO through and throttle them sounds like a bad
> > decision in the first place but there might be other mempool users which
> > could cause issues. Anyway how about setting PF_LESS_THROTTLE
> > unconditionally inside mempool_alloc? Something like the following:
> >
> > diff --git a/mm/mempool.c b/mm/mempool.c
> > index 8f65464da5de..e21fb632983f 100644
> > --- a/mm/mempool.c
> > +++ b/mm/mempool.c
> > @@ -310,7 +310,8 @@ EXPORT_SYMBOL(mempool_resize);
> > */
> > void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> > {
> > - void *element;
> > + unsigned int pflags = current->flags;
> > + void *element = NULL;
> > unsigned long flags;
> > wait_queue_t wait;
> > gfp_t gfp_temp;
> > @@ -327,6 +328,12 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> >
> > gfp_temp = gfp_mask & ~(__GFP_DIRECT_RECLAIM|__GFP_IO);
> >
> > + /*
> > + * Make sure that the allocation doesn't get throttled during the
> > + * reclaim
> > + */
> > + if (gfpflags_allow_blocking(gfp_mask))
> > + current->flags |= PF_LESS_THROTTLE;
> > repeat_alloc:
> > if (likely(pool->curr_nr)) {
> > /*
> > @@ -339,7 +346,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> >
> > element = pool->alloc(gfp_temp, pool->pool_data);
> > if (likely(element != NULL))
> > - return element;
> > + goto out;
> >
> > spin_lock_irqsave(&pool->lock, flags);
> > if (likely(pool->curr_nr)) {
> > @@ -352,7 +359,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> > * for debugging.
> > */
> > kmemleak_update_trace(element);
> > - return element;
> > + goto out;
> > }
> >
> > /*
> > @@ -369,7 +376,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> > /* We must not sleep if !__GFP_DIRECT_RECLAIM */
> > if (!(gfp_mask & __GFP_DIRECT_RECLAIM)) {
> > spin_unlock_irqrestore(&pool->lock, flags);
> > - return NULL;
> > + goto out;
> > }
> >
> > /* Let's wait for someone else to return an element to @pool */
> > @@ -386,6 +393,10 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> >
> > finish_wait(&pool->wait, &wait);
> > goto repeat_alloc;
> > +out:
> > + if (gfpflags_allow_blocking(gfp_mask))
> > + tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
> > + return element;
> > }
> > EXPORT_SYMBOL(mempool_alloc);
> >
>
> But it needs other changes to honor the PF_LESS_THROTTLE flag:
>
> static int current_may_throttle(void)
> {
> return !(current->flags & PF_LESS_THROTTLE) ||
> current->backing_dev_info == NULL ||
> bdi_write_congested(current->backing_dev_info);
> }
> --- if you set PF_LESS_THROTTLE, current_may_throttle may still return
> true if one of the other conditions is met.
That is true but doesn't that mean that the device is congested and
waiting a bit is the right thing to do?
> shrink_zone_memcg calls throttle_vm_writeout without checking
> PF_LESS_THROTTLE at all.
Yes it doesn't call it because it relies on
global_dirty_limits()->domain_dirty_limits() to DTRT. It will give the
caller with PF_LESS_THROTTLE some boost wrt. all other writers.
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Ondrej Kozina <okozina@redhat.com> |
|---|---|
| Date | 2016-07-14 17:30 +0200 |
| Message-ID | <rUQKm-2Yl-17@gated-at.bofh.it> |
| In reply to | #1443499 |
On 07/14/2016 04:59 PM, Michal Hocko wrote:
> On Thu 14-07-16 10:00:16, Mikulas Patocka wrote:
>>
>>
>> On Thu, 14 Jul 2016, Michal Hocko wrote:
>>
>>> On Wed 13-07-16 11:02:15, Mikulas Patocka wrote:
>>
>>>>> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
>>>>> index 4f3cb3554944..0b806810efab 100644
>>>>> --- a/drivers/md/dm-crypt.c
>>>>> +++ b/drivers/md/dm-crypt.c
>>>>> @@ -1392,11 +1392,14 @@ static void kcryptd_async_done(struct crypto_async_request *async_req,
>>>>> static void kcryptd_crypt(struct work_struct *work)
>>>>> {
>>>>> struct dm_crypt_io *io = container_of(work, struct dm_crypt_io, work);
>>>>> + unsigned int pflags = current->flags;
>>>>>
>>>>> + current->flags |= PF_LESS_THROTTLE;
>>>>> if (bio_data_dir(io->base_bio) == READ)
>>>>> kcryptd_crypt_read_convert(io);
>>>>> else
>>>>> kcryptd_crypt_write_convert(io);
>>>>> + tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
>>>>> }
>>>>>
>>>>> static void kcryptd_queue_crypt(struct dm_crypt_io *io)
>>>>
>>>> ^^^ That fixes just one specific case - but there may be other threads
>>>> doing mempool allocations in the device mapper subsystem - and you would
>>>> need to mark all of them.
>>>
>>> Now that I am thinking about it some more. Are there any mempool users
>>> which would actually want to be throttled? I would expect mempool users
>>> are necessary to push IO through and throttle them sounds like a bad
>>> decision in the first place but there might be other mempool users which
>>> could cause issues. Anyway how about setting PF_LESS_THROTTLE
>>> unconditionally inside mempool_alloc? Something like the following:
>>>
>>> diff --git a/mm/mempool.c b/mm/mempool.c
>>> index 8f65464da5de..e21fb632983f 100644
>>> --- a/mm/mempool.c
>>> +++ b/mm/mempool.c
>>> @@ -310,7 +310,8 @@ EXPORT_SYMBOL(mempool_resize);
>>> */
>>> void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>>> {
>>> - void *element;
>>> + unsigned int pflags = current->flags;
>>> + void *element = NULL;
>>> unsigned long flags;
>>> wait_queue_t wait;
>>> gfp_t gfp_temp;
>>> @@ -327,6 +328,12 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>>>
>>> gfp_temp = gfp_mask & ~(__GFP_DIRECT_RECLAIM|__GFP_IO);
>>>
>>> + /*
>>> + * Make sure that the allocation doesn't get throttled during the
>>> + * reclaim
>>> + */
>>> + if (gfpflags_allow_blocking(gfp_mask))
>>> + current->flags |= PF_LESS_THROTTLE;
>>> repeat_alloc:
>>> if (likely(pool->curr_nr)) {
>>> /*
>>> @@ -339,7 +346,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>>>
>>> element = pool->alloc(gfp_temp, pool->pool_data);
>>> if (likely(element != NULL))
>>> - return element;
>>> + goto out;
>>>
>>> spin_lock_irqsave(&pool->lock, flags);
>>> if (likely(pool->curr_nr)) {
>>> @@ -352,7 +359,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>>> * for debugging.
>>> */
>>> kmemleak_update_trace(element);
>>> - return element;
>>> + goto out;
>>> }
>>>
>>> /*
>>> @@ -369,7 +376,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>>> /* We must not sleep if !__GFP_DIRECT_RECLAIM */
>>> if (!(gfp_mask & __GFP_DIRECT_RECLAIM)) {
>>> spin_unlock_irqrestore(&pool->lock, flags);
>>> - return NULL;
>>> + goto out;
>>> }
>>>
>>> /* Let's wait for someone else to return an element to @pool */
>>> @@ -386,6 +393,10 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>>>
>>> finish_wait(&pool->wait, &wait);
>>> goto repeat_alloc;
>>> +out:
>>> + if (gfpflags_allow_blocking(gfp_mask))
>>> + tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
>>> + return element;
>>> }
>>> EXPORT_SYMBOL(mempool_alloc);
>>>
>>
>> But it needs other changes to honor the PF_LESS_THROTTLE flag:
>>
>> static int current_may_throttle(void)
>> {
>> return !(current->flags & PF_LESS_THROTTLE) ||
>> current->backing_dev_info == NULL ||
>> bdi_write_congested(current->backing_dev_info);
>> }
>> --- if you set PF_LESS_THROTTLE, current_may_throttle may still return
>> true if one of the other conditions is met.
>
> That is true but doesn't that mean that the device is congested and
> waiting a bit is the right thing to do?
>
>> shrink_zone_memcg calls throttle_vm_writeout without checking
>> PF_LESS_THROTTLE at all.
>
> Yes it doesn't call it because it relies on
> global_dirty_limits()->domain_dirty_limits() to DTRT. It will give the
> caller with PF_LESS_THROTTLE some boost wrt. all other writers.
>
Not sure it'll help but I had to apply following patch to your original
one. Without it it didn't work.
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index e248194..1616192 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1940,11 +1940,23 @@ bool wb_over_bg_thresh(struct bdi_writeback *wb)
return false;
}
+static int current_may_throttle(void)
+{
+ if (current->flags & PF_LESS_THROTTLE)
+ return 0;
+
+ return current->backing_dev_info == NULL ||
+ bdi_write_congested(current->backing_dev_info);
+}
+
void throttle_vm_writeout(gfp_t gfp_mask)
{
unsigned long background_thresh;
unsigned long dirty_thresh;
+ if (!current_may_throttle())
+ return;
+
for ( ; ; ) {
global_dirty_limits(&background_thresh, &dirty_thresh);
dirty_thresh = hard_dirty_limit(&global_wb_domain,
dirty_thresh);
Regards Ondra
[toc] | [prev] | [next] | [standalone]
| From | Mikulas Patocka <mpatocka@redhat.com> |
|---|---|
| Date | 2016-07-14 19:40 +0200 |
| Message-ID | <rUSMa-4cH-25@gated-at.bofh.it> |
| In reply to | #1443499 |
On Thu, 14 Jul 2016, Michal Hocko wrote:
> On Thu 14-07-16 10:00:16, Mikulas Patocka wrote:
> >
> >
> > On Thu, 14 Jul 2016, Michal Hocko wrote:
> >
> > > On Wed 13-07-16 11:02:15, Mikulas Patocka wrote:
> >
> > > > > diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> > > > > index 4f3cb3554944..0b806810efab 100644
> > > > > --- a/drivers/md/dm-crypt.c
> > > > > +++ b/drivers/md/dm-crypt.c
> > > > > @@ -1392,11 +1392,14 @@ static void kcryptd_async_done(struct crypto_async_request *async_req,
> > > > > static void kcryptd_crypt(struct work_struct *work)
> > > > > {
> > > > > struct dm_crypt_io *io = container_of(work, struct dm_crypt_io, work);
> > > > > + unsigned int pflags = current->flags;
> > > > >
> > > > > + current->flags |= PF_LESS_THROTTLE;
> > > > > if (bio_data_dir(io->base_bio) == READ)
> > > > > kcryptd_crypt_read_convert(io);
> > > > > else
> > > > > kcryptd_crypt_write_convert(io);
> > > > > + tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
> > > > > }
> > > > >
> > > > > static void kcryptd_queue_crypt(struct dm_crypt_io *io)
> > > >
> > > > ^^^ That fixes just one specific case - but there may be other threads
> > > > doing mempool allocations in the device mapper subsystem - and you would
> > > > need to mark all of them.
> > >
> > > Now that I am thinking about it some more. Are there any mempool users
> > > which would actually want to be throttled? I would expect mempool users
> > > are necessary to push IO through and throttle them sounds like a bad
> > > decision in the first place but there might be other mempool users which
> > > could cause issues. Anyway how about setting PF_LESS_THROTTLE
> > > unconditionally inside mempool_alloc? Something like the following:
> > >
> > > diff --git a/mm/mempool.c b/mm/mempool.c
> > > index 8f65464da5de..e21fb632983f 100644
> > > --- a/mm/mempool.c
> > > +++ b/mm/mempool.c
> > > @@ -310,7 +310,8 @@ EXPORT_SYMBOL(mempool_resize);
> > > */
> > > void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> > > {
> > > - void *element;
> > > + unsigned int pflags = current->flags;
> > > + void *element = NULL;
> > > unsigned long flags;
> > > wait_queue_t wait;
> > > gfp_t gfp_temp;
> > > @@ -327,6 +328,12 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> > >
> > > gfp_temp = gfp_mask & ~(__GFP_DIRECT_RECLAIM|__GFP_IO);
> > >
> > > + /*
> > > + * Make sure that the allocation doesn't get throttled during the
> > > + * reclaim
> > > + */
> > > + if (gfpflags_allow_blocking(gfp_mask))
> > > + current->flags |= PF_LESS_THROTTLE;
> > > repeat_alloc:
> > > if (likely(pool->curr_nr)) {
> > > /*
> > > @@ -339,7 +346,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> > >
> > > element = pool->alloc(gfp_temp, pool->pool_data);
> > > if (likely(element != NULL))
> > > - return element;
> > > + goto out;
> > >
> > > spin_lock_irqsave(&pool->lock, flags);
> > > if (likely(pool->curr_nr)) {
> > > @@ -352,7 +359,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> > > * for debugging.
> > > */
> > > kmemleak_update_trace(element);
> > > - return element;
> > > + goto out;
> > > }
> > >
> > > /*
> > > @@ -369,7 +376,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> > > /* We must not sleep if !__GFP_DIRECT_RECLAIM */
> > > if (!(gfp_mask & __GFP_DIRECT_RECLAIM)) {
> > > spin_unlock_irqrestore(&pool->lock, flags);
> > > - return NULL;
> > > + goto out;
> > > }
> > >
> > > /* Let's wait for someone else to return an element to @pool */
> > > @@ -386,6 +393,10 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> > >
> > > finish_wait(&pool->wait, &wait);
> > > goto repeat_alloc;
> > > +out:
> > > + if (gfpflags_allow_blocking(gfp_mask))
> > > + tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
> > > + return element;
> > > }
> > > EXPORT_SYMBOL(mempool_alloc);
> > >
> >
> > But it needs other changes to honor the PF_LESS_THROTTLE flag:
> >
> > static int current_may_throttle(void)
> > {
> > return !(current->flags & PF_LESS_THROTTLE) ||
> > current->backing_dev_info == NULL ||
> > bdi_write_congested(current->backing_dev_info);
> > }
> > --- if you set PF_LESS_THROTTLE, current_may_throttle may still return
> > true if one of the other conditions is met.
>
> That is true but doesn't that mean that the device is congested and
> waiting a bit is the right thing to do?
You shouldn't really throttle mempool allocations at all. It's better to
fail the allocation quickly and allocate from a mempool reserve than to
wait 0.1 seconds in the reclaim path.
dm-crypt can do approximatelly 100MB/s. That means that it processes 25k
swap pages per second. If you wait in mempool_alloc, the allocation would
be satisfied in 0.00004s. If you wait in the allocator's throttle
function, you waste 0.1s.
It is also questionable if those 0.1 second sleeps are reasonable at all.
SSDs with 100k IOPS are common - they can drain the request queue in much
less time than 0.1 second. I think those hardcoded 0.1 second sleeps
should be replaced with sleeps until the device stops being congested.
Mikulas
> > shrink_zone_memcg calls throttle_vm_writeout without checking
> > PF_LESS_THROTTLE at all.
>
> Yes it doesn't call it because it relies on
> global_dirty_limits()->domain_dirty_limits() to DTRT. It will give the
> caller with PF_LESS_THROTTLE some boost wrt. all other writers.
> --
> Michal Hocko
> SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-07-15 10:40 +0200 |
| Message-ID | <rV6P7-4CX-15@gated-at.bofh.it> |
| In reply to | #1443629 |
On Thu 14-07-16 13:35:35, Mikulas Patocka wrote:
> On Thu, 14 Jul 2016, Michal Hocko wrote:
> > On Thu 14-07-16 10:00:16, Mikulas Patocka wrote:
> > > But it needs other changes to honor the PF_LESS_THROTTLE flag:
> > >
> > > static int current_may_throttle(void)
> > > {
> > > return !(current->flags & PF_LESS_THROTTLE) ||
> > > current->backing_dev_info == NULL ||
> > > bdi_write_congested(current->backing_dev_info);
> > > }
> > > --- if you set PF_LESS_THROTTLE, current_may_throttle may still return
> > > true if one of the other conditions is met.
> >
> > That is true but doesn't that mean that the device is congested and
> > waiting a bit is the right thing to do?
>
> You shouldn't really throttle mempool allocations at all. It's better to
> fail the allocation quickly and allocate from a mempool reserve than to
> wait 0.1 seconds in the reclaim path.
Well, but we do that already, no? The first allocation request is NOWAIT
and then we try to consume an object from the pool. We are re-adding
__GFP_DIRECT_RECLAIM in case both fail. The point of throttling is to
prevent from scanning through LRUs too quickly while we know that the
bdi is congested.
> dm-crypt can do approximatelly 100MB/s. That means that it processes 25k
> swap pages per second. If you wait in mempool_alloc, the allocation would
> be satisfied in 0.00004s. If you wait in the allocator's throttle
> function, you waste 0.1s.
>
>
> It is also questionable if those 0.1 second sleeps are reasonable at all.
> SSDs with 100k IOPS are common - they can drain the request queue in much
> less time than 0.1 second. I think those hardcoded 0.1 second sleeps
> should be replaced with sleeps until the device stops being congested.
Well if we do not do throttle_vm_writeout then the only remaining
writeout throttling for PF_LESS_THROTTLE is wait_iff_congested for
the direct reclaim and that should wake up if the device stops being
congested AFAIU.
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
Page 1 of 3 [1] 2 3 Next page →
Back to top | Article view | linux.kernel
csiph-web