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

Groups > linux.kernel > #1158877 > unrolled thread

Re: [PATCH] oom: always panic on OOM when panic_on_oom is configured

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: [PATCH] oom: always panic on OOM when panic_on_oom is  configured David Rientjes <rientjes@google.com> - 2015-06-05 01:20 +0200
    Re: [PATCH] oom: always panic on OOM when panic_on_oom is configured Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2015-06-08 14:00 +0200
    Re: [PATCH] oom: always panic on OOM when panic_on_oom is  configured David Rientjes <rientjes@google.com> - 2015-06-08 22:00 +0200
      Re: [PATCH] oom: always panic on OOM when panic_on_oom is  configured David Rientjes <rientjes@google.com> - 2015-06-11 02:40 +0200
    Re: [PATCH] oom: always panic on OOM when panic_on_oom is  configured David Rientjes <rientjes@google.com> - 2015-06-08 22:00 +0200

#1158877 — Re: [PATCH] oom: always panic on OOM when panic_on_oom is configured

On Mon, 1 Jun 2015, Michal Hocko wrote:

> panic_on_oom allows administrator to set OOM policy to panic the system
> when it is out of memory to reduce failover time e.g. when resolving
> the OOM condition would take much more time than rebooting the system.
> 
> out_of_memory tries to be clever and prevent from premature panics
> by checking the current task and prevent from panic when the task
> has fatal signal pending and so it should die shortly and release some
> memory. This is fair enough but Tetsuo Handa has noted that this might
> lead to a silent deadlock when current cannot exit because of
> dependencies invisible to the OOM killer.
> 
> panic_on_oom is disabled by default and if somebody enables it then any
> risk of potential deadlock is certainly unwelcome. The risk is really
> low because there are usually more sources of allocation requests and
> one of them would eventually trigger the panic but it is better to
> reduce the risk as much as possible.
> 
> Let's move check_panic_on_oom up before the current task is
> checked so that the knob value is . Do the same for the memcg in
> mem_cgroup_out_of_memory.
> 
> Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Signed-off-by: Michal Hocko <mhocko@suse.cz>

Nack, this is not the appropriate response to exit path livelocks.  By 
doing this, you are going to start unnecessarily panicking machines that 
have panic_on_oom set when it would not have triggered before.  If there 
is no reclaimable memory and a process that has already been signaled to 
die to is in the process of exiting has to allocate memory, it is 
perfectly acceptable to give them access to memory reserves so they can 
allocate and exit.  Under normal circumstances, that allows the process to 
naturally exit.  With your patch, it will cause the machine to panic.

It's this simple: panic_on_oom is not a solution to workaround oom killer 
livelocks and shouldn't be suggested as the canonical way that such 
possibilities should be addressed.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1160422 — Re: [PATCH] oom: always panic on OOM when panic_on_oom is configured

Michal Hocko wrote:
> On Sat 06-06-15 15:51:35, Tetsuo Handa wrote:
> > For me, !__GFP_FS allocations not calling out_of_memory() _forever_ is a
> > violation of the user policy.
> 
> Yes, the current behavior of GFP_NOFS is highly suboptimal, but this has
> _nothing_ what so ever to do with this patch and panic_on_oom handling.
> The former one is the page allocator proper while we are in the OOM
> killer layer here.
> 
> This is not the first time you have done that. Please stop it. It makes
> a complete mess of the original discussions.

My specific area of expertise in Linux kernel is security/tomoyo/ directory.
What I could contribute with other areas is to pay attention to potential
possibilities. But I'm such outsider about other areas that I can't
differentiate the page allocator proper from the OOM killer layer.

Excuse me for focusing on the wrong issue, but I will likely unconsciously
make the same mistake again. Please ignore or point out when I made mistakes.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1160745

On Fri, 5 Jun 2015, Michal Hocko wrote:

