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 5E7EAC6FD1D for ; Fri, 17 Mar 2023 11:47:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4E23F6B0074; Fri, 17 Mar 2023 07:47:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 492446B0075; Fri, 17 Mar 2023 07:47:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3595C6B0078; Fri, 17 Mar 2023 07:47:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 233766B0074 for ; Fri, 17 Mar 2023 07:47:40 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id D951E1C725C for ; Fri, 17 Mar 2023 11:47:39 +0000 (UTC) X-FDA: 80578215438.07.D3AF3D9 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by imf08.hostedemail.com (Postfix) with ESMTP id 0125F16000A for ; Fri, 17 Mar 2023 11:47:37 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=QEr6ZWlE; spf=pass (imf08.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.29 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=1679053658; 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=q6PNEKXZBANXXYlgBFt4BSZwmt2KtLDfnR7EpFF/l24=; b=7cb3o5tGdRX8H56tA1QhCGOtUuhUHwl62g3zuXPkOpAYWIlHP69SiQTosf0SIKGTpjF4Ix 6msXYUYSuab63k73jkH3yuF61IkRTLP/TApVH43hIwL0fgBzVlkn1K/epE0tgGE+ebnO1j tieWg4Eq94MZsVcQKZwZNWAkthUkeT8= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=QEr6ZWlE; spf=pass (imf08.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.29 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=1679053658; a=rsa-sha256; cv=none; b=kA46Z3BpS4QXYR3bdnEJ2SQslQ8cr1rd827gLyFA+ALfA8EiN6/jcpmHqdnMh0FNifxiFj q42y9MlUFiCSAYvkmpsr4PS7mo/HbDyfdhn24bQ1j834Rcp6RU3GpeiaE0nx0sl4MApwVJ xJqeFT0+QN8sBNaWf86b2XUrw17UVcs= 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-out2.suse.de (Postfix) with ESMTPS id 6FE171FDDE; Fri, 17 Mar 2023 11:47:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1679053656; 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=q6PNEKXZBANXXYlgBFt4BSZwmt2KtLDfnR7EpFF/l24=; b=QEr6ZWlEz1RxsRIxf3gBBdzWoMa3CRgVx5t/K0QBsh+2JW9lT9jT958osg1G9LMq5EfWAe /7eQIFU+x4+jJ/RSizu581yJu+pNbKGGA6itD36+TcN/xJ6+RuPg8ChWrcZPajfqbeeSFj XIBe+Bs26fzGkkS8W7XaXRjpRjJBar0= 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 4C7D91346F; Fri, 17 Mar 2023 11:47:36 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id unqGEFhTFGTOHQAAMHmgww (envelope-from ); Fri, 17 Mar 2023 11:47:36 +0000 Date: Fri, 17 Mar 2023 12:47:35 +0100 From: Michal Hocko To: Haifeng Xu Cc: hannes@cmpxchg.org, shakeelb@google.com, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [RFC] memcg, oom: clean up mem_cgroup_oom_synchronize Message-ID: References: <20230315070302.268316-1-haifeng.xu@shopee.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230315070302.268316-1-haifeng.xu@shopee.com> X-Rspamd-Queue-Id: 0125F16000A X-Stat-Signature: aqi1xm713iy5e3tty5aid5u5zgmhfzga X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1679053657-890076 X-HE-Meta: U2FsdGVkX18uFcKQgnwxrTfSZ4pYVlh1Krt7JoarevMH3mOKmGck+oF2hJKHlqgjSd9Om1FLhYuO3DV5T83Z0RONFhd3B6iRf0dCei8tlLx76y9VtmqzuuFPJYijr02ybj8F2uCoyPy0/2pD3f8wximURbjnjW3BROsc/B3Fgej5FZP6eeUVMgRXFuGi5yvUf85UoTea5I3R0+GJrkARv6euGKYBApxOor/IaecnUS6jLUnnQNwR9LFIoDpKdcK5Tsuwn2fP662aHRec0PTJ0aS4ny2NvtgAoq58Zhr8Jx+l+6rnW6AfRRE77JdhiOf1350RQ+3lbnwcsUrYKid3ejO0kBFE5Whc1WYpa0zpTXb8p7WRpjaAkoHdKEDkUOb9IhCkrD8PJGfd0yDmhaRuIEtIWaHQdrRa4AowTJNx3ScU6F/3IT2LfHvnd8soP5cCG1XYkVdMyf6z6k/oha/86FIV0dToAI1SZWJ91yksrP2+s9HS7HJNbIl6EfY6wIW/jUNxYepiKbOpL2SeLnzC6is97Vy+F5YwBQvJ3f75zNGEGAHts9hIVJULXako1oZvc9vuakPOKhuKEq2//IxANeL9qLyphQnSmo8Q5JW7Z6ynwYASb+sL+Vl0SAE5Wq5l+LpwzxTHyHs/krcwp/q9luonPQKAkCecyRsperVrK1ate4gZ//wI/2ykWI1rhaRsfUNdoOyAFYw9oqQymMtx94uHHNr6vszj37CBSZ9NzPRNaWUcS92k0o+7cCNTkuW87MKut1oLLnHRDnJPZXyi+AJEZ0/I+1/eznOmDte+ocfTx8rcfMebAXZIYAeVOeDqcgAiY/QmUsjK5OGMQ79dNIYSEu9yqA6aLerxo3G+QbCFWDi+bmI/3ahD0ckasiVYKGuNjn6yBFjq3+XHAbNvfTeDvf2DHG2jEo8qGa5eT4FXWVtT2tOKyH0sv4VjWmb+7n6eKEVBJPANVudjVbq gG9srOlO swSdDpRm/cOWb6lM5xoly+1Y6uXBTI/NVJLSvqzZIGjppEJ07i0/SiB6xY0jGur+0cGRA/T/JRJcSRFHRmSEZGMQn5yQsa0WAvvwDFSUYvrdnaMaAXTuqU5u3Zt+LGAN8sJ6Ttf/QaBpYyMmq3uDMTarCCWXhhv0b8iW+9MEqYmBn0NW3bb+fOOLkTGZQGbWGr0qDo458A61y+FZW6m+iyr4LEl3wiOlT4xwcpvc30OzAe/SdgC0P0n4gmCcb+Ajd74oj4FjivInKCG8= 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 15-03-23 07:03:02, Haifeng Xu wrote: > Since commit 29ef680ae7c2 ("memcg, oom: move out_of_memory back to > the charge path"), only oom_kill_disable is set, oom killer will > be delayed to page fault path. In the charge patch, even if the > oom_lock in memcg can't be acquired, the oom handing can also be > invoked. In order to keep the behavior consistent with it, remove > the lock check, just leave oom_kill_disable check behind in the > page fault path. I do not understand the actual problem you are trying to deal with here. > Furthermore, the lock contender won't be scheduled out, this doesn't > fit the sixth description in commit fb2a6fc56be66 ("mm: memcg: > rework and document OOM waiting and wakeup"). So remove the explicit > wakeup for the lock holder. > > Fixes: fb2a6fc56be6 ("mm: memcg: rework and document OOM waiting and wakeup") The subject mentions a clean up but the fixes tag would indicate an acutal fix. > Signed-off-by: Haifeng Xu > --- > mm/memcontrol.c | 11 ++--------- > 1 file changed, 2 insertions(+), 9 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 5abffe6f8389..360fa7cf7879 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1999,7 +1999,7 @@ bool mem_cgroup_oom_synchronize(bool handle) > if (locked) > mem_cgroup_oom_notify(memcg); > > - if (locked && !memcg->oom_kill_disable) { > + if (!memcg->oom_kill_disable) { > mem_cgroup_unmark_under_oom(memcg); > finish_wait(&memcg_oom_waitq, &owait.wait); > mem_cgroup_out_of_memory(memcg, current->memcg_oom_gfp_mask, Now looking at the actual code I suspect you in fact want to simplify the logic here as mem_cgroup_oom_synchronize is only ever triggered whe oom_kill_disable == true because current->memcg_in_oom is never non NULL otherwise. So the check is indeed unnecessary. Your patch, however doesn't really simplify the code much. Did you want this instead? diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 12559c08d976..a77dc88cfa12 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -1999,16 +1999,9 @@ bool mem_cgroup_oom_synchronize(bool handle) if (locked) mem_cgroup_oom_notify(memcg); - if (locked && !READ_ONCE(memcg->oom_kill_disable)) { - mem_cgroup_unmark_under_oom(memcg); - finish_wait(&memcg_oom_waitq, &owait.wait); - mem_cgroup_out_of_memory(memcg, current->memcg_oom_gfp_mask, - current->memcg_oom_order); - } else { - schedule(); - mem_cgroup_unmark_under_oom(memcg); - finish_wait(&memcg_oom_waitq, &owait.wait); - } + schedule(); + mem_cgroup_unmark_under_oom(memcg); + finish_wait(&memcg_oom_waitq, &owait.wait); if (locked) { mem_cgroup_oom_unlock(memcg); > @@ -2010,15 +2010,8 @@ bool mem_cgroup_oom_synchronize(bool handle) > 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); > - } Hmm, so this seems unneded as well for the oom_kill_disable case as well. Rather than referring to fb2a6fc56be66 it would be better to why the explicit recovery is not really needed anymore. > cleanup: > current->memcg_in_oom = NULL; > css_put(&memcg->css); -- Michal Hocko SUSE Labs