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=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,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 67619C433EF for ; Thu, 16 Sep 2021 00:38:00 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 06EE5610A6 for ; Thu, 16 Sep 2021 00:37:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 06EE5610A6 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=fromorbit.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 810FC900002; Wed, 15 Sep 2021 20:37:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7C10B6B0072; Wed, 15 Sep 2021 20:37:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 63D77900002; Wed, 15 Sep 2021 20:37:59 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0251.hostedemail.com [216.40.44.251]) by kanga.kvack.org (Postfix) with ESMTP id 552A96B0071 for ; Wed, 15 Sep 2021 20:37:59 -0400 (EDT) Received: from smtpin14.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 05979181D6068 for ; Thu, 16 Sep 2021 00:37:59 +0000 (UTC) X-FDA: 78591574278.14.64342BC Received: from mail109.syd.optusnet.com.au (mail109.syd.optusnet.com.au [211.29.132.80]) by imf20.hostedemail.com (Postfix) with ESMTP id 12C55D0000AA for ; Thu, 16 Sep 2021 00:37:57 +0000 (UTC) Received: from dread.disaster.area (pa49-195-238-16.pa.nsw.optusnet.com.au [49.195.238.16]) by mail109.syd.optusnet.com.au (Postfix) with ESMTPS id 2CA3E87E46; Thu, 16 Sep 2021 10:37:53 +1000 (AEST) Received: from dave by dread.disaster.area with local (Exim 4.92.3) (envelope-from ) id 1mQfPU-00CxNW-5j; Thu, 16 Sep 2021 10:37:52 +1000 Date: Thu, 16 Sep 2021 10:37:52 +1000 From: Dave Chinner To: NeilBrown Cc: Michal Hocko , Mel Gorman , Andrew Morton , Theodore Ts'o , Andreas Dilger , "Darrick J. Wong" , Jan Kara , Matthew Wilcox , linux-xfs@vger.kernel.org, linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/6] EXT4: Remove ENOMEM/congestion_wait() loops. Message-ID: <20210916003752.GN2361455@dread.disaster.area> References: <163157808321.13293.486682642188075090.stgit@noble.brown> <163157838437.13293.14244628630141187199.stgit@noble.brown> <20210914163432.GR3828@suse.com> <163165609100.3992.1570739756456048657@noble.neil.brown.name> <163174534006.3992.15394603624652359629@noble.neil.brown.name> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <163174534006.3992.15394603624652359629@noble.neil.brown.name> X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.3 cv=YKPhNiOx c=1 sm=1 tr=0 a=DzKKRZjfViQTE5W6EVc0VA==:117 a=DzKKRZjfViQTE5W6EVc0VA==:17 a=IkcTkHD0fZMA:10 a=7QKq2e-ADPsA:10 a=7-415B0cAAAA:8 a=CuGUvM-gP3KvDOsnQb0A:9 a=QEXdDO2ut3YA:10 a=biEYGPWJfzWAr4FL6Ov7:22 X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 12C55D0000AA X-Stat-Signature: y7ipzexthjoz16539fqrje33g5yd8o7j Authentication-Results: imf20.hostedemail.com; dkim=none; dmarc=none; spf=none (imf20.hostedemail.com: domain of david@fromorbit.com has no SPF policy when checking 211.29.132.80) smtp.mailfrom=david@fromorbit.com X-HE-Tag: 1631752677-514757 Content-Transfer-Encoding: quoted-printable 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, Sep 16, 2021 at 08:35:40AM +1000, NeilBrown wrote: > On Wed, 15 Sep 2021, Michal Hocko wrote: > > On Wed 15-09-21 07:48:11, Neil Brown wrote: > > >=20 > > > Why does __GFP_NOFAIL access the reserves? Why not require that the > > > relevant "Try harder" flag (__GFP_ATOMIC or __GFP_MEMALLOC) be incl= uded > > > with __GFP_NOFAIL if that is justified? > >=20 > > Does 5020e285856c ("mm, oom: give __GFP_NOFAIL allocations access to > > memory reserves") help? >=20 > Yes, that helps. A bit. >=20 > I'm not fond of the clause "the allocation request might have come with= some > locks held". What if it doesn't? Does it still have to pay the price. >=20 > Should we not require that the caller indicate if any locks are held? > That way callers which don't hold locks can use __GFP_NOFAIL without > worrying about imposing on other code. >=20 > Or is it so rare that __GFP_NOFAIL would be used without holding a lock > that it doesn't matter? >=20 > The other commit of interest is >=20 > Commit: 6c18ba7a1899 ("mm: help __GFP_NOFAIL allocations which do not t= rigger OOM killer") >=20 > I don't find the reasoning convincing. It is a bit like "Robbing Peter > to pay Paul". It takes from the reserves to allow a __GFP_NOFAIL to > proceed, with out any reason to think this particular allocation has an= y > more 'right' to the reserves than anything else. >=20 > While I don't like the reasoning in either of these, they do make it > clear (to me) that the use of reserves is entirely an internal policy > decision. They should *not* be seen as part of the API and callers > should not have to be concerned about it when deciding whether to use > __GFP_NOFAIL or not. Agree totally with this - we just want to block until allocation succeeds, and if the -filesystem- deadlocks because allocation never succeeds then that's a problem that needs to be solved in the filesystem with a different memory allocation strategy... OTOH, setting up a single __GFP_NOFAIL call site with the ability to take the entire system down seems somewhat misguided. > The use of these reserves is, at most, a hypothetical problem. If it > ever looks like becoming a real practical problem, it needs to be fixed > internally to the page allocator. Maybe an extra water-mark which isn'= t > quite as permissive as ALLOC_HIGH... >=20 > I'm inclined to drop all references to reserves from the documentation > for __GFP_NOFAIL. I think there are enough users already that adding a > couple more isn't going to make problems substantially more likely. An= d > more will be added anyway that the mm/ team won't have the opportunity > or bandwidth to review. Yup, we've been replacing open coded loops like in kmem_alloc() with explicit __GFP_NOFAIL usage for a while now: $ =E2=96=B6 git grep __GFP_NOFAIL fs/xfs |wc -l 33 $ ANd we've got another 100 or so call sites planned for conversion to __GFP_NOFAIL. Hence the suggestion to remove the use of reserves from __GFP_NOFAIL seems like a sensible plan because it has never been necessary in the past for all the allocation sites we are converting from open coded loops to __GFP_NOFAIL... Cheers, Dave. --=20 Dave Chinner david@fromorbit.com