archive mirror
 help / color / mirror / Atom feed
From: Michal Hocko <>
To: Yafang Shao <>
Cc: Andrew Morton <>,
	Matthew Wilcox <>,
	Johannes Weiner <>,
	Vladimir Davydov <>,
	Linux MM <>
Subject: Re: [PATCH v3] mm, memcg: fix error return value of mem_cgroup_css_alloc()
Date: Tue, 7 Apr 2020 13:10:17 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Tue 07-04-20 17:31:33, Yafang Shao wrote:
> On Tue, Apr 7, 2020 at 2:43 PM Michal Hocko <> wrote:
> >
> > [I have only now noticed there was another version posted. Please try to
> > wait a bit longer for other feedback before reposting a newer version]
> >
> Sure I will. sorry about that.
> But after we send a patch, we always don't know in which state this
> patch is, e.g. Changes Requested, Not Applicable, Rejected, Needs
> Review / ACK and etc, as listed in netdev list[1]. That could give us
> an explicit instruction on what to do next.

Yes I can see how that can be unclear or consuming at times. But a
general rule of thumb I would suggest is to simply wait few days before
reposting a new version. If you need to discuss an incremental change
based on the review feedback you can always do that in the original
email thread and repost the new version after the review feedback

> [1].*&page=2&param=3
> > On Tue 07-04-20 11:11:13, Yafang Shao wrote:
> > See my comment about EBUSY in the previous version
> >
> >
> >From the perspective of mkdir(2), it is better to use ENOSPC here. But
> I'm not sure whether it is better to update the man page of mkdir(2)
> or not.

Well, I do not know whether EBUSY missing in the man page is an omission
or an intention. But adding it only for this single operation failure
would sound like an overkill to me.

> 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.
All I am saying is that preserving the failure mode would be preferable.
Especially unless there a strong reason to use EBUSY which then should
be carefully explained in the changelog.

Michal Hocko

  reply	other threads:[~2020-04-07 11: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 [this message]
2020-04-07 18:10               ` Johannes Weiner
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:

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

  git send-email \ \ \ \ \ \ \ \ \

* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).