Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1440722 > unrolled thread

Re: System freezes after OOM

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.

Contents

  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 3 of 3 — ← Prev page 1 2 [3]

#1443803

On Fri, 15 Jul 2016, Tetsuo Handa wrote:

> Whether the OOM reaper will free some memory no longer matters. Instead,
> whether the OOM reaper will let the OOM killer select next OOM victim matters.
> 
> Are you aware that the OOM reaper will let the OOM killer select next OOM
> victim (currently by clearing TIF_MEMDIE)? Clearing TIF_MEMDIE in 4.6 occurred
> only when OOM reaping succeeded. But we are going to change the OOM reaper
> always clear TIF_MEMDIE in 4.8 (or presumably change the OOM killer not to
> depend on TIF_MEMDIE) so that the OOM reaper guarantees that the OOM killer
> always selects next OOM victim.
> 

That's cute, I'll have to look into those patches.

[toc] | [prev] | [next] | [standalone]

#1444199

On Thu, 14 Jul 2016, David Rientjes wrote:

> On Thu, 14 Jul 2016, Tetsuo Handa wrote:
> 
> > David Rientjes wrote:
> > > On Wed, 13 Jul 2016, Mikulas Patocka wrote:
> > > 
> > > > What are the real problems that f9054c70d28bc214b2857cf8db8269f4f45a5e23 
> > > > tries to fix?
> > > > 
> > > 
> > > It prevents the whole system from livelocking due to an oom killed process 
> > > stalling forever waiting for mempool_alloc() to return.  No other threads 
> > > may be oom killed while waiting for it to exit.
> > 
> > Is that concern still valid? We have the OOM reaper for CONFIG_MMU=y case.
> > 
> 
> Umm, show me an explicit guarantee where the oom reaper will free memory 
> such that other threads may return memory to this process's mempool so it 
> can make forward progress in mempool_alloc() without the need of utilizing 
> memory reserves.  First, it might be helpful to show that the oom reaper 
> is ever guaranteed to free any memory for a selected oom victim.

The function mempool_alloc sleeps with "io_schedule_timeout(5*HZ);"

So, if the oom reaper frees some memory into the page allocator, the 
process that is stuck in mempoo_alloc will sleep for up to 5 seconds, then 
it will retry the allocation with "element = pool->alloc(gfp_temp, 
pool->pool_data)" (that will allocate from the page allocator) and succed.

Mikulas

[toc] | [prev] | [next] | [standalone]

#1444603

On Fri, 15 Jul 2016, Mikulas Patocka wrote:

> > Umm, show me an explicit guarantee where the oom reaper will free memory 
> > such that other threads may return memory to this process's mempool so it 
> > can make forward progress in mempool_alloc() without the need of utilizing 
> > memory reserves.  First, it might be helpful to show that the oom reaper 
> > is ever guaranteed to free any memory for a selected oom victim.
> 
> The function mempool_alloc sleeps with "io_schedule_timeout(5*HZ);"
> 
> So, if the oom reaper frees some memory into the page allocator, the 
> process that is stuck in mempoo_alloc will sleep for up to 5 seconds, then 
> it will retry the allocation with "element = pool->alloc(gfp_temp, 
> pool->pool_data)" (that will allocate from the page allocator) and succed.
> 

No, the state of the 4.7 oom killer does not explicitly guarantee any 
memory freeing of the victim and there is no guarantee that elements will 
be returned to the mempool.  If you're talking about patches that you're 
proposing for the 4.8 merge window, please post them with complete 
changelogs.  Thanks.

[toc] | [prev] | [next] | [standalone]

#1443408

On Wed, 13 Jul 2016, David Rientjes wrote:

> On Wed, 13 Jul 2016, Mikulas Patocka wrote:
> 
> > What are the real problems that f9054c70d28bc214b2857cf8db8269f4f45a5e23 
> > tries to fix?
> > 
> 
> It prevents the whole system from livelocking due to an oom killed process 
> stalling forever waiting for mempool_alloc() to return.  No other threads 
> may be oom killed while waiting for it to exit.
> 
> > Do you have a stacktrace where it deadlocked, or was just a theoretical 
> > consideration?
> > 
> 
> schedule
> schedule_timeout
> io_schedule_timeout
> mempool_alloc
> __split_and_process_bio
> dm_request
> generic_make_request
> submit_bio
> mpage_readpages
> ext4_readpages
> __do_page_cache_readahead
> ra_submit
> filemap_fault
> handle_mm_fault
> __do_page_fault
> do_page_fault
> page_fault

Device mapper should be able to proceed if there is no available memory. 
If it doesn't proceed, there is a bug in it.

I'd like to ask - what device mapper targets did you use in this case? Are 
there some other deadlocked processes? (show sysrq-t, sysrq-w when this 
happened)

Did the machine lock up completely with that stacktrace, or was it just 
slowed down?

> > Mempool users generally (except for some flawed cases like fs_bio_set) do 
> > not require memory to proceed. So if you just loop in mempool_alloc, the 
> > processes that exhasted the mempool reserve will eventually return objects 
> > to the mempool and you should proceed.
> > 
> 
> That's obviously not the case if we have hundreds of machines timing out 
> after two hours waiting for that fault to succeed.  The mempool interface 
> cannot require that users return elements to the pool synchronous with all 
> allocators so that we can happily loop forever, the only requirement on 

Mempool users must return objects to the mempool.

> the interface is that mempool_alloc() must succeed.  If the context of the 
> thread doing mempool_alloc() allows access to memory reserves, this will 
> always be allowed by the page allocator.  This is not a mempool problem.

Mikulas

[toc] | [prev] | [next] | [standalone]

#1443739

On Thu, 14 Jul 2016, Mikulas Patocka wrote:

> > schedule
> > schedule_timeout
> > io_schedule_timeout
> > mempool_alloc
> > __split_and_process_bio
> > dm_request
> > generic_make_request
> > submit_bio
> > mpage_readpages
> > ext4_readpages
> > __do_page_cache_readahead
> > ra_submit
> > filemap_fault
> > handle_mm_fault
> > __do_page_fault
> > do_page_fault
> > page_fault
> 
> Device mapper should be able to proceed if there is no available memory. 
> If it doesn't proceed, there is a bug in it.
> 

