All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shakeel Butt <shakeelb@google.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	Roman Gushchin <guro@fb.com>, Greg Thelen <gthelen@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux MM <linux-mm@kvack.org>, Cgroups <cgroups@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] memcg: oom: ignore oom warnings from memory.max
Date: Tue, 5 May 2020 08:03:17 -0700	[thread overview]
Message-ID: <CALvZod5H-fDbvu73=hkzN0Man_+03ZW0d5zc1N0YObVHSiy0Tw@mail.gmail.com> (raw)
In-Reply-To: <20200505071324.GB16322@dhcp22.suse.cz>

On Tue, May 5, 2020 at 12:13 AM Michal Hocko <mhocko@kernel.org> wrote:
>
> On Mon 04-05-20 12:23:51, Shakeel Butt wrote:
> [...]
> > *Potentially* useful for debugging versus actually beneficial for
> > "sweep before tear down" use-case.
>
> I definitely do not want to prevent you from achieving what you
> want/need. Let's get back to your argument on why you cannot use
> memory.high for this purpose and what is the actual difference from
> memory.max on the "sweep before removal". You've said
>
> : Yes that would work but remote charging concerns me. Remote charging
> : can still happen after the memcg is offlined and at the moment, high
> : reclaim does not work for remote memcg and the usage can go till max
> : or global pressure. This is most probably a misconfiguration and we
> : might not receive the warnings in the log ever. Setting memory.max to
> : 0 will definitely give such warnings.
>
> So essentially the only reason you are not using memory.high which would
> effectively achieve the same level of reclaim for your usecase is that
> potential future remote charges could get unnoticed.

Yes.

> I have proposed to
> warn when charging to an offline memcg because that looks like a sign of
> bug to me.

Instead of a bug, I would say misconfiguration but there is at least a
genuine user i.e. buffer_head. It can be allocated in reclaim and
trigger remote charging but it should be fine as the page it is
attached to will possibly get freed soon. So, I don't think we want to
warn for all remote charges to an offlined memcg.

> Having the hard limit clamped to 0 (or some other small
> value) would complain loud by the oom report and no eligible tasks
> message but it will unlikely help to stop such a usage because, well,
> there is nothing reclaimable and we force the charge in that case. So
> you are effectively in the memory.high like situation.

Yes, effectively it will be similar to memory.high but at least we
will get early warnings.

Now rethinking about the remote charging of buffer_head to an offlined
memcg with memory.max=0. It seems like it is special in the sense that
it is using __GFP_NOFAIL and will skip the oom-killer and thus
warnings. Maybe the right approach is, as you suggested, always warn
for charging an offline memcg unless
(__GFP_NOFAIL|__GFP_RETRY_MAYFAIL). Though I am not sure if this is
doable without code duplication.

>
> So instead of potentially removing a useful information can we focus on
> the remote charging side of the problem and deal with it in a sensible
> way? That would make memory.high usable for your usecase and I still
> believe that this is what you should be using in the first place.

We talked about this at LSFMM'19 and I think the decision was to not
fix high reclaim for remote memcg until it will be an actual issue. I
suppose now we can treat it as an actual issue.

There are a couple of open questions:
1) Should the remote chargers be throttled and do the high reclaim?
2) There can be multiple remote charges to multiple memcgs in a single
kernel entry. Should we handle such scenarios?

