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 D2818C4707B for ; Thu, 11 Jan 2024 08:46:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2DE6F6B007D; Thu, 11 Jan 2024 03:46:13 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 28DF36B007E; Thu, 11 Jan 2024 03:46:13 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0E2356B0080; Thu, 11 Jan 2024 03:46:13 -0500 (EST) 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 ECE656B007D for ; Thu, 11 Jan 2024 03:46:12 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id B3736A20E5 for ; Thu, 11 Jan 2024 08:46:12 +0000 (UTC) X-FDA: 81666398184.08.A34FF35 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131]) by imf08.hostedemail.com (Postfix) with ESMTP id 8E7D716000D for ; Thu, 11 Jan 2024 08:46:10 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=e4m9cSu+; dkim=pass header.d=suse.com header.s=susede1 header.b=lW37eauM; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf08.hostedemail.com: domain of mhocko@suse.com designates 195.135.223.131 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1704962771; 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=IdRbXT6KWpUzahecRbqmMLz9QYgtWWSOiBuKfzahGm4=; b=5hWk21ioyJwwbLCl8SUGD6DPepZEbA9OaPn0PaFpbwBQFSlPpD7QXqraAXCK8FOwBfgsdb HVPF3yRNl5S2NluXpPs2NJ+lVOPoryoTgFrsdOR6p6kyBBRGKlLjlF4EJlfTJazGBaHoMn kev4xvOxh62AIBxA7ciglaLvHDr31Nw= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=e4m9cSu+; dkim=pass header.d=suse.com header.s=susede1 header.b=lW37eauM; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf08.hostedemail.com: domain of mhocko@suse.com designates 195.135.223.131 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1704962771; a=rsa-sha256; cv=none; b=Bj//moKR/3I78Fj5WiBcnPpE5Aw6UPzEuMjcs3SwlrJQzmTNhsEdaFkjX8mZxvvXJAve4f w0UbZgQACirTHOy1CQOzorxJqNC8tJvwxHYHeKs9YIIDKicgesTO7B+j4ahenM/bvXCIoX DgC/qjSpaxeuVlXItLXdarOlgDVPKJA= Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [10.150.64.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 237BB1FB8D; Thu, 11 Jan 2024 08:44:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1704962678; 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=IdRbXT6KWpUzahecRbqmMLz9QYgtWWSOiBuKfzahGm4=; b=e4m9cSu+JtjkKXBsa/A8ayGBh/KZsh+30cjHUIaGPv4RGg/99sP8YbSWDzHTA9ZUScXEdf S4I2cmqrF9SCcF/3h4GiNkaBFvUJeArqaQMICR+GO08pQAvTSLMKbm2rUt1FxWV3AnzAWh nk2iN8sT6cMWx/v18I5a5Fn7R3Zrgd8= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1704962754; 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=IdRbXT6KWpUzahecRbqmMLz9QYgtWWSOiBuKfzahGm4=; b=lW37eauMFxGqcxJ5l1Qy3X5TVFa5XvZzxj7ajhxTiqQa3hJHQeW+2LkT/aXoJQ3skpEKJ7 pJCu8CDZx6QDMag1fR0KR7ZP+JZDxAuZID7dwdqJJX9ySUuOK6aYmzjNTsnFjQ3tahEETS Wo7fcYNDFemMdH5BjxKSQ0k91iX5qps= Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 413C513635; Thu, 11 Jan 2024 08:44:38 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id afVYDHaqn2VFbwAAD6G6ig (envelope-from ); Thu, 11 Jan 2024 08:44:38 +0000 Date: Thu, 11 Jan 2024 09:46:08 +0100 From: Michal Hocko To: Jianfeng Wang Cc: akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm, oom: Add lru_add_drain() in __oom_reap_task_mm() Message-ID: References: <20240109091511.8299-1-jianfeng.w.wang@oracle.com> <1d866f1b-94b3-43ec-8f4c-2de31b82d3d1@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1d866f1b-94b3-43ec-8f4c-2de31b82d3d1@oracle.com> X-Rspam-User: X-Stat-Signature: 4ewyxgu7sfwi6543ucu5r7ezqy8osg5m X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 8E7D716000D X-HE-Tag: 1704962770-578296 X-HE-Meta: U2FsdGVkX1+VeBvlJKdtuPG8xKFvEwyPHEYvU6MvVO2JfyjdIswTndaC9bLWry3zyN1GG+nk7LqG/rh6Rfb/PQsF0yhTI3yjMs3TXa42oQiSII+v+/xBRTCIaBEAgrOyALQKH58k9bM5Oj6nApzQVtCzwMuLsAUIUsEve0OIQ8wqiCmfEEic52XqTHNzTzJlrgrsqp3Hxx1fTeZL2w7+Xksce7MgCm5MdkTJBcoZk8+ogMLK4IIjFkxx+kPXY5rriu+iqBz0ed4J3Of0mi6220+aeZk07ZjNq3qrxIlBqdy2h41r0ZvDwefU21fNaWMYW6cKRFGB8uhagRiKNfcr/REpEaAFoi7ss0PiLEpwG7I5mJ3XVS4O/H3ykuPFk+EBcCLj2NFe9uj7qSgsmz44ZjbUsArlkCZBwuU/5TgrDYiqUZmZS4uAzlyJtoMLhUJM6qohqpas4eWbpsxpBpWKTSBkcIix1FQtxaLChEcp0cg0ZQdTHEGpgBc3g4/RNMykih9UkeyfuYRQpytKoV/rp9POtIfwrclDnB4oO6Fu1M0a0GgxrfBbCZ7QqCYGR6CxqHvs01UrcHdu5V+vr/+Xk1tiEwi+W1903imGVDF5N0APWoVzThnf9r6py6z/7xn0fDKjoarVG6ybNGuAoc/GjdSp2Vw3oWURznBr4VShUhEo4o5nT6k3xTRMu4cCye2Zaq28HYcMbSYQCz02ziv8Jj2dUrIWQxNFACvz6E0YpDXS4fUagnBBboER4nHTBEfQF0JAmd74Kd8O7KPWR7bEshqchN2lWlZAjsCJKC/Rto2Wt+G0t3eJoaiEUKfiXqHfcl5Tk97U+7+wX04gyUIXfPvKmXfQdLxHr9VQjXPQgubDDl8orQ3qUFPkIEIxkfAXU+9X0FXiFCeDfiPaYjo7pEMIG51HFOR6u0QvAePHFxdSS2ViTHHm4Pf3QW2EfGEerjwy5BJ1PKFbUtTrUSZ lnntdAED 2S+2pXileuD5Ts5wfxUa5ev/rU2H3saZ8IVz0y45gTSM9xym3w3uG9L/7QTPmoLzi6cvwqr5efoUuVg1H25lAHfosh06oB/9Q3CsuLRhN84l6oq0u3oGtqDmPWX5EHqpmlPyYHl6SdbHmSVPGmujVMWF4lAEcyssYZBKXnD6qRMQngCqRpOwAs3wwqgK4IXQ47crMQuCYK4smb7DZKNK/wkVhHcl76jjJjigQ5sxmxzGABv+umAy/H4aI0QaPO+cb+bIE5ZWy4pbqdvM= 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 Wed 10-01-24 11:02:03, Jianfeng Wang wrote: > On 1/10/24 12:46 AM, Michal Hocko wrote: > > On Tue 09-01-24 01:15:11, Jianfeng Wang wrote: > >> The oom_reaper tries to reclaim additional memory owned by the oom > >> victim. In __oom_reap_task_mm(), it uses mmu_gather for batched page > >> free. After oom_reaper was added, mmu_gather feature introduced > >> CONFIG_MMU_GATHER_NO_GATHER (in 'commit 952a31c9e6fa ("asm-generic/tlb: > >> Introduce CONFIG_HAVE_MMU_GATHER_NO_GATHER=y")', an option to skip batched > >> page free. If set, tlb_batch_pages_flush(), which is responsible for > >> calling lru_add_drain(), is skipped during tlb_finish_mmu(). Without it, > >> pages could still be held by per-cpu fbatches rather than be freed. > >> > >> This fix adds lru_add_drain() prior to mmu_gather. This makes the code > >> consistent with other cases where mmu_gather is used for freeing pages. > > > > Does this fix any actual problem or is this pure code consistency thing? > > I am asking because it doesn't make much sense to me TBH, LRU cache > > draining is usually important when we want to ensure that cached pages > > are put to LRU to be dealt with because otherwise the MM code wouldn't > > be able to deal with them. OOM reaper doesn't necessarily run on the > > same CPU as the oom victim so draining on a local CPU doesn't > > necessarily do anything for the victim's pages. > > > > While this patch is not harmful I really do not see much point in adding > > the local draining here. Could you clarify please? > > > It targets the case described in the patch's commit message: oom_killer > thinks that it 'reclaims' pages while pages are still held by per-cpu > fbatches with a ref count. > > I admit that pages may sit on a different core(s). Given that > doing remote calls to all CPUs with lru_add_drain_all() is expensive, > this line of code can be helpful if it happens to give back a few pages > to the system right away without the overhead, especially when oom is > involved. Plus, it also makes the code consistent with other places > using mmu_gather feature to free pages in batch. I would argue that consistency the biggest problem of this patch. It tries to follow a pattern that is just not really correct. First it operates on a random CPU from the oom victim perspective and second it doesn't really block any unmapping operation and that is the main purpose of the reaper. Sure it frees a lot of unmapped memory but if there are couple of pages that cannot be freed imeediately because they are sitting on a per-cpu LRU caches then this is not a deal breaker. As you have noted those pages might be sitting on any per-cpu cache. So I do not really see that as a good justification. People will follow that pattern even more and spread lru_add_drain to other random places. Unless you can show any actual runtime effect of this patch then I think it shouldn't be merged. -- Michal Hocko SUSE Labs