The above stack trace has nothing to do with the device mapper with 
pre-f9054c70d28b behavior.  It simply is calling into mempool_alloc() and 
no elements are being returned to the mempool that allow it to return.

Recall that in the above situation, the whole system is oom; nothing can 
allocate memory.  The oom killer has selected the above process to be oom 
killed, so all other processes on the system trying to allocate memory 
will stall in the page allocator waiting for this process to exit.

The natural response to this situation is to allow access to memory 
reserves, if possible, so that mempool_alloc() may return.  There is no 
guarantee that _anything_ can return memory to the mempool, especially in 
a system oom condition where nothing can make forward progress.  Insisting 
that should be guaranteed is not helpful.

> I'd like to ask - what device mapper targets did you use in this case? Are 
> there some other deadlocked processes? (show sysrq-t, sysrq-w when this 
> happened)
> 

Every process on the system is deadlocked because they cannot get memory 
through the page allocator until the above process exits.  That is how the 
oom killer works: select a process, kill it, give it access to memory 
reserves so it may exit and free its memory, and wait.

> Did the machine lock up completely with that stacktrace, or was it just 
> slowed down?
> 

Hundreds of machines locked up and rebooted after a two hour watchdog 
timeout.

> > That's obviously not the case if we have hundreds of machines timing out 
> > after two hours waiting for that fault to succeed.  The mempool interface 
> > cannot require that users return elements to the pool synchronous with all 
> > allocators so that we can happily loop forever, the only requirement on 
> 
> Mempool users must return objects to the mempool.
> 

Not possible when the system is livelocked.

[toc] | [prev] | [next] | [standalone]

#1444200

On Thu, 14 Jul 2016, David Rientjes wrote:

> There is no guarantee that _anything_ can return memory to the mempool,

You misunderstand mempools if you make such claims.

There is in fact guarantee that objects will be returned to mempool. In 
the past I reviewed device mapper thoroughly to make sure that it can make 
forward progress even if there is no available memory.

I don't know what should I tell you if you keep on repeating the same 
false claim over and over again. Should I explain mempool oprerations to 
you in detail? Or will you find it on your own?

Mikulas

[toc] | [prev] | [next] | [standalone]

#1444602

On Fri, 15 Jul 2016, Mikulas Patocka wrote:

> > There is no guarantee that _anything_ can return memory to the mempool,
> 
> You misunderstand mempools if you make such claims.
> 
> There is in fact guarantee that objects will be returned to mempool. In 
> the past I reviewed device mapper thoroughly to make sure that it can make 
> forward progress even if there is no available memory.
> 
> I don't know what should I tell you if you keep on repeating the same 
> false claim over and over again. Should I explain mempool oprerations to 
> you in detail? Or will you find it on your own?
> 

If you are talking about patches you're proposing for 4.8 or any guarantee 
of memory freeing that the oom killer/reaper will provide in 4.8, that's 
fine.  However, the state of the 4.7 kernel is the same as it was when I 
fixed this issue that timed out hundreds of our machines and is 
contradicted by that evidence.  Our machines time out after two hours with 
the oom victim looping forever in mempool_alloc(), so if there was a 
guarantee that elements would be returned in a completely livelocked 
kernel in 4.7 or earlier kernels, that would not have been the case.  I 
frankly don't care about your patch reviewing of dm mempool usage when 
dm_request() livelocked our kernel.

Feel free to formally propose patches either for 4.7 or 4.8.

[toc] | [prev] | [next] | [standalone]

#1444619

On Fri, 15 Jul 2016, David Rientjes wrote:

> On Fri, 15 Jul 2016, Mikulas Patocka wrote:
> 
> > > There is no guarantee that _anything_ can return memory to the mempool,
> > 
> > You misunderstand mempools if you make such claims.
> > 
> > There is in fact guarantee that objects will be returned to mempool. In 
> > the past I reviewed device mapper thoroughly to make sure that it can make 
> > forward progress even if there is no available memory.
> > 
> > I don't know what should I tell you if you keep on repeating the same 
> > false claim over and over again. Should I explain mempool oprerations to 
> > you in detail? Or will you find it on your own?
> > 
> 
> If you are talking about patches you're proposing for 4.8 or any guarantee 
> of memory freeing that the oom killer/reaper will provide in 4.8, that's 
> fine.  However, the state of the 4.7 kernel is the same as it was when I 
> fixed this issue that timed out hundreds of our machines and is 
> contradicted by that evidence.  Our machines time out after two hours with 
> the oom victim looping forever in mempool_alloc(), so if there was a 

And what about the oom reaper? It should have freed all victim's pages 
even if the victim is looping in mempool_alloc. Why the oom reaper didn't 
free up memory?

> guarantee that elements would be returned in a completely livelocked 
> kernel in 4.7 or earlier kernels, that would not have been the case.  I 

And what kind of targets do you use in device mapper in the configuration 
that livelocked? Do you use some custom google-developed drivers?

Please describe the whole stack of block I/O devices when this livelock 
happened.

Most device mapper drivers can really make forward progress when they are 
out of memory, so I'm interested what kind of configuration do you have.

> frankly don't care about your patch reviewing of dm mempool usage when 
> dm_request() livelocked our kernel.

If it livelocked, it is a bug in some underlying block driver, not a bug 
in mempool_alloc.

> Feel free to formally propose patches either for 4.7 or 4.8.

Mikulas

[toc] | [prev] | [next] | [standalone]

#1444632

On Fri, 15 Jul 2016, Mikulas Patocka wrote:

> And what about the oom reaper? It should have freed all victim's pages 
> even if the victim is looping in mempool_alloc. Why the oom reaper didn't 
> free up memory?
> 

Is that possible with mlock or shared memory?  Nope.  The oom killer does 
not have the benefit of selecting a process to kill that will likely free 
the most memory or reap the most memory, the choice is configurable by the 
user.

> > guarantee that elements would be returned in a completely livelocked 
> > kernel in 4.7 or earlier kernels, that would not have been the case.  I 
> 
> And what kind of targets do you use in device mapper in the configuration 
> that livelocked? Do you use some custom google-developed drivers?
> 
> Please describe the whole stack of block I/O devices when this livelock 
> happened.
> 
> Most device mapper drivers can really make forward progress when they are 
> out of memory, so I'm interested what kind of configuration do you have.
> 

