On Tue, Apr 7, 2020 at 2:43 PM Michal Hocko <mhocko@xxxxxxxxxx> 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. [1]. https://patchwork.ozlabs.org/project/netdev/list/?state=*&page=2¶m=3 > On Tue 07-04-20 11:11:13, Yafang Shao wrote: > > On Tue, Apr 7, 2020 at 11:09 AM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > > > On Tue, 7 Apr 2020 11:02:31 +0800 Yafang Shao <laoar.shao@xxxxxxxxx> wrote: > > > > > > > On Tue, Apr 7, 2020 at 7:23 AM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > > > > > > > On Tue, 7 Apr 2020 00:56:03 +0800 Yafang Shao <laoar.shao@xxxxxxxxx> wrote: > > > > > > > > > > > When I run my memcg testcase which creates lots of memcgs, I found > > > > > > there're unexpected out of memory logs while there're still enough > > > > > > available free memory. The error log is, > > > > > > mkdir: cannot create directory 'foo.65533': Cannot allocate memory > > > > > > > > > > > > The reason is when we try to create more than MEM_CGROUP_ID_MAX memcgs, an > > > > > > -ENOMEM errno will be set by mem_cgroup_css_alloc(), but the right errno > > > > > > should be -EBUSY "Device or resource busy". That is same with > > > > > > memcg_alloc_cache_id(). > > See my comment about EBUSY in the previous version > http://lkml.kernel.org/r/20200407063621.GA18914@xxxxxxxxxxxxxx > >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. 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. Per Matthew, EBUSY is better, while per you, ENOSPC is better. Matthew, would you mind to give more detailed explanation ? > > > > > > As the errno really misled me, we should make it right. After this patch, > > > > > > the error log will be, > > > > > > mkdir: cannot create directory 'foo.65533': Device or resource busy > > > > > > > > > > Thanks. > > > > > > > > > > Was a -stable backport considered? > > > > > > > > I only backported to our kernel version 4.18, but I'm not sure whether > > > > it will apply to -stable or not. > > > > Seems this issue is introduced long time ago. I will try to backport > > > > it to -stable. > > > > Should I try it based on 5.5.y and 5.6.y only ? Or all the LTS kernel > > > > versions ? > > > > > > What I'm asking (of you and of reviewers) is whether this issue is > > > sufficiently serious to require fixing in -stable kernels. The > > > patch merging details can be sorted out later. > > > > > > > > > > I think it is worth to cc stable, as this errno will mislead the user. > > We have idr failure path since 73f576c04b94 ("mm: memcontrol: fix cgroup > creation failure after many small jobs") which is more than four years > ago without anybody ever noticing. While I do agree that the existing > behavior might be confusing I am not really sure this qualifies as > stable material TBH. OK Thanks Yafang