From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DA4E1C54FCB for ; Mon, 20 Apr 2020 23:44:40 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 8CC3F2082E for ; Mon, 20 Apr 2020 23:44:40 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="de/OXlB6" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8CC3F2082E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linux-foundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 1C4B78E0007; Mon, 20 Apr 2020 19:44:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1754C8E0003; Mon, 20 Apr 2020 19:44:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 03D168E0007; Mon, 20 Apr 2020 19:44:39 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0168.hostedemail.com [216.40.44.168]) by kanga.kvack.org (Postfix) with ESMTP id DDAFE8E0003 for ; Mon, 20 Apr 2020 19:44:39 -0400 (EDT) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 95C06824556B for ; Mon, 20 Apr 2020 23:44:39 +0000 (UTC) X-FDA: 76729865478.07.low24_289e2c18b710d X-HE-Tag: low24_289e2c18b710d X-Filterd-Recvd-Size: 7153 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf21.hostedemail.com (Postfix) with ESMTP for ; Mon, 20 Apr 2020 23:44:39 +0000 (UTC) Received: from localhost.localdomain (c-73-231-172-41.hsd1.ca.comcast.net [73.231.172.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id C691C206F9; Mon, 20 Apr 2020 23:44:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1587426278; bh=Q1GMHHwul0okp0QhQ8sWc8nYUvP8rPavNqOLJ5iS9bQ=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=de/OXlB6XI/VmvgIL4E2NFxR9NFoBsIXCQKZajWshfVHqzsXqXBwFbwPfCUyEUxAs ZQy5QEV9tUsnvFuRqZSguhaFCXhVTRErF6KmAtt6kXBrj06rblIDxutfS4oY7gnl+j MuuYb9RM1qDmvcAf5Mp+jhkJd9ELnkzLQbfal+cI= Date: Mon, 20 Apr 2020 16:44:37 -0700 From: Andrew Morton To: Michal Hocko Cc: Yafang Shao , Johannes Weiner , Matthew Wilcox , Vladimir Davydov , Linux MM Subject: Re: [PATCH v3] mm, memcg: fix error return value of mem_cgroup_css_alloc() Message-Id: <20200420164437.80b7c43dc8962ea9acf0519e@linux-foundation.org> In-Reply-To: <20200409140729.GL18386@dhcp22.suse.cz> References: <20200406200916.2623f34403155264d8c8e9e7@linux-foundation.org> <20200407064329.GB18914@dhcp22.suse.cz> <20200407111017.GN18914@dhcp22.suse.cz> <20200407181012.GA12461@cmpxchg.org> <20200408182956.0906ebc4f8ec421689ec6df5@linux-foundation.org> <20200409065722.GA18386@dhcp22.suse.cz> <20200409140729.GL18386@dhcp22.suse.cz> X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, 9 Apr 2020 16:07:29 +0200 Michal Hocko wrote: > On Thu 09-04-20 21:59:42, Yafang Shao wrote: > > On Thu, Apr 9, 2020 at 2:57 PM Michal Hocko wrote: > > > > > > On Wed 08-04-20 18:29:56, Andrew Morton wrote: > > > > On Tue, 7 Apr 2020 14:10:12 -0400 Johannes Weiner wrote: > > > > > > > > > Given how unlikely this is to affect real setups, I don't think this > > > > > patch is stable material. > > > > > > > > OK, thanks, done. > > > > > > > > I didn't notice any acks - is this patch otherwise good to go upstream? > > > > > > I will ack it if s@EBUSY@ENOSPC@ > > > > > > > BTW, there's no EBUSY in man page of write(2) neither. > > But when we echo some procs into cgroup.subtree_control of a non-root > > cgroup, it will return EBUSY. See also cgroup_subtree_control_write(). > > Pls. correct me if I missed something. > > > > We have to admit that the man page is out of date. > > Or the code is returning something unexpected but nobody has noticed so > far. Please talk to cgroup maintainers on how to address that. I agree with your ENOSPC reasoning in http://lkml.kernel.org/r/20200407063621.GA18914@dhcp22.suse.cz That means this change, methinks: --- a/mm/memcontrol.c~mm-memcg-fix-error-return-value-of-mem_cgroup_css_alloc-fix +++ a/mm/memcontrol.c @@ -2721,8 +2721,6 @@ static int memcg_alloc_cache_id(void) id = ida_simple_get(&memcg_cache_ida, 0, MEMCG_CACHES_MAX_SIZE, GFP_KERNEL); - if (id == -ENOSPC) - return -EBUSY; if (id < 0) return id; @@ -5004,10 +5002,6 @@ static struct mem_cgroup *mem_cgroup_all memcg->id.id = idr_alloc(&mem_cgroup_idr, NULL, 1, MEM_CGROUP_ID_MAX, GFP_KERNEL); - if (memcg->id.id == -ENOSPC) { - error = -EBUSY; - goto fail; - } if (memcg->id.id < 0) { error = memcg->id.id; goto fail; _ So the final patch will be as below. Please review my changelog alterations. From: Yafang Shao Subject: mm, memcg: fix error return value of mem_cgroup_css_alloc() 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 -ENOSPC "No space left on device", which is an appropriate errno for userspace's failed mkdir. 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': No space left on device" [akpm@linux-foundation.org: s/EBUSY/ENOSPC/, per Michal] Link: http://lkml.kernel.org/r/20200407063621.GA18914@dhcp22.suse.cz Link: http://lkml.kernel.org/r/1586192163-20099-1-git-send-email-laoar.shao@gmail.com Signed-off-by: Yafang Shao Suggested-by: Matthew Wilcox Acked-by: Michal Hocko Cc: Johannes Weiner Cc: Vladimir Davydov Signed-off-by: Andrew Morton --- mm/memcontrol.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) --- a/mm/memcontrol.c~mm-memcg-fix-error-return-value-of-mem_cgroup_css_alloc +++ a/mm/memcontrol.c @@ -4990,19 +4990,22 @@ static struct mem_cgroup *mem_cgroup_all unsigned int size; int node; int __maybe_unused i; + long error = -ENOMEM; size = sizeof(struct mem_cgroup); size += nr_node_ids * sizeof(struct mem_cgroup_per_node *); memcg = kzalloc(size, GFP_KERNEL); if (!memcg) - return NULL; + return ERR_PTR(error); memcg->id.id = idr_alloc(&mem_cgroup_idr, NULL, 1, MEM_CGROUP_ID_MAX, GFP_KERNEL); - if (memcg->id.id < 0) + if (memcg->id.id < 0) { + error = memcg->id.id; goto fail; + } memcg->vmstats_local = alloc_percpu(struct memcg_vmstats_percpu); if (!memcg->vmstats_local) @@ -5046,7 +5049,7 @@ static struct mem_cgroup *mem_cgroup_all fail: mem_cgroup_id_remove(memcg); __mem_cgroup_free(memcg); - return NULL; + return ERR_PTR(error); } static struct cgroup_subsys_state * __ref @@ -5057,8 +5060,8 @@ mem_cgroup_css_alloc(struct cgroup_subsy long error = -ENOMEM; memcg = mem_cgroup_alloc(); - if (!memcg) - return ERR_PTR(error); + if (IS_ERR(memcg)) + return ERR_CAST(memcg); WRITE_ONCE(memcg->high, PAGE_COUNTER_MAX); memcg->soft_limit = PAGE_COUNTER_MAX; @@ -5108,7 +5111,7 @@ mem_cgroup_css_alloc(struct cgroup_subsy fail: mem_cgroup_id_remove(memcg); mem_cgroup_free(memcg); - return ERR_PTR(-ENOMEM); + return ERR_PTR(error); } static int mem_cgroup_css_online(struct cgroup_subsys_state *css) _