diff mbox series

[memcg,1/3] mm: do not firce global OOM from inside dying tasks

Message ID 2c13c739-7282-e6f4-da0a-c0b69e68581e@virtuozzo.com (mailing list archive)
State New
Headers show
Series memcg: prohibit unconditional exceeding the limit of dying tasks | expand

Commit Message

Vasily Averin Oct. 20, 2021, 12:12 p.m. UTC
There is no sense to force global OOM if current task is dying.

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 mm/oom_kill.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Michal Hocko Oct. 20, 2021, 12:33 p.m. UTC | #1
s@firce@force@

On Wed 20-10-21 15:12:19, Vasily Averin wrote:
> There is no sense to force global OOM if current task is dying.

This really begs for much more information. Feel free to get an
inspiration from my previous attempt to achieve something similar.
In minimum it is important to mention that the OOM killer is already
handled at the page allocator level for the global OOM and at the
charging level for the memcg one. Both have much more information
about the scope of allocation/charge request. This means that either the
OOM killer has been invoked properly and didn't lead to the allocation
success or it has been skipped because it couldn't have been invoked.
In both cases triggering it from here is pointless and even harmful.

Another argument is that it is more reasonable to let killed task die
rather than hit the oom killer and retry the allocation.

> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
> ---
>  mm/oom_kill.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 831340e7ad8b..1deef8c7a71b 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -1137,6 +1137,9 @@ void pagefault_out_of_memory(void)
>  	if (mem_cgroup_oom_synchronize(true))
>  		return;
>  
> +	if (fatal_signal_pending(current))
> +		return;
> +
>  	if (!mutex_trylock(&oom_lock))
>  		return;
>  	out_of_memory(&oc);
> -- 
> 2.32.0
Vasily Averin Oct. 20, 2021, 1:52 p.m. UTC | #2
On 20.10.2021 15:33, Michal Hocko wrote:
> s@firce@force@
> 
> On Wed 20-10-21 15:12:19, Vasily Averin wrote:
>> There is no sense to force global OOM if current task is dying.
> 
> This really begs for much more information. Feel free to get an
> inspiration from my previous attempt to achieve something similar.
> In minimum it is important to mention that the OOM killer is already
> handled at the page allocator level for the global OOM and at the
> charging level for the memcg one. Both have much more information
> about the scope of allocation/charge request. This means that either the
> OOM killer has been invoked properly and didn't lead to the allocation
> success or it has been skipped because it couldn't have been invoked.
> In both cases triggering it from here is pointless and even harmful.
> 
> Another argument is that it is more reasonable to let killed task die
> rather than hit the oom killer and retry the allocation.

Thank you,
I'll update  patch description later,
this time I would like to clarify patch content. 

>> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
>> ---
>>  mm/oom_kill.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
>> index 831340e7ad8b..1deef8c7a71b 100644
>> --- a/mm/oom_kill.c
>> +++ b/mm/oom_kill.c
>> @@ -1137,6 +1137,9 @@ void pagefault_out_of_memory(void)
>>  	if (mem_cgroup_oom_synchronize(true))
>>  		return;
>>  
>> +	if (fatal_signal_pending(current))
>> +		return;
>> +
>>  	if (!mutex_trylock(&oom_lock))
>>  		return;
>>  	out_of_memory(&oc);
>> -- 
>> 2.32.0
>
diff mbox series

Patch

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 831340e7ad8b..1deef8c7a71b 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -1137,6 +1137,9 @@  void pagefault_out_of_memory(void)
 	if (mem_cgroup_oom_synchronize(true))
 		return;
 
+	if (fatal_signal_pending(current))
+		return;
+
 	if (!mutex_trylock(&oom_lock))
 		return;
 	out_of_memory(&oc);