Shakeel

  reply	other threads:[~2020-05-05 15:03 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-30 18:27 [PATCH] memcg: oom: ignore oom warnings from memory.max Shakeel Butt
2020-04-30 18:27 ` Shakeel Butt
2020-04-30 19:06 ` Roman Gushchin
2020-04-30 19:06   ` Roman Gushchin
2020-04-30 19:30   ` Johannes Weiner
2020-04-30 19:30     ` Johannes Weiner
2020-04-30 20:23     ` Roman Gushchin
2020-04-30 20:23       ` Roman Gushchin
2020-04-30 19:31   ` Shakeel Butt
2020-04-30 19:31     ` Shakeel Butt
2020-04-30 19:31     ` Shakeel Butt
2020-04-30 19:29 ` Johannes Weiner
2020-04-30 20:20   ` Shakeel Butt
2020-04-30 20:20     ` Shakeel Butt
2020-04-30 20:20     ` Shakeel Butt
2020-05-04  6:57     ` Michal Hocko
2020-05-04  6:57       ` Michal Hocko
2020-05-04 13:54       ` Shakeel Butt
2020-05-04 13:54         ` Shakeel Butt
2020-05-04 13:54         ` Shakeel Butt
2020-05-01  1:39 ` Yafang Shao
2020-05-01  1:39   ` Yafang Shao
2020-05-01  2:04   ` Shakeel Butt
2020-05-01  2:04     ` Shakeel Butt
2020-05-01  2:12     ` Yafang Shao
2020-05-01  2:12       ` Yafang Shao
2020-05-01  2:12       ` Yafang Shao
2020-05-04  7:03   ` Michal Hocko
2020-05-04  7:03     ` Michal Hocko
2020-05-04  7:26     ` Yafang Shao
2020-05-04  7:26       ` Yafang Shao
2020-05-04  7:26       ` Yafang Shao
2020-05-04  7:35       ` Michal Hocko
2020-05-04  7:40         ` Yafang Shao
2020-05-04  7:40           ` Yafang Shao
2020-05-04  7:40           ` Yafang Shao
2020-05-04  8:03           ` Michal Hocko
2020-05-04  8:03             ` Michal Hocko
2020-05-04  6:56 ` Michal Hocko
2020-05-04  6:56   ` Michal Hocko
2020-05-04 13:54   ` Shakeel Butt
2020-05-04 13:54     ` Shakeel Butt
2020-05-04 14:11     ` Michal Hocko
2020-05-04 14:53       ` Shakeel Butt
2020-05-04 14:53         ` Shakeel Butt
2020-05-04 14:53         ` Shakeel Butt
2020-05-04 15:00         ` Michal Hocko
2020-05-04 15:35           ` Shakeel Butt
2020-05-04 15:35             ` Shakeel Butt
2020-05-04 15:35             ` Shakeel Butt
2020-05-04 15:39             ` Yafang Shao
2020-05-04 15:39               ` Yafang Shao
2020-05-04 15:39               ` Yafang Shao
2020-05-04 16:06             ` Michal Hocko
2020-05-04 16:06               ` Michal Hocko
2020-05-04 19:23               ` Shakeel Butt
2020-05-04 19:23                 ` Shakeel Butt
2020-05-05  7:13                 ` Michal Hocko
2020-05-05  7:13                   ` Michal Hocko
2020-05-05 15:03                   ` Shakeel Butt [this message]
2020-05-05 15:03                     ` Shakeel Butt
2020-05-05 16:57                     ` Johannes Weiner
2020-05-05 16:57                       ` Johannes Weiner
2020-05-05 15:27                 ` Johannes Weiner
2020-05-05 15:27                   ` Johannes Weiner
2020-05-05 15:35                   ` Shakeel Butt
2020-05-05 15:35                     ` Shakeel Butt
2020-05-05 15:35                     ` Shakeel Butt
2020-05-05 15:49                     ` Michal Hocko
2020-05-05 15:49                       ` Michal Hocko
2020-05-05 16:40                     ` Johannes Weiner
2020-05-05 16:40                       ` Johannes Weiner
2020-05-04 14:20     ` Tetsuo Handa
2020-05-04 14:20       ` Tetsuo Handa
2020-05-04 14:57       ` Shakeel Butt
2020-05-04 14:57         ` Shakeel Butt
2020-05-04 15:44         ` Tetsuo Handa
2020-05-04 15:44           ` Tetsuo Handa

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='CALvZod5H-fDbvu73=hkzN0Man_+03ZW0d5zc1N0YObVHSiy0Tw@mail.gmail.com' \
    --to=shakeelb@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=gthelen@google.com \
    --cc=guro@fb.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    /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.