> > Nack, this is not the appropriate response to exit path livelocks.  By 
> > doing this, you are going to start unnecessarily panicking machines that 
> > have panic_on_oom set when it would not have triggered before.  If there 
> > is no reclaimable memory and a process that has already been signaled to 
> > die to is in the process of exiting has to allocate memory, it is 
> > perfectly acceptable to give them access to memory reserves so they can 
> > allocate and exit.  Under normal circumstances, that allows the process to 
> > naturally exit.  With your patch, it will cause the machine to panic.
> 
> Isn't that what the administrator of the system wants? The system
> is _clearly_ out of memory at this point. A coincidental exiting task
> doesn't change a lot in that regard. Moreover it increases a risk of
> unnecessarily unresponsive system which is what panic_on_oom tries to
> prevent from. So from my POV this is a clear violation of the user
> policy.
> 

We rely on the functionality that this patch is short cutting because we 
rely on userspace to trigger oom kills.  For system oom conditions, we 
must then rely on the kernel oom killer to set TIF_MEMDIE since userspace 
cannot grant it itself.  (I think the memcg case is very similar in that 
this patch is short cutting it, but I'm more concerned for the system oom 
in this case because it's a show stopper for us.)

We want to send the SIGKILL, which will interrupt things like 
get_user_pages() which we find is our culprit most of the time.  When the 
process enters the exit path, it must allocate other memory (slab, 
coredumping and the very problematic proc_exit_connector()) to free 
memory.  This patch would cause the machine to panic rather than utilizing 
memory reserves so that it can exit, not as a result of a kernel oom kill 
but rather a userspace kill.

Panic_on_oom is to suppress the kernel oom killer.  It's not a sysctl that 
triggers whenever watermarks are hit and it doesn't suppress memory 
reserves from being used for things like GFP_ATOMIC.  Setting TIF_MEMDIE 
for an exiting process is another type of memory reserves and is 
imperative that we have it to make forward progress.  Panic_on_oom should 
only trigger when the kernel can't make forward progress without killing 
something (not true in this case).  I believe that's how the documentation 
has always been interpreted and the tunable used in the wild.

It would be interesting to consider your other patch that refactors the 
sysrq+f tunable.  I think we should make that never trigger panic_on_oom 
(the sysadmin can use other sysrqs for that) and allow userspace to use 
sysrq+f as a trigger when it is responsive to handle oom conditions.

But this patch itself can't possibly be merged.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1162951

On Wed, 10 Jun 2015, Michal Hocko wrote:

> > Not necessarily.  We pin a lot of memory with get_user_pages() and 
> > short-circuit it by checking for fatal_signal_pending() specifically for 
> > oom conditions.  This was done over six years ago by commit 4779280d1ea4 
> > ("mm: make get_user_pages() interruptible").  When such a process is 
> > faulting in memory, and it is killed by userspace as a result of an oom 
> > condition, it needs to be able to allocate (TIF_MEMDIE set by the oom 
> > killer due to SIGKILL), return to __get_user_pages(), abort, handle the 
> > signal, and exit.
> > 
> > I can't possibly make that any more clear.
> 
> Are you even reading what I've written? I will ask for the last
> time. What exactly prevents other allocation to trigger to oom path and
> panic the system before the killed task has a chance to terminate?
> 

If there are other threads that call into the oom killer that are not in 
the exit path or have a SIGKILL to handle, then the machine panics.  
That's the purpose of panic_on_oom: the kernel has no way to free memory 
without killing a process, so the admin has chosen to panic rather than 
wait for memory to become available, which may never happen.

This is how panic_on_oom has always worked.

> > Your patch causes that to instead panic the system if panic_on_oom is set.  
> > It's inappropriate and userspace breakage.  The fact that I don't 
> > personally use panic_on_oom is completely and utterly irrelevant.
> > 
> > There is absolutely nothing wrong with a process that has been killed 
> > either directly by userspace or as part of a group exit, or a process that 
> > is already in the exit path and needs to allocate memory to be able to 
> > free its memory, to get access to memory reserves.  That's not an oom 
> > condition, that's memory reserves.  Panic_on_oom has nothing to do with 
> > this scenario whatsoever.
> 
> It very much has and I have presented arguments about that which you
> didn't bother to comment on. TIF_MEMDIE is not a magic which will help a
> task to exit in all cases. It is a heuristic and it can fail.
> panic_on_oops is a hand break when things go wrong and you want to
> reduce your unresponsive time (read failover part in the documentation).
> 

Threads that have been oom killed and have TIF_MEMDIE set should exit.  
It's certainly a problem if they do not, since the oom killer relies on it 
and will defer forever until it does exit.  (We don't actually require 
that the thread fully exit, we just require that its memory is freed.)  If 
you're trying to address the issue that Tetsuo Handa brought up (strange, 
because you seemed to not want Tetsuo to talk), then that needs to be 
handled in a way that makes forward progress.  I suggested three methods 
for doing that in this thread that can be pursued to do that, but 
panicking the system is not one of them.

