All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Michal Hocko <mhocko@kernel.org>
Cc: Yafang Shao <laoar.shao@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Matthew Wilcox <willy@infradead.org>,
	Vladimir Davydov <vdavydov.dev@gmail.com>,
	Linux MM <linux-mm@kvack.org>
Subject: Re: [PATCH v3] mm, memcg: fix error return value of mem_cgroup_css_alloc()
Date: Tue, 7 Apr 2020 14:10:12 -0400	[thread overview]
Message-ID: <20200407181012.GA12461@cmpxchg.org> (raw)
In-Reply-To: <20200407111017.GN18914@dhcp22.suse.cz>

On Tue, Apr 07, 2020 at 01:10:17PM +0200, Michal Hocko wrote:
> On Tue 07-04-20 17:31:33, Yafang Shao wrote:
> > I checked the explaination about ENOSPC in 73f576c04b94 ("mm:
> > memcontrol: fix cgroup creation failure after many small jobs")
> > carefully, but I don't a clear idea which one is better now.
> 
> The changelog simply mentioned that without the additional id tracking
> the ENOSPC would have been returned from elsewhere. I haven't checked
> but I suspect it would be from the cgroup core. This patch just didn't
> propagate the idr failure specifically and hid it under the ENOMEM
> failure path. I can only speculate why that was the case but I suspect
> that Johannes simply didn't consider the distinction important enough.

The old -ENOSPC came directly from the memcg css online callback:

-       if (css->id > MEM_CGROUP_ID_MAX)
-               return -ENOSPC;

And it only became a problem because, on big memory machines (128+G)
with a high rate of short-lived jobs, lazily freed cgroups piled up
over the course of multiple days and clogged up the ID space. Nobody
actually tried to create 64k user-visible cgroups concurrently. It's
hard to imagine any machine running that many meaningfully distinct
memory consumers in parallel. So I didn't think (and still don't tbh)
it matters all that much in practice what we return here.

I'm not against changing it back to -ENOSPC. And I agree with Michal
it might be better than -EBUSY because of the mkdir() interface.

Given how unlikely this is to affect real setups, I don't think this
patch is stable material.


  reply	other threads:[~2020-04-07 18:10 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-06 16:56 [PATCH v3] mm, memcg: fix error return value of mem_cgroup_css_alloc() Yafang Shao
2020-04-06 23:23 ` Andrew Morton
2020-04-07  3:02   ` Yafang Shao
2020-04-07  3:09     ` Andrew Morton
2020-04-07  3:11       ` Yafang Shao
2020-04-07  6:43         ` Michal Hocko
2020-04-07  9:31           ` Yafang Shao
2020-04-07 11:10             ` Michal Hocko
2020-04-07 18:10               ` Johannes Weiner [this message]
2020-04-09  1:29                 ` Andrew Morton
2020-04-09  6:57                   ` Michal Hocko
2020-04-09 13:59                     ` Yafang Shao
2020-04-09 14:07                       ` Michal Hocko
2020-04-20 23:44                         ` Andrew Morton
2020-04-21 14:44                           ` Johannes Weiner

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=20200407181012.GA12461@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=akpm@linux-foundation.org \
    --cc=laoar.shao@gmail.com \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=vdavydov.dev@gmail.com \
    --cc=willy@infradead.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.