Kworkers are processing writeback, ext4_writepages() relies on kmem that 
is reclaiming memory itself through kmem_getpages() and they are waiting 
on the oom victim to exit so they endlessly loop in the page allocator 
themselves.  Same situation with __alloc_skb() so we can intermittently 
lose access to hundreds of the machines over the network.  There are no 
custom drivers required for this to happen, the stack trace has already 
been posted of the livelock victim and this can happen for anything in 
filemap_fault() that has TIF_MEMDIE set.

> > frankly don't care about your patch reviewing of dm mempool usage when 
> > dm_request() livelocked our kernel.
> 
> If it livelocked, it is a bug in some underlying block driver, not a bug 
> in mempool_alloc.
> 

Lol, the interface is quite clear and can be modified to allow mempool 
users to set __GFP_NOMEMALLOC on their mempool_alloc() request if they can 
guarantee elements will be returned to the freelist in all situations, 
including system oom situations.  We may revert that ourselves if our 
machines time out once we use a post-4.7 kernel and report that as 
necessary.

[toc] | [prev] | [next] | [standalone]

#1444687

On Fri, 15 Jul 2016, David Rientjes wrote:

> Kworkers are processing writeback, ext4_writepages() relies on kmem that 

ext4_writepages is above device mapper, not below, so how it could block 
device mapper progress?

Do you use device mapper on the top of block loop device? Writing to loop 
is prone to deadlock anyway, you should avoid that in production code.

> is reclaiming memory itself through kmem_getpages() and they are waiting 
> on the oom victim to exit so they endlessly loop in the page allocator 
> themselves.  Same situation with __alloc_skb() so we can intermittently 
> lose access to hundreds of the machines over the network.  There are no 
> custom drivers required for this to happen, the stack trace has already 
> been posted of the livelock victim and this can happen for anything in 
> filemap_fault() that has TIF_MEMDIE set.

Again - filemap_failt() is above device mapper, not below (unless you use 
loop).

> > > frankly don't care about your patch reviewing of dm mempool usage when 
> > > dm_request() livelocked our kernel.
> > 
> > If it livelocked, it is a bug in some underlying block driver, not a bug 
> > in mempool_alloc.
> > 
> 
> Lol, the interface is quite clear and can be modified to allow mempool 
> users to set __GFP_NOMEMALLOC on their mempool_alloc() request if they can 
> guarantee elements will be returned to the freelist in all situations, 

You still didn't post configuration of your block stack, so I have no clue 
why entries are not returned to the mempool.

> including system oom situations.  We may revert that ourselves if our 
> machines time out once we use a post-4.7 kernel and report that as 
> necessary.

Mikulas

[toc] | [prev] | [next] | [standalone]

#1445584

CC Dave Chinner, who I recall had strong opinions on the mempool model

