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 9E1C9C6FD18 for ; Wed, 19 Apr 2023 07:38:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 07D598E0005; Wed, 19 Apr 2023 03:38:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 036DE8E0001; Wed, 19 Apr 2023 03:38:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E34EC8E0005; Wed, 19 Apr 2023 03:38:45 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id D68088E0001 for ; Wed, 19 Apr 2023 03:38:45 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 9D3F3801F7 for ; Wed, 19 Apr 2023 07:38:45 +0000 (UTC) X-FDA: 80697338610.13.A73B7EF Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by imf15.hostedemail.com (Postfix) with ESMTP id AFC26A0011 for ; Wed, 19 Apr 2023 07:38:43 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b="MG/521ap"; spf=pass (imf15.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.28 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681889924; a=rsa-sha256; cv=none; b=ksxx2tEoqVhTp1PpbkCfqT0zIKXQSdLbqIy7GIXFC7C3eeYNNCO7fkuuWk3RxXoyRBhUoc epCfZ4lXIG1XuteypiEF4yVpaiCwnNGpm8HYTho1cHU9FaorPkFLkuFt8FZCuceYPF6DU4 EKbU+3MN48b5t/xwopnA4YS2oRQJsMU= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b="MG/521ap"; spf=pass (imf15.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.28 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1681889924; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=G0ScSRd7XdrWa3mKgZYiQMwWQPIvOIHXV2c6WBE5gyQ=; b=JtrAG+tstPccGBHCOJZt8wvW+yDvm59eEJMtkxHSQAtIECKAgnth6mKr3QYAIP7S9EeBr/ QK8/mayH8y1PHAldFwiMAgXaQhS1q33QL2bwTl9stRAjsqWLZ3r1Www/XOAaLx8jgZBUWZ IP7zGAB7uH/PpHD50RVgkJ9atDSHNNw= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 7FF14218A9; Wed, 19 Apr 2023 07:38:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1681889922; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=G0ScSRd7XdrWa3mKgZYiQMwWQPIvOIHXV2c6WBE5gyQ=; b=MG/521apeHblfAHRLJbK+QJuWM3lOblltBAW47vHMjDzJTMCPCT4JDCN5JYbQBieaO+UiV 3E+Z/kJbrHQkzRuL1Is1hZBUoka63P/WOHxIAyAPsgAd98RoSuX7GqwVD2YCflM3dMopa7 XBYSgM4SrXdR9jTXtHntFIjwLAHYlZw= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 62B9213580; Wed, 19 Apr 2023 07:38:42 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id CYzUFIKaP2QOSgAAMHmgww (envelope-from ); Wed, 19 Apr 2023 07:38:42 +0000 Date: Wed, 19 Apr 2023 09:38:41 +0200 From: Michal Hocko To: Haifeng Xu Cc: hannes@cmpxchg.org, roman.gushchin@linux.dev, shakeelb@google.com, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 2/2] memcg, oom: remove explicit wakeup in mem_cgroup_oom_synchronize() Message-ID: References: <20230419030739.115845-2-haifeng.xu@shopee.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230419030739.115845-2-haifeng.xu@shopee.com> X-Rspam-User: X-Rspamd-Queue-Id: AFC26A0011 X-Rspamd-Server: rspam01 X-Stat-Signature: fuqzmctxry7qsp53wk1katy8uji3bfdm X-HE-Tag: 1681889923-375516 X-HE-Meta: U2FsdGVkX18Wevhwh8c5jwfXmwGEv+96ICIhXh/2L+pSmnHfwHThqfsKQe2do2u4Phq0EV969Mb2/g01QXYa3fx22epMo6phhzlWLDq3imyXpB4xUV9YyQw2BkKHdWmzpXUVHb9Hfkh/QNZai45JuLxT3OIhvYXR8L86/5susnIBHy0kgK2w+/h8ywez2tCSl5DmmB43EiBARlkPDR4F77w8/7xfBUoRDFIffGZpS7hk1O8Nt3PxZQuiKMA2oshwFifGspR/ZiGmjIUKGPfHGdzOUr4GLAV+1Lg77u4dqNnBSMVegueMWz3WGBzLMFbPwK59SrXRhS1hoPdLyaFUILUvo/ZZmT79fl3c32Kba7EBQLcdHsb46ooyWwXLaP0iokYnAUVbEG5QNEEXRuDoWUw8ocXIgJpbXJfjRx0XOQI7Xoqsf8PK9Lsa7/1lbyTLKpQX+q8/2laxukA7r3dSiIvRVRrJ2lMVMwrMVcqw4xd7zVmqFhSlFBsU6ebWmo6dkK5f++Uq1ENyYOnNTJkgpjxn67dU+sXhPK0HZzGf8s8vKHVS/hhyJZHJhjbFbs/D9F5jlNwyk5yOxA8iTFH0VOVp2wO5U5OolqpQF/mj2b+PiUGJc8XcX9SiiRM27zxrpN3JU+raPgMzH0Xa1DBEkVyr4uhRXLZGURDVt+ijxNnAR8e68Y7r/JFubBcIWuXoAAptexoVLBPy4oinWvCXctEDCqowHVaKFMZc8e4QyXV50V68m75uL+PLQLk+HMDzx1bh5MFL32lct1AoNbsPtqh9tHY7IVWCIzPVPFmx9cp1drS9l+NF3p+hl8Cs6v3OkjryAPZ3OAlGEd9xmmcZJiQKOexIruX3wQ5Xj8oqO9zI/47+0MC+e4Lc3dhrWqOdAC5zNXYQ1Uk0PK8vRbhGRNo5B1wTSK1AvyVUojeInHb0hnL8+ydbTlEy+rTRHv+FFvGZuxqgZXb6Y9TJDWi qLq66rMW eg2hFnEo0GKFDmihS71fYYl9Aj4v2xQEBSEoaUGYtJKU4mbQ3W9FIknz1DQ8FVPrHHMhDR7ZHz6mKuSS1FB+VcSmV07FzncocK4IysyHS2W02Xe6K7PwM/RGY6fiGxxj3LUmpH15urhAemlaaBZA/DFUlYv5urukUdFtD4+L04tkgYxJ56HAXkpNsOommlG897vK1Z1phHwDGJufu47pgjg2zi3wstAKyiv0g7oqb4MRsQ1DwzoXHEmSOnRvQt0WuGeNNqoHcp7ViIFOFLIfFEti5eAEeWPNYgRyLvNkDWxS3jguYyPisYnyg7ryfjGztsmHijcmt0ci8QiQ= 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 Wed 19-04-23 03:07:39, Haifeng Xu wrote: > Before commit 29ef680ae7c2 ("memcg, oom: move out_of_memory back to > the charge path"), all memcg oom killers were delayed to page fault > path. And the explicit wakeup is used in this case: > > thread A: > ... > if (locked) { // complete oom-kill, hold the lock > mem_cgroup_oom_unlock(memcg); > ... > } > ... > > thread B: > ... > > if (locked && !memcg->oom_kill_disable) { > ... > } else { > schedule(); // can't acquire the lock > ... > } > ... > > The reason is that thread A kicks off the OOM-killer, which leads to > wakeups from the uncharges of the exiting task. But thread B is not > guaranteed to see them if it enters the OOM path after the OOM kills > but before thread A releases the lock. > > Now only oom_kill_disable case is handled from the #PF path. In that > case it is userspace to trigger the wake up not the #PF path itself. > All potential paths to free some charges are responsible to call > memcg_oom_recover() , so the explicit wakeup is not needed in the > mem_cgroup_oom_synchronize() path which doesn't release any memory > itself. > > Signed-off-by: Haifeng Xu > Suggested-by: Michal Hocko I hope I haven't missed anything but this looks good to me. One way to test this would be a parallel OOM triggering workload which uses page faults and an automatic user space driven oom killer (detect under_oom and send SIGKILL to a random task after a random timeout). Objective is that no task gets stuck in mem_cgroup_oom_synchronize. I am pretty sure this could be easily turned into a selftest. Acked-by: Michal Hocko > --- > v2: split original into two and improve patch description > --- > mm/memcontrol.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index fbf4d2bb1003..710ce3e7824f 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2003,15 +2003,8 @@ bool mem_cgroup_oom_synchronize(bool handle) > mem_cgroup_unmark_under_oom(memcg); > finish_wait(&memcg_oom_waitq, &owait.wait); > > - if (locked) { > + if (locked) > mem_cgroup_oom_unlock(memcg); > - /* > - * There is no guarantee that an OOM-lock contender > - * sees the wakeups triggered by the OOM kill > - * uncharges. Wake any sleepers explicitly. > - */ > - memcg_oom_recover(memcg); > - } > cleanup: > current->memcg_in_oom = NULL; > css_put(&memcg->css); > -- > 2.25.1 -- Michal Hocko SUSE Labs