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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 83C5CE7717F for ; Thu, 12 Dec 2024 21:35:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E22746B00A0; Thu, 12 Dec 2024 16:35:36 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DD1E06B00A3; Thu, 12 Dec 2024 16:35:36 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C73736B00A4; Thu, 12 Dec 2024 16:35:36 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id A824F6B00A0 for ; Thu, 12 Dec 2024 16:35:36 -0500 (EST) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 66A4FA0982 for ; Thu, 12 Dec 2024 21:35:36 +0000 (UTC) X-FDA: 82887613326.05.0B542AE Received: from out-175.mta1.migadu.com (out-175.mta1.migadu.com [95.215.58.175]) by imf23.hostedemail.com (Postfix) with ESMTP id D56A414000C for ; Thu, 12 Dec 2024 21:35:17 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=Hhhrsyp1; spf=pass (imf23.hostedemail.com: domain of shakeel.butt@linux.dev designates 95.215.58.175 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1734039317; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=C9LCAsrG2v3sr11vxtTgPaYAQ3cbRB7r/yjBqH3E80c=; b=BNB7k3bfWGFbMkgBwgGkaR7YBIOZouQoh6msPr+nS3JOZl9y9gdrVcGjlRlRL4cY7GcIyR NNfr6WH70vVSP3UO4iNjrYV/KS+jHemnSHj0kn65QHIhBauWH0Yb1QTmqiwhnlN8QM8C3i tnbD+1peSBO0q/nY+0a67cKgK4H8K+A= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=Hhhrsyp1; spf=pass (imf23.hostedemail.com: domain of shakeel.butt@linux.dev designates 95.215.58.175 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1734039317; a=rsa-sha256; cv=none; b=kadcjlp2GP/VGlAFG9mDa3Xa5DBUZtOIEJuloDHpvW03RwDd0EsBi4XB6rVoU6lM++YrJQ jd2TBr9iR7qiSpJfWW88oMfwKPaVWaQj0rFl6YHZFKVwf6lWMuURtzPqpqh6uOA8RsfpWh lLaSjKG48ABO4G7/KEk5ON+K1xFSSfQ= Date: Thu, 12 Dec 2024 13:35:26 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1734039332; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=C9LCAsrG2v3sr11vxtTgPaYAQ3cbRB7r/yjBqH3E80c=; b=Hhhrsyp1EVV0VY+VzT9YVcEva2u9oBERN0zCU87WwGAMHnBk0CPrBSw31IHWdSLofPO6h+ EDzXg4V/9M20/KmJxWSVw9pJxHxU3s+m0esGx3CBJbNZn+PRM3sNShj4z4BAvrOwV0jBJ0 Lh4LTjgodia5R0d+aF8f/Z2uYb5jW9w= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Shakeel Butt To: Johannes Weiner Cc: Yosry Ahmed , Rik van Riel , Balbir Singh , Michal Hocko , Roman Gushchin , Muchun Song , Andrew Morton , cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@meta.com, Nhat Pham Subject: Re: [PATCH v2] memcg: allow exiting tasks to write back data to swap Message-ID: References: <20241212115754.38f798b3@fangorn> <20241212183012.GB1026@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20241212183012.GB1026@cmpxchg.org> X-Migadu-Flow: FLOW_OUT X-Rspamd-Server: rspam05 X-Stat-Signature: 4sdh7d74fe3brwghucdc1o6pryub1i5p X-Rspamd-Queue-Id: D56A414000C X-Rspam-User: X-HE-Tag: 1734039317-708736 X-HE-Meta: U2FsdGVkX19AKcs9Fp5vesMn5sIRnWKhvGipeL1C/MUP5+6ehB8SOZow1UVn6w6cO6kic8waTNgIBVc4BSbzheZ8VzIJUvle40kJK0Y9D/WyQURE5sslySMTxYwj+cH59s3mMwmNTVaNnG/3mYG0ARhTLpdxJdzPM3Erpr7lL4NZoRiJKTVWoE7HbwK/5Mvs7gIzkYbBXRYdrgymOb29KYTVP0+Ma6lt8HH4gCxixCZxw4aFOPsF6ydNxmzFdg3SCDPyh18KaUKSTh1Qhy2sG3Q3kGXzaHA3PcIkvN1j8MSRhsFjI5c8KvMR8V8H8T6QqL4KaiQzBrJnSbAVv4JAl1TcIlxNVd2hFsGEedEPGqztnL5LsxEgdioZaJ3YMDFnQUQ7z79P4HNMyhHX86dirYsXvycn10HXv2cz0arw2PdFHiNYg24MIpUJReo07fcqK7s9FDAHpbrw0IiaBS/op5h5WBmhk2/DyZpmIefwFghbaDwq3HwXlgOqgXfxzmOPUmJXy4RQ6AcC58981oA9uZ8FBZy3dpHl4FO1LkwSRXE5Z/DmHRCnRcVAMYc8TpQDOEqDl2GP9kPMtUQVcD5Q8zUpL1Koc872BjNe2HVsJsfI2/b1tmC3R8z5hYVpZspigA8MZTFXACO+WGgY7Ov8XdoE1eGTQV+weGJKSsWHmdkF7X3UotP7MXU+SGchgUIthNgSksfRg71TRDnKNFvYEs5VcITsOLml1oc8X2w92gWUUdz6WeMSIN06Bgd4hDjtxU3AU91PEUj+d/yaPOlmGgUPNR8Ur3jCATlxpsy+o3b1gOlhx0z7INC2VfF69Gc7A/S3A5tNrXtvsqW1vjcXQdn9YsUzpfXeXfHR2LADRTaY3zatOPUjrfxr7z6aa6sH/utX6BSIzcVKUV5I4XGWCoe9PRZZ6gcCX+brTLYVCOVD4kkyGlUr2gjZqrX4mZ0cPrHGJg10gCRZTJT8U5Q 4gJRctNe BCURGWgQ4lGVF4Mq/DsGd0zYUNWSr0+l962YaCwoebnfui9iY4eFRWQQHCdNDIwhRDov9GmiQfy6YCUvLYaePNC5MGBTca8FLHGjUswsqMIJrzfDEY1r0tJS29O/6avpnrqpCIR25nOiLjlxeacr48R6lbp2q09jIOGJah1YMEMbMi7Zotb0Yn+5R2oYiQClLKTr1nDEheqYbwjkkb8nEBIkOKDd0oEy1I2jwMLoRUEAaOPQ= 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: List-Subscribe: List-Unsubscribe: On Thu, Dec 12, 2024 at 01:30:12PM -0500, Johannes Weiner wrote: > On Thu, Dec 12, 2024 at 09:06:25AM -0800, Yosry Ahmed wrote: > > On Thu, Dec 12, 2024 at 8:58 AM Rik van Riel wrote: > > > > > > A task already in exit can get stuck trying to allocate pages, if its > > > cgroup is at the memory.max limit, the cgroup is using zswap, but > > > zswap writeback is enabled, and the remaining memory in the cgroup is > > > not compressible. > > > > > > This seems like an unlikely confluence of events, but it can happen > > > quite easily if a cgroup is OOM killed due to exceeding its memory.max > > > limit, and all the tasks in the cgroup are trying to exit simultaneously. > > > > > > When this happens, it can sometimes take hours for tasks to exit, > > > as they are all trying to squeeze things into zswap to bring the group's > > > memory consumption below memory.max. > > > > > > Allowing these exiting programs to push some memory from their own > > > cgroup into swap allows them to quickly bring the cgroup's memory > > > consumption below memory.max, and exit in seconds rather than hours. > > > > > > Signed-off-by: Rik van Riel > > > > Thanks for sending a v2. > > > > I still think maybe this needs to be fixed on the memcg side, at least > > by not making exiting tasks try really hard to reclaim memory to the > > point where this becomes a problem. IIUC there could be other reasons > > why reclaim may take too long, but maybe not as pathological as this > > case to be fair. I will let the memcg maintainers chime in for this. > > > > If there's a fundamental reason why this cannot be fixed on the memcg > > side, I don't object to this change. > > > > Nhat, any objections on your end? I think your fleet workloads were > > the first users of this interface. Does this break their expectations? > > Yes, I don't think we can do this, unfortunately :( There can be a > variety of reasons for why a user might want to prohibit disk swap for > a certain cgroup, and we can't assume it's okay to make exceptions. > > There might also not *be* any disk swap to overflow into after Nhat's > virtual swap patches. Presumably zram would still have the issue too. Very good points. > > So I'm also inclined to think this needs a reclaim/memcg-side fix. We > have a somewhat tumultous history of policy in that space: > > commit 7775face207922ea62a4e96b9cd45abfdc7b9840 > Author: Tetsuo Handa > Date: Tue Mar 5 15:46:47 2019 -0800 > > memcg: killed threads should not invoke memcg OOM killer > > allowed dying tasks to simply force all charges and move on. This > turned out to be too aggressive; there were instances of exiting, > uncontained memcg tasks causing global OOMs. This lead to that: > > commit a4ebf1b6ca1e011289677239a2a361fde4a88076 > Author: Vasily Averin > Date: Fri Nov 5 13:38:09 2021 -0700 > > memcg: prohibit unconditional exceeding the limit of dying tasks > > which reverted the bypass rather thoroughly. Now NO dying tasks, *not > even OOM victims*, can force charges. I am not sure this is correct, > either: > > If we return -ENOMEM to an OOM victim in a fault, the fault handler > will re-trigger OOM, which will find the existing OOM victim and do > nothing, then restart the fault. This is a memory deadlock. The page > allocator gives OOM victims access to reserves for that reason. > > Actually, it looks even worse. For some reason we're not triggering > OOM from dying tasks: > > ret = task_is_dying() || out_of_memory(&oc); > > Even though dying tasks are in no way privileged or allowed to exit > expediently. Why shouldn't they trigger the OOM killer like anybody > else trying to allocate memory? This is a very good point and actually out_of_memory() will mark the dying process as oom victim and put it in the oom reaper's list which should help further in such situation. > > As it stands, it seems we have dying tasks getting trapped in an > endless fault->reclaim cycle; with no access to the OOM killer and no > access to reserves. Presumably this is what's going on here? > > I think we want something like this: The following patch looks good to me. Let's test this out (hopefully Rik will be able to find a live impacted machine) and move forward with this fix. > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 53db98d2c4a1..be6b6e72bde5 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1596,11 +1596,7 @@ static bool mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask, > if (mem_cgroup_margin(memcg) >= (1 << order)) > goto unlock; > > - /* > - * A few threads which were not waiting at mutex_lock_killable() can > - * fail to bail out. Therefore, check again after holding oom_lock. > - */ > - ret = task_is_dying() || out_of_memory(&oc); > + ret = out_of_memory(&oc); > > unlock: > mutex_unlock(&oom_lock); > @@ -2198,6 +2194,9 @@ int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask, > if (unlikely(current->flags & PF_MEMALLOC)) > goto force; > > + if (unlikely(tsk_is_oom_victim(current))) > + goto force; > + > if (unlikely(task_in_memcg_oom(current))) > goto nomem; >