The context is commit f9054c7 ("mm, mempool: only set __GFP_NOMEMALLOC
if there are free elements"), which gives MEMALLOC/TIF_MEMDIE mempool
allocations access to the system emergency reserves when there is no
reserved object currently residing in the mempool.

On Fri, Jul 15, 2016 at 07:21:59AM -0400, Mikulas Patocka wrote:
> On Thu, 14 Jul 2016, David Rientjes wrote:
> 
> > There is no guarantee that _anything_ can return memory to the mempool,
> 
> You misunderstand mempools if you make such claims.

Uhm, fully agreed.

The point of mempools is that they have their own reserves, separate
from the system reserves, to make forward progress in OOM situations.

All mempool object holders promise to make forward progress, and when
memory is depleted, the mempool allocations serialize against each
other. In this case, every allocation has to wait for in-flight IO to
finish to pass the reserved object on to the next IO. That's how the
mempool model is designed. The commit in question breaks this by not
waiting for outstanding object holders and instead quickly depletes
the system reserves. That's a mempool causing a memory deadlock...

David observed systems hanging 2+h inside mempool allocations. But
where would an object holders get stuck? It can't be taking a lock
that the waiting mempool_alloc() is holding, obviously. It also can't
be waiting for another allocation, it makes no sense to use mempools
to guarantee forward progress, but then have the whole sequence rely
on an unguaranteed allocation to succeed after the mempool ones. So
how could a system-wide OOM situation cause a mempool holder to hang?

These hangs are fishy, but it seems reasonable to assume that somebody
is breaking the mempool contract somewhere. The solution can not to be
to abandon the mempool model. f9054c7 should be reverted.

[toc] | [prev] | [next] | [standalone]

#1443539

On Wed 13-07-16 16:53:28, David Rientjes wrote:
> On Wed, 13 Jul 2016, Mikulas Patocka wrote:
> 
> > What are the real problems that f9054c70d28bc214b2857cf8db8269f4f45a5e23 
> > tries to fix?
> > 
> 
> It prevents the whole system from livelocking due to an oom killed process 
> stalling forever waiting for mempool_alloc() to return.  No other threads 
> may be oom killed while waiting for it to exit.

But it is true that the patch has unintended side effect for any mempool
allocation from the reclaim path (aka PF_MEMALLOC context). So do you
think we should rework your additional patch to be explicit about
TIF_MEMDIE? Something like the following (not even compile tested for
illustration). Tetsuo has properly pointed out that this doesn't work
for multithreaded processes reliable but put that aside for now as that
needs a fix on a different layer. I believe we can fix that quite
easily after recent/planned changes.
---
diff --git a/mm/mempool.c b/mm/mempool.c
index 8f65464da5de..ea26d75c8adf 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -322,20 +322,20 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
 
 	might_sleep_if(gfp_mask & __GFP_DIRECT_RECLAIM);
 
+	gfp_mask |= __GFP_NOMEMALLOC;   /* don't allocate emergency reserves */
 	gfp_mask |= __GFP_NORETRY;	/* don't loop in __alloc_pages */
 	gfp_mask |= __GFP_NOWARN;	/* failures are OK */
 
 	gfp_temp = gfp_mask & ~(__GFP_DIRECT_RECLAIM|__GFP_IO);
 
 repeat_alloc:
-	if (likely(pool->curr_nr)) {
-		/*
-		 * Don't allocate from emergency reserves if there are
-		 * elements available.  This check is racy, but it will
-		 * be rechecked each loop.
-		 */
-		gfp_temp |= __GFP_NOMEMALLOC;
-	}
+	/*
+	 * Make sure that the OOM victim will get access to memory reserves
+	 * properly if there are no objects in the pool to prevent from
+	 * livelocks.
+	 */
+	if (!likely(pool->curr_nr) && test_thread_flag(TIF_MEMDIE))
+		gfp_temp &= ~__GFP_NOMEMALLOC;
 
 	element = pool->alloc(gfp_temp, pool->pool_data);
 	if (likely(element != NULL))
@@ -359,7 +359,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
 	 * We use gfp mask w/o direct reclaim or IO for the first round.  If
 	 * alloc failed with that and @pool was empty, retry immediately.
 	 */
-	if ((gfp_temp & ~__GFP_NOMEMALLOC) != gfp_mask) {
+	if ((gfp_temp & __GFP_DIRECT_RECLAIM) != (gfp_mask & __GFP_DIRECT_RECLAIM)) {
 		spin_unlock_irqrestore(&pool->lock, flags);
 		gfp_temp = gfp_mask;
 		goto repeat_alloc;
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1443756

On Thu, 14 Jul 2016, Michal Hocko wrote:

> > It prevents the whole system from livelocking due to an oom killed process 
> > stalling forever waiting for mempool_alloc() to return.  No other threads 
> > may be oom killed while waiting for it to exit.
> 
> But it is true that the patch has unintended side effect for any mempool
> allocation from the reclaim path (aka PF_MEMALLOC context).

If PF_MEMALLOC context is allocating too much memory reserves, then I'd 
argue that is a problem independent of using mempool_alloc() since 
mempool_alloc() can evolve directly into a call to the page allocator.  
How does such a process guarantee that it cannot deplete memory reserves 
with a simple call to the page allocator?  Since nothing in the page 
allocator is preventing complete depletion of reserves (it simply uses 
ALLOC_NO_WATERMARKS), the caller in a PF_MEMALLOC context must be 
responsible.

> So do you
> think we should rework your additional patch to be explicit about
> TIF_MEMDIE?

Not sure which additional patch you're referring to, the only patch that I 
proposed was commit f9054c70d28b which solved hundreds of machines from 
timing out.

> Something like the following (not even compile tested for
> illustration). Tetsuo has properly pointed out that this doesn't work
> for multithreaded processes reliable but put that aside for now as that
> needs a fix on a different layer. I believe we can fix that quite
> easily after recent/planned changes.
> ---
> diff --git a/mm/mempool.c b/mm/mempool.c
> index 8f65464da5de..ea26d75c8adf 100644
> --- a/mm/mempool.c
> +++ b/mm/mempool.c
> @@ -322,20 +322,20 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>  
>  	might_sleep_if(gfp_mask & __GFP_DIRECT_RECLAIM);
>  
> +	gfp_mask |= __GFP_NOMEMALLOC;   /* don't allocate emergency reserves */
>  	gfp_mask |= __GFP_NORETRY;	/* don't loop in __alloc_pages */
>  	gfp_mask |= __GFP_NOWARN;	/* failures are OK */
>  
>  	gfp_temp = gfp_mask & ~(__GFP_DIRECT_RECLAIM|__GFP_IO);
>  
>  repeat_alloc:
> -	if (likely(pool->curr_nr)) {
> -		/*
> -		 * Don't allocate from emergency reserves if there are
> -		 * elements available.  This check is racy, but it will
> -		 * be rechecked each loop.
> -		 */
> -		gfp_temp |= __GFP_NOMEMALLOC;
> -	}
> +	/*
> +	 * Make sure that the OOM victim will get access to memory reserves
> +	 * properly if there are no objects in the pool to prevent from
> +	 * livelocks.
> +	 */
> +	if (!likely(pool->curr_nr) && test_thread_flag(TIF_MEMDIE))
> +		gfp_temp &= ~__GFP_NOMEMALLOC;
>  
>  	element = pool->alloc(gfp_temp, pool->pool_data);
>  	if (likely(element != NULL))
> @@ -359,7 +359,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>  	 * We use gfp mask w/o direct reclaim or IO for the first round.  If
>  	 * alloc failed with that and @pool was empty, retry immediately.
>  	 */
> -	if ((gfp_temp & ~__GFP_NOMEMALLOC) != gfp_mask) {
> +	if ((gfp_temp & __GFP_DIRECT_RECLAIM) != (gfp_mask & __GFP_DIRECT_RECLAIM)) {
>  		spin_unlock_irqrestore(&pool->lock, flags);
>  		gfp_temp = gfp_mask;
>  		goto repeat_alloc;

This is bogus and quite obviously leads to oom livelock: if a process is 
holding a mutex and does mempool_alloc(), since __GFP_WAIT is allowed in 
process context for mempool allocation, it can stall here in an oom 
condition if there are no elements available on the mempool freelist.  If 
the oom victim contends the same mutex, the system livelocks and the same 
bug arises because the holder of the mutex loops forever.  This is the 
exact behavior that f9054c70d28b also fixes.

These aren't hypothetical situations, the patch fixed hundreds of machines 
from regularly timing out.  The fundamental reason is that mempool_alloc() 
must not loop forever in process context: that is needed when the 
allocator is either an oom victim itself or the oom victim is blocked by 
an allocator.  mempool_alloc() must guarantee forward progress in such a 
context.

The end result is that when in PF_MEMALLOC context, allocators must be 
responsible and not deplete all memory reserves.

[toc] | [prev] | [next] | [standalone]

#1443996

On Thu 14-07-16 13:38:42, David Rientjes wrote:
> On Thu, 14 Jul 2016, Michal Hocko wrote:
> 
> > > It prevents the whole system from livelocking due to an oom killed process 
> > > stalling forever waiting for mempool_alloc() to return.  No other threads 
> > > may be oom killed while waiting for it to exit.
> > 
> > But it is true that the patch has unintended side effect for any mempool
> > allocation from the reclaim path (aka PF_MEMALLOC context).
> 
> If PF_MEMALLOC context is allocating too much memory reserves, then I'd 
> argue that is a problem independent of using mempool_alloc() since 
> mempool_alloc() can evolve directly into a call to the page allocator.  
> How does such a process guarantee that it cannot deplete memory reserves 
> with a simple call to the page allocator?  Since nothing in the page 
> allocator is preventing complete depletion of reserves (it simply uses 
> ALLOC_NO_WATERMARKS), the caller in a PF_MEMALLOC context must be 
> responsible.

Well, the reclaim throttles the allocation request if there are too many
pages under writeback and that should slow down the allocation rate and
give the writeback some time to complete. But yes you are right there is
nothing to prevent from memory depletion and it is really hard to come
up with something with no fail semantic.

Or do you have an idea how to throttle withou knowing how much memory
will be actually consumed on the writeout path?

> > So do you
> > think we should rework your additional patch to be explicit about
> > TIF_MEMDIE?
> 
> Not sure which additional patch you're referring to, the only patch that I 
> proposed was commit f9054c70d28b which solved hundreds of machines from 
> timing out.

I would like separate TIF_MEMDIE as an access to memory reserves from
oom selection selection semantic. And let me repeat your proposed patch
has a undesirable side effects so we should think about a way to deal
with those cases. It might work for your setups but it shouldn't break
others at the same time. OOM situation is quite unlikely compared to
simple memory depletion by writing to a swap...
 
> > Something like the following (not even compile tested for
> > illustration). Tetsuo has properly pointed out that this doesn't work
> > for multithreaded processes reliable but put that aside for now as that
> > needs a fix on a different layer. I believe we can fix that quite
> > easily after recent/planned changes.
> > ---
> > diff --git a/mm/mempool.c b/mm/mempool.c
> > index 8f65464da5de..ea26d75c8adf 100644
> > --- a/mm/mempool.c
> > +++ b/mm/mempool.c
> > @@ -322,20 +322,20 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> >  
> >  	might_sleep_if(gfp_mask & __GFP_DIRECT_RECLAIM);
> >  
> > +	gfp_mask |= __GFP_NOMEMALLOC;   /* don't allocate emergency reserves */
> >  	gfp_mask |= __GFP_NORETRY;	/* don't loop in __alloc_pages */
> >  	gfp_mask |= __GFP_NOWARN;	/* failures are OK */
> >  
> >  	gfp_temp = gfp_mask & ~(__GFP_DIRECT_RECLAIM|__GFP_IO);
> >  
> >  repeat_alloc:
> > -	if (likely(pool->curr_nr)) {
> > -		/*
> > -		 * Don't allocate from emergency reserves if there are
> > -		 * elements available.  This check is racy, but it will
> > -		 * be rechecked each loop.
> > -		 */
> > -		gfp_temp |= __GFP_NOMEMALLOC;
> > -	}
> > +	/*
> > +	 * Make sure that the OOM victim will get access to memory reserves
> > +	 * properly if there are no objects in the pool to prevent from
> > +	 * livelocks.
> > +	 */
> > +	if (!likely(pool->curr_nr) && test_thread_flag(TIF_MEMDIE))
> > +		gfp_temp &= ~__GFP_NOMEMALLOC;
> >  
> >  	element = pool->alloc(gfp_temp, pool->pool_data);
> >  	if (likely(element != NULL))
> > @@ -359,7 +359,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> >  	 * We use gfp mask w/o direct reclaim or IO for the first round.  If
> >  	 * alloc failed with that and @pool was empty, retry immediately.
> >  	 */
> > -	if ((gfp_temp & ~__GFP_NOMEMALLOC) != gfp_mask) {
> > +	if ((gfp_temp & __GFP_DIRECT_RECLAIM) != (gfp_mask & __GFP_DIRECT_RECLAIM)) {
> >  		spin_unlock_irqrestore(&pool->lock, flags);
> >  		gfp_temp = gfp_mask;
> >  		goto repeat_alloc;
> 
> This is bogus and quite obviously leads to oom livelock: if a process is 
> holding a mutex and does mempool_alloc(), since __GFP_WAIT is allowed in 
> process context for mempool allocation, it can stall here in an oom 
> condition if there are no elements available on the mempool freelist.  If 
> the oom victim contends the same mutex, the system livelocks and the same 
> bug arises because the holder of the mutex loops forever.  This is the 
> exact behavior that f9054c70d28b also fixes.

Just to make sure I understand properly:
Task A				Task B			Task C
current->flags = PF_MEMALLOC
mutex_lock(&foo)		mutex_lock(&foo)	out_of_memory
mempool_alloc()						  select_bad__process = Task B
  alloc_pages(__GFP_NOMEMALLOC)


That would be really unfortunate but it doesn't really differ much from
other oom deadlocks when the victim is stuck behind an allocating task.
This is a generic problem and our answer for that is the oom reaper
which will tear down the address space of the victim asynchronously.
Sure there is no guarantee it will free enough to get us unstuck because
we are freeing only private unlocked memory but we rather fallback to
another oom victim if the situation prevails even after the unmapping
pass. So we shouldn't be stuck for ever.

That being said should we rely for the mempool allocations the same as
any other oom deadlock due to locks?

> These aren't hypothetical situations, the patch fixed hundreds of machines 
> from regularly timing out.  The fundamental reason is that mempool_alloc() 
> must not loop forever in process context: that is needed when the 
> allocator is either an oom victim itself or the oom victim is blocked by 
> an allocator.  mempool_alloc() must guarantee forward progress in such a 
> context.
> 
> The end result is that when in PF_MEMALLOC context, allocators must be 
> responsible and not deplete all memory reserves.

How do you propose to guarantee that? You might have really complex IO
setup and mempools have been the answer for guaranteeing forward progress
for ages.

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1444047

Let me paste the patch with the full changelog and the explanation so
that we can reason about it more easily. If I am making some false
assumptions then please point them out.
--- 
From ed46e3f7f5a6e896331eeadc9d09e2796acb3d01 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Thu, 14 Jul 2016 19:31:54 +0200
Subject: [PATCH] mempool: do not consume memory reserves from the reclaim path

There has been a report about OOM killer invoked when swapping out to
a dm-crypt device. The primary reason seems to be that the swapout
out IO managed to completely deplete memory reserves. Mikulas was
able to bisect and explained the issue by pointing to f9054c70d28b
("mm, mempool: only set __GFP_NOMEMALLOC if there are free elements").

The reason is that the swapout path is not throttled properly because
the md layer needs to allocate from the generic_make_request path
which means it allocates from the PF_MEMALLOC context. dm layer uses
mempool_alloc in order to guarantee a forward progress which used to
inhibit access to memory reserves when using page allocator. This has
changed by f9054c70d28b ("mm, mempool: only set __GFP_NOMEMALLOC if
there are free elements") which has dropped the __GFP_NOMEMALLOC
protection when the memory pool is depleted.

If we are running out of memory and the only way forward to free memory
is to perform swapout we just keep consuming memory reserves rather than
throttling the mempool allocations and allowing the pending IO to
complete up to a moment when the memory is depleted completely and there
is no way forward but invoking the OOM killer. This is less than
optimal.

The original intention of f9054c70d28b was to help with the OOM
situations where the oom victim depends on mempool allocation to make a
forward progress. We can handle that case in a different way, though. We
can check whether the current task has access to memory reserves ad an
OOM victim (TIF_MEMDIE) and drop __GFP_NOMEMALLOC protection if the pool
is empty.

David Rientjes was objecting that such an approach wouldn't help if the
oom victim was blocked on a lock held by process doing mempool_alloc. This
is very similar to other oom deadlock situations and we have oom_reaper
to deal with them so it is reasonable to rely on the same mechanism
rather inventing a different one which has negative side effects.

Fixes: f9054c70d28b ("mm, mempool: only set __GFP_NOMEMALLOC if there are free elements")
Bisected-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/mempool.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/mm/mempool.c b/mm/mempool.c
index 8f65464da5de..ea26d75c8adf 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -322,20 +322,20 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
 
 	might_sleep_if(gfp_mask & __GFP_DIRECT_RECLAIM);
 
+	gfp_mask |= __GFP_NOMEMALLOC;   /* don't allocate emergency reserves */
 	gfp_mask |= __GFP_NORETRY;	/* don't loop in __alloc_pages */
 	gfp_mask |= __GFP_NOWARN;	/* failures are OK */
 
 	gfp_temp = gfp_mask & ~(__GFP_DIRECT_RECLAIM|__GFP_IO);
 
 repeat_alloc:
-	if (likely(pool->curr_nr)) {
-		/*
-		 * Don't allocate from emergency reserves if there are
-		 * elements available.  This check is racy, but it will
-		 * be rechecked each loop.
-		 */
-		gfp_temp |= __GFP_NOMEMALLOC;
-	}
+	/*
+	 * Make sure that the OOM victim will get access to memory reserves
+	 * properly if there are no objects in the pool to prevent from
+	 * livelocks.
+	 */
+	if (!likely(pool->curr_nr) && test_thread_flag(TIF_MEMDIE))
+		gfp_temp &= ~__GFP_NOMEMALLOC;
 
 	element = pool->alloc(gfp_temp, pool->pool_data);
 	if (likely(element != NULL))
@@ -359,7 +359,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
 	 * We use gfp mask w/o direct reclaim or IO for the first round.  If
 	 * alloc failed with that and @pool was empty, retry immediately.
 	 */
-	if ((gfp_temp & ~__GFP_NOMEMALLOC) != gfp_mask) {
+	if ((gfp_temp & __GFP_DIRECT_RECLAIM) != (gfp_mask & __GFP_DIRECT_RECLAIM)) {
 		spin_unlock_irqrestore(&pool->lock, flags);
 		gfp_temp = gfp_mask;
 		goto repeat_alloc;
-- 
2.8.1

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1444232

On Fri, 15 Jul 2016, Michal Hocko wrote:

> On Thu 14-07-16 13:38:42, David Rientjes wrote:
> > On Thu, 14 Jul 2016, Michal Hocko wrote:
> > 
> > > > It prevents the whole system from livelocking due to an oom killed process 
> > > > stalling forever waiting for mempool_alloc() to return.  No other threads 
> > > > may be oom killed while waiting for it to exit.
> > > 
> > > But it is true that the patch has unintended side effect for any mempool
> > > allocation from the reclaim path (aka PF_MEMALLOC context).
> > 
> > If PF_MEMALLOC context is allocating too much memory reserves, then I'd 
> > argue that is a problem independent of using mempool_alloc() since 
> > mempool_alloc() can evolve directly into a call to the page allocator.  
> > How does such a process guarantee that it cannot deplete memory reserves 
> > with a simple call to the page allocator?  Since nothing in the page 
> > allocator is preventing complete depletion of reserves (it simply uses 
> > ALLOC_NO_WATERMARKS), the caller in a PF_MEMALLOC context must be 
> > responsible.

Well-written drivers should use mempools and not allocate pages directly 
from the allocator. These drivers can proceed even if there is no memory 
available.

Badly written drivers (loop block device; swapping to NFS) are prone to 
deadlock anyway - there is no easy way to fix it.

> Well, the reclaim throttles the allocation request if there are too many
> pages under writeback and that should slow down the allocation rate and
> give the writeback some time to complete. But yes you are right there is
> nothing to prevent from memory depletion and it is really hard to come
> up with something with no fail semantic.
> 
> Or do you have an idea how to throttle withou knowing how much memory
> will be actually consumed on the writeout path?
> 
> > > So do you
> > > think we should rework your additional patch to be explicit about
> > > TIF_MEMDIE?
> > 
> > Not sure which additional patch you're referring to, the only patch that I 
> > proposed was commit f9054c70d28b which solved hundreds of machines from 
> > timing out.
> 
> I would like separate TIF_MEMDIE as an access to memory reserves from
> oom selection selection semantic. And let me repeat your proposed patch
> has a undesirable side effects so we should think about a way to deal
> with those cases. It might work for your setups but it shouldn't break
> others at the same time. OOM situation is quite unlikely compared to
> simple memory depletion by writing to a swap...
>  
> > > Something like the following (not even compile tested for
> > > illustration). Tetsuo has properly pointed out that this doesn't work
> > > for multithreaded processes reliable but put that aside for now as that
> > > needs a fix on a different layer. I believe we can fix that quite
> > > easily after recent/planned changes.
> > > ---
> > > diff --git a/mm/mempool.c b/mm/mempool.c
> > > index 8f65464da5de..ea26d75c8adf 100644
> > > --- a/mm/mempool.c
> > > +++ b/mm/mempool.c
> > > @@ -322,20 +322,20 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> > >  
> > >  	might_sleep_if(gfp_mask & __GFP_DIRECT_RECLAIM);
> > >  
> > > +	gfp_mask |= __GFP_NOMEMALLOC;   /* don't allocate emergency reserves */
> > >  	gfp_mask |= __GFP_NORETRY;	/* don't loop in __alloc_pages */
> > >  	gfp_mask |= __GFP_NOWARN;	/* failures are OK */
> > >  
> > >  	gfp_temp = gfp_mask & ~(__GFP_DIRECT_RECLAIM|__GFP_IO);
> > >  
> > >  repeat_alloc:
> > > -	if (likely(pool->curr_nr)) {
> > > -		/*
> > > -		 * Don't allocate from emergency reserves if there are
> > > -		 * elements available.  This check is racy, but it will
> > > -		 * be rechecked each loop.
> > > -		 */
> > > -		gfp_temp |= __GFP_NOMEMALLOC;
> > > -	}
> > > +	/*
> > > +	 * Make sure that the OOM victim will get access to memory reserves
> > > +	 * properly if there are no objects in the pool to prevent from
> > > +	 * livelocks.
> > > +	 */
> > > +	if (!likely(pool->curr_nr) && test_thread_flag(TIF_MEMDIE))
> > > +		gfp_temp &= ~__GFP_NOMEMALLOC;
> > >  
> > >  	element = pool->alloc(gfp_temp, pool->pool_data);
> > >  	if (likely(element != NULL))
> > > @@ -359,7 +359,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
> > >  	 * We use gfp mask w/o direct reclaim or IO for the first round.  If
> > >  	 * alloc failed with that and @pool was empty, retry immediately.
> > >  	 */
> > > -	if ((gfp_temp & ~__GFP_NOMEMALLOC) != gfp_mask) {
> > > +	if ((gfp_temp & __GFP_DIRECT_RECLAIM) != (gfp_mask & __GFP_DIRECT_RECLAIM)) {
> > >  		spin_unlock_irqrestore(&pool->lock, flags);
> > >  		gfp_temp = gfp_mask;
> > >  		goto repeat_alloc;
> > 
> > This is bogus and quite obviously leads to oom livelock: if a process is 
> > holding a mutex and does mempool_alloc(), since __GFP_WAIT is allowed in 
> > process context for mempool allocation, it can stall here in an oom 
> > condition if there are no elements available on the mempool freelist.  If 
> > the oom victim contends the same mutex, the system livelocks and the same 
> > bug arises because the holder of the mutex loops forever.  This is the 
> > exact behavior that f9054c70d28b also fixes.
> 
> Just to make sure I understand properly:
> Task A				Task B			Task C
> current->flags = PF_MEMALLOC
> mutex_lock(&foo)		mutex_lock(&foo)	out_of_memory
> mempool_alloc()						  select_bad__process = Task B
>   alloc_pages(__GFP_NOMEMALLOC)
> 
> 
> That would be really unfortunate but it doesn't really differ much from
> other oom deadlocks when the victim is stuck behind an allocating task.
> This is a generic problem and our answer for that is the oom reaper
> which will tear down the address space of the victim asynchronously.
> Sure there is no guarantee it will free enough to get us unstuck because
> we are freeing only private unlocked memory but we rather fallback to
> another oom victim if the situation prevails even after the unmapping
> pass. So we shouldn't be stuck for ever.
> 
> That being said should we rely for the mempool allocations the same as
> any other oom deadlock due to locks?

I think that mempool deadlock is really non-existent issue.

mempool for device mapper and most block device drivers guarantees forward 
progress even if there is no memory available. It doesn't deadlock as long 
as the underlying block device is processing requests. There is no reason 
why should it deadlock when OOM killer is activated.

> > These aren't hypothetical situations, the patch fixed hundreds of machines 
> > from regularly timing out.  The fundamental reason is that mempool_alloc() 
> > must not loop forever in process context: that is needed when the 
> > allocator is either an oom victim itself or the oom victim is blocked by 
> > an allocator.  mempool_alloc() must guarantee forward progress in such a 
> > context.
> > 
> > The end result is that when in PF_MEMALLOC context, allocators must be 
> > responsible and not deplete all memory reserves.
> 
> How do you propose to guarantee that? You might have really complex IO
> setup and mempools have been the answer for guaranteeing forward progress
> for ages.
> 
> -- 
> Michal Hocko
> SUSE Labs

Mikulas

[toc] | [prev] | [next] | [standalone]

#1444618

On Fri, 15 Jul 2016, Michal Hocko wrote:

> > If PF_MEMALLOC context is allocating too much memory reserves, then I'd 
> > argue that is a problem independent of using mempool_alloc() since 
> > mempool_alloc() can evolve directly into a call to the page allocator.  
> > How does such a process guarantee that it cannot deplete memory reserves 
> > with a simple call to the page allocator?  Since nothing in the page 
> > allocator is preventing complete depletion of reserves (it simply uses 
> > ALLOC_NO_WATERMARKS), the caller in a PF_MEMALLOC context must be 
> > responsible.
> 
> Well, the reclaim throttles the allocation request if there are too many
> pages under writeback and that should slow down the allocation rate and
> give the writeback some time to complete. But yes you are right there is
> nothing to prevent from memory depletion and it is really hard to come
> up with something with no fail semantic.
> 

If the reclaimer is allocating memory, it can fully deplete memory 
reserves with ALLOC_NO_WATERMARKS without any direct reclaim itself and 
we're relying on kswapd entirely if nothing else is reclaiming in parallel 
(and depleting memory reserves itself in parallel).  It's a difficult 
problem because memory reserves can be very small and concurrent 
PF_MEMALLOC allocation contexts can lead to quick depletion.  I don't 
think it's a throttling problem itself, it's more scalability.

> I would like separate TIF_MEMDIE as an access to memory reserves from
> oom selection selection semantic. And let me repeat your proposed patch
> has a undesirable side effects so we should think about a way to deal
> with those cases. It might work for your setups but it shouldn't break
> others at the same time. OOM situation is quite unlikely compared to
> simple memory depletion by writing to a swap...
>  

I haven't proposed any patch, not sure what the reference is to.  There's 
two fundamental ways to go about it: (1) ensure mempool_alloc() can make 
forward progress (whether that's by way of gfp flags or access to memory 
reserves, which may depend on the process context such as PF_MEMALLOC) or 
(2) rely on an implementation detail of mempools to never access memory 
reserves, although it is shown to not livelock systems on 4.7 and earlier 
kernels, and instead rely on users of the same mempool to return elements 
to the freelist in all contexts, including oom contexts.  The mempool 
implementation itself shouldn't need any oom awareness, that should be a 
page allocator issue.

If the mempool user can guarantee that elements will be returned to the 
freelist in all contexts, we could relax the restriction that mempool 
users cannot use __GFP_NOMEMALLOC and leave it up to them to prevent 
access to memory reserves but only in situations where forward progress 
can be guaranteed.  That's a simple change and doesn't change mempool or 
page allocator behavior for everyone, but rather only for those that 
opt-in.  I think this is the way the dm folks should proceed, but let's 
not encode any special restriction on access to memory reserves as an 
implementation detail to mempools, specifically for processes that have 
PF_MEMALLOC set.

> Just to make sure I understand properly:
> Task A				Task B			Task C
> current->flags = PF_MEMALLOC
> mutex_lock(&foo)		mutex_lock(&foo)	out_of_memory
> mempool_alloc()						  select_bad__process = Task B
>   alloc_pages(__GFP_NOMEMALLOC)
> 

Not sure who is grabbing foo first with this, I assume Task A and Task B 
is contending.  If that's the case, then yes, this is the dm_request() oom 
livelock that went unresolved for two hours on our machines and timed 
them all out.  This is a swapless environment that heavily oversubscribes 
the machine, so not everybody's use case, but it needs to be resolved.

> That would be really unfortunate but it doesn't really differ much from
> other oom deadlocks when the victim is stuck behind an allocating task.

I'm well aware of many of the system oom and memcg oom livelocks from 
experience, unfortunately :)

[toc] | [prev] | [next] | [standalone]

#1445279

On Fri 15-07-16 14:47:30, David Rientjes wrote:
> On Fri, 15 Jul 2016, Michal Hocko wrote:
[...]
> > And let me repeat your proposed patch
> > has a undesirable side effects so we should think about a way to deal
> > with those cases. It might work for your setups but it shouldn't break
> > others at the same time. OOM situation is quite unlikely compared to
> > simple memory depletion by writing to a swap...
> >  
> 
> I haven't proposed any patch, not sure what the reference is to.

I was talking about f9054c70d28b ("mm, mempool: only set
__GFP_NOMEMALLOC if there are free elements"). Do you at least recognize
it has caused a regression which is more likely than the OOM lockup you
are referring to and that might be very specific to your particular
workload? I would really like to move on here and come up with a fix
which can handle dm-crypt swapout gracefully and also deal with the
typical case when the OOM victim is inside the mempool_alloc which
should help your usecase as well (at least the writeout path).

> There's 
> two fundamental ways to go about it: (1) ensure mempool_alloc() can make 
> forward progress (whether that's by way of gfp flags or access to memory 
> reserves, which may depend on the process context such as PF_MEMALLOC) or 
> (2) rely on an implementation detail of mempools to never access memory 
> reserves, although it is shown to not livelock systems on 4.7 and earlier 
> kernels, and instead rely on users of the same mempool to return elements 
> to the freelist in all contexts, including oom contexts.  The mempool 
> implementation itself shouldn't need any oom awareness, that should be a 
> page allocator issue.

OK, I agree that we have a certain layer violation here. __GFP_NOMEMALLOC at
the mempool level is kind of hack (like the whole existence of the
flag TBH). So if you believe that the OOM part should be handled at the
page allocator level then that has already been proposed
http://lkml.kernel.org/r/2d5e1f84-e886-7b98-cb11-170d7104fd13@I-love.SAKURA.ne.jp
and not welcome because it might have other side effects as _all_
__GFP_NOMEMALLOC users would be affected.

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1445846

On Mon, 18 Jul 2016, Michal Hocko wrote:

> > There's 
> > two fundamental ways to go about it: (1) ensure mempool_alloc() can make 
> > forward progress (whether that's by way of gfp flags or access to memory 
> > reserves, which may depend on the process context such as PF_MEMALLOC) or 
> > (2) rely on an implementation detail of mempools to never access memory 
> > reserves, although it is shown to not livelock systems on 4.7 and earlier 
> > kernels, and instead rely on users of the same mempool to return elements 
> > to the freelist in all contexts, including oom contexts.  The mempool 
> > implementation itself shouldn't need any oom awareness, that should be a 
> > page allocator issue.
> 
> OK, I agree that we have a certain layer violation here. __GFP_NOMEMALLOC at
> the mempool level is kind of hack (like the whole existence of the
> flag TBH). So if you believe that the OOM part should be handled at the
> page allocator level then that has already been proposed
> http://lkml.kernel.org/r/2d5e1f84-e886-7b98-cb11-170d7104fd13@I-love.SAKURA.ne.jp
> and not welcome because it might have other side effects as _all_
> __GFP_NOMEMALLOC users would be affected.
> 

__GFP_NOMEMALLOC is opt-in and is a workaround for PF_MEMALLOC in this 
context to prevent a depletion of reserves, so it seems trivial to allow 
mempool_alloc(__GFP_NOMEMALLOC) in contexts where it's needed and leave it 
to the user.

[toc] | [prev] | [next] | [standalone]

#1442950

On Wed, 13 Jul 2016, Tetsuo Handa wrote:

> I wonder whether commit f9054c70d28bc214 ("mm, mempool: only set
> __GFP_NOMEMALLOC if there are free elements") is doing correct thing.
> It says
> 
>     If an oom killed thread calls mempool_alloc(), it is possible that it'll
>     loop forever if there are no elements on the freelist since
>     __GFP_NOMEMALLOC prevents it from accessing needed memory reserves in
>     oom conditions.
> 
> but we can allow mempool_alloc(__GFP_NOMEMALLOC) requests to access
> memory reserves via below change, can't we? The purpose of allowing
> ALLOC_NO_WATERMARKS via TIF_MEMDIE is to make sure current allocation
> request does not to loop forever inside the page allocator, isn't it?

This would defeat the purpose of __GFP_NOMEMALLOC for oom killed threads, 
so you'd need to demonstrate that isn't a problem for the current users 
and then change the semantics of the gfp flag.

> Why we need to allow mempool_alloc(__GFP_NOMEMALLOC) requests to use
> ALLOC_NO_WATERMARKS when TIF_MEMDIE is not set?
> 

mempool_alloc(__GFP_NOMEMALLOC) is forbidden.

[toc] | [prev] | [standalone]

Page 3 of 3 — ← Prev page 1 2 [3]

Back to top | Article view | linux.kernel

csiph-web