All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roman Gushchin <guro@fb.com>
To: Shakeel Butt <shakeelb@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@suse.com>, Chris Down <chris@chrisdown.name>,
	Andrew Morton <akpm@linux-foundation.org>,
	cgroups@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 4/4] memcg: synchronously enforce memory.high for large overcharges
Date: Tue, 15 Feb 2022 15:27:32 -0800	[thread overview]
Message-ID: <Ygw25FypLsN9ZQaZ@carbon.dhcp.thefacebook.com> (raw)
In-Reply-To: <20220211064917.2028469-5-shakeelb@google.com>

On Thu, Feb 10, 2022 at 10:49:17PM -0800, Shakeel Butt wrote:
> The high limit is used to throttle the workload without invoking the
> oom-killer. Recently we tried to use the high limit to right size our
> internal workloads. More specifically dynamically adjusting the limits
> of the workload without letting the workload get oom-killed. However due
> to the limitation of the implementation of high limit enforcement, we
> observed the mechanism fails for some real workloads.
> 
> The high limit is enforced on return-to-userspace i.e. the kernel let
> the usage goes over the limit and when the execution returns to
> userspace, the high reclaim is triggered and the process can get
> throttled as well. However this mechanism fails for workloads which do
> large allocations in a single kernel entry e.g. applications that
> mlock() a large chunk of memory in a single syscall. Such applications
> bypass the high limit and can trigger the oom-killer.
> 
> To make high limit enforcement more robust, this patch makes the limit
> enforcement synchronous only if the accumulated overcharge becomes
> larger than MEMCG_CHARGE_BATCH. So, most of the allocations would still
> be throttled on the return-to-userspace path but only the extreme
> allocations which accumulates large amount of overcharge without
> returning to the userspace will be throttled synchronously. The value
> MEMCG_CHARGE_BATCH is a bit arbitrary but most of other places in the
> memcg codebase uses this constant therefore for now uses the same one.
> 
> Signed-off-by: Shakeel Butt <shakeelb@google.com>
> ---
> Changes since v1:
> - Based on Roman's comment simply the sync enforcement and only target
>   the extreme cases.

Reviewed-by: Roman Gushchin <guro@fb.com>

This version indeed looks more safe to me.

Thanks!

WARNING: multiple messages have this Message-ID (diff)
From: Roman Gushchin <guro@fb.com>
To: Shakeel Butt <shakeelb@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@suse.com>, Chris Down <chris@chrisdown.name>,
	Andrew Morton <akpm@linux-foundation.org>,
	<cgroups@vger.kernel.org>, <linux-mm@kvack.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 4/4] memcg: synchronously enforce memory.high for large overcharges
Date: Tue, 15 Feb 2022 15:27:32 -0800	[thread overview]
Message-ID: <Ygw25FypLsN9ZQaZ@carbon.dhcp.thefacebook.com> (raw)
In-Reply-To: <20220211064917.2028469-5-shakeelb@google.com>

On Thu, Feb 10, 2022 at 10:49:17PM -0800, Shakeel Butt wrote:
> The high limit is used to throttle the workload without invoking the
> oom-killer. Recently we tried to use the high limit to right size our
> internal workloads. More specifically dynamically adjusting the limits
> of the workload without letting the workload get oom-killed. However due
> to the limitation of the implementation of high limit enforcement, we
> observed the mechanism fails for some real workloads.
> 
> The high limit is enforced on return-to-userspace i.e. the kernel let
> the usage goes over the limit and when the execution returns to
> userspace, the high reclaim is triggered and the process can get
> throttled as well. However this mechanism fails for workloads which do
> large allocations in a single kernel entry e.g. applications that
> mlock() a large chunk of memory in a single syscall. Such applications
> bypass the high limit and can trigger the oom-killer.
> 
> To make high limit enforcement more robust, this patch makes the limit
> enforcement synchronous only if the accumulated overcharge becomes
> larger than MEMCG_CHARGE_BATCH. So, most of the allocations would still
> be throttled on the return-to-userspace path but only the extreme
> allocations which accumulates large amount of overcharge without
> returning to the userspace will be throttled synchronously. The value
> MEMCG_CHARGE_BATCH is a bit arbitrary but most of other places in the
> memcg codebase uses this constant therefore for now uses the same one.
> 
> Signed-off-by: Shakeel Butt <shakeelb@google.com>
> ---
> Changes since v1:
> - Based on Roman's comment simply the sync enforcement and only target
>   the extreme cases.

Reviewed-by: Roman Gushchin <guro@fb.com>

This version indeed looks more safe to me.

Thanks!


  parent reply	other threads:[~2022-02-15 23:27 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-11  6:49 [PATCH v2 0/4] memcg: robust enforcement of memory.high Shakeel Butt
     [not found] ` <20220211064917.2028469-1-shakeelb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2022-02-11  6:49   ` [PATCH v2 1/4] memcg: refactor mem_cgroup_oom Shakeel Butt
2022-02-11  6:49     ` Shakeel Butt
2022-02-11  6:49   ` [PATCH v2 2/4] memcg: unify force charging conditions Shakeel Butt
2022-02-11  6:49     ` Shakeel Butt
2022-02-11  6:49   ` [PATCH v2 3/4] selftests: memcg: test high limit for single entry allocation Shakeel Butt
2022-02-11  6:49     ` Shakeel Butt
     [not found]     ` <20220211064917.2028469-4-shakeelb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2022-02-15 23:28       ` Roman Gushchin
2022-02-15 23:28         ` Roman Gushchin
2022-02-11  6:49   ` [PATCH v2 4/4] memcg: synchronously enforce memory.high for large overcharges Shakeel Butt
2022-02-11  6:49     ` Shakeel Butt
2022-02-11 12:13     ` Chris Down
     [not found]       ` <YgZS+YijLo0/WmEd-6Bi1550iOqEnzZ6mRAm98g@public.gmane.org>
2022-02-11 20:36         ` Shakeel Butt
2022-02-11 20:36           ` Shakeel Butt
     [not found]           ` <CALvZod6FwcSyi3B-3fkw4e+7BGrjFF2iRLEZVeurLp2+v-k-dg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2022-02-16 13:12             ` Chris Down
2022-02-16 13:12               ` Chris Down
     [not found]     ` <20220211064917.2028469-5-shakeelb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2022-02-15 18:50       ` Shakeel Butt
2022-02-15 18:50         ` Shakeel Butt
2022-02-15 23:27     ` Roman Gushchin [this message]
2022-02-15 23:27       ` Roman Gushchin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Ygw25FypLsN9ZQaZ@carbon.dhcp.thefacebook.com \
    --to=guro@fb.com \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=chris@chrisdown.name \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=shakeelb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.