> > Panic_on_oom is not panic_when_reclaim_fails. 
> 
> OOM is when all other reclaim attempts fail. Jeez we are in
> out_of_memory how can this be potentially unclear to you? Yes oom killer
> path might use heuristics to reduce the impact of the OOM condition but
> once we are in this path _we_are_OOM_.
> 

Hmm, not exactly.  You can't make the same argument for GFP_ATOMIC 
allocations, for instance, where we don't have the ability to reclaim.  
They get access to a memory reserve so they may succeed in this context.  
In the case your patch is short-circuiting, a GFP_KERNEL allocation can 
fail to reclaim and then you've decided to panic rather than give an 
exiting thread access to memory reserves.  It's unnecessary.

(I personally don't care what you do or do not label "oom", I only care 
about panic vs. no-panic when the kernel has the ability to allow the 
allocation to succeed and make forward progress.)

Let me be clear: the issue that Tetsuo brings up is very real and serious.  
It exists for system memory as well as memcg.  Trying to address it with 
panic_on_oom is absurd.  It may be difficult to address, and require 
substantial VM work to fix, but panicking is not a solution and would lead 
to arbitrary machines in a very large fleet rebooting.  There's nothing 
the userspace programmer could have done differently to prevent it, this 
is solely a kernel issue.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1160748

On Sat, 6 Jun 2015, Tetsuo Handa wrote:

> For me, !__GFP_FS allocations not calling out_of_memory() _forever_ is a
> violation of the user policy.
> 

I agree that we need work in this area to prevent livelocks that rely on 
the contexts of other allocators to make forward progress, but let's be 
clear that panicking the system is not the appropriate response.  It's 
nice and convenient to say we should repurpose panic_on_oom to solve 
livelocks because it triggers a fast reboot in your configuration, but we, 
and certainly others, can't tolerate reboots when the kernel just gives up 
and the majority of the time these situations do resolve themselves.

I think the appropriate place to be attacking this problem is in the page 
allocator, which is responsible for the page allocations in the context of 
the gfp flags given to it, and not the oom killer.  The oom killer is just 
supposed to pick a process, send it a SIGKILL, and give it a reasonable 
expectation of being able to exit.

> If kswapd found nothing more to reclaim and/or kswapd cannot continue
> reclaiming due to lock dependency, can't we consider as out of memory
> because we already tried to reclaim memory which would have been done by
> __GFP_FS allocations?
> 
> Why do we do "!__GFP_FS allocations do not call out_of_memory() because
> they have very limited reclaim ability"? Both GFP_NOFS and GFP_NOIO
> allocations will wake up kswapd due to !__GFP_NO_KSWAPD, doesn't it?
> 

The !__GFP_FS exception is historic because the oom killer would trigger 
waaay too often if it were removed simply because it doesn't have a great 
chance of allowing reclaim to succeed.  Instead, we rely on external 
memory freeing or other parallel allocators being able to reclaim memory.  
What happens when there is no external freeing, nothing else is trying to 
reclaim, or nothing else is able to reclaim?  Yeah, that's the big 
problem.  In my opinion, there's three ways of attacking it: (1) 
preallocation so we are less dependent on the page allocator in these 
contexts, (2) memory reserves in extraordinary circumstances to allow 
forward progress (it's already tunable by min_free_kbytes), and (3) 
eventual page allocation failure when neither of these succeed.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web