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 C74E1C52D7F for ; Thu, 15 Aug 2024 06:22:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 42A2F6B007B; Thu, 15 Aug 2024 02:22:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3D8556B0082; Thu, 15 Aug 2024 02:22:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2A05F6B0083; Thu, 15 Aug 2024 02:22:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 0E7116B007B for ; Thu, 15 Aug 2024 02:22:08 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 90F70A8643 for ; Thu, 15 Aug 2024 06:22:07 +0000 (UTC) X-FDA: 82453484694.09.CE1DBCF Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) by imf23.hostedemail.com (Postfix) with ESMTP id 9FAF5140010 for ; Thu, 15 Aug 2024 06:22:04 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=V0v8YElq; spf=pass (imf23.hostedemail.com: domain of mhocko@suse.com designates 209.85.128.41 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=1723702852; 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=EOgK6J8p2w/S1o1OKH/zY9ss8HubkQJlyV2xM6D/lbA=; b=JlfRCqzk8xf4NQfl9+MOZ1yVF6oA/BIHqFCjBKjt6sYzkLmYslESbJZDyJ2VSkZ9qirt88 BIgbXI2m0wEr0Ns2qesyZdREkDJ7kO5LYOEJn8tf4Buj+ivpxU/KmFwpq5WoQQfiqtMx43 pzDM/jaaux9p+YeSg7xj2LrWCLL5+4U= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723702852; a=rsa-sha256; cv=none; b=fI6/rGP/euV3CPbAUqzi3dZu4jP6bbnH9/ztH3s9PmaHrgZ3ib4UkfKjh8ub9D/aSQklrr dGFh1FhNdCZ3NjcCWPd3chSbwpAnjHLaDbZn+36g6EuGsVrouzA2DhA37wYO6qn0FTJCg6 AYNN8S5V8axTVSDosOdKE6nX0JJ/Qo4= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=V0v8YElq; spf=pass (imf23.hostedemail.com: domain of mhocko@suse.com designates 209.85.128.41 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com Received: by mail-wm1-f41.google.com with SMTP id 5b1f17b1804b1-42808071810so3649715e9.1 for ; Wed, 14 Aug 2024 23:22:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1723702923; x=1724307723; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=EOgK6J8p2w/S1o1OKH/zY9ss8HubkQJlyV2xM6D/lbA=; b=V0v8YElqzFJPRNOEnSd4l4rOsH4YOT/VDsVCHru3C6ZQrj6rBXjLEdxfFR7kP09ZKI S9G85zmxzFlMAiUuvWVoiM4+6ORoLP+GxyP4FOoVKZ61QTK5s+KN67lEmm0d6BAM6E1G srv8QHYUI0PHfIntAuGhj4r1RnEszYQRHZrwQBtKJitAexCUlFS6f+NnGDPov0ejs+s5 P5BcAt1zHQltVSJTZbzjkORKWODwpaGbgBRBP4EG5EQnfCNS99kH5FttVj/+XaV/D5Df GyZx7y4StciCcSN/elqoQxb1FJZzguBBoYEekzMFgZDDR5l/bPzzNyGAVX+MN26wYtHe AyPQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723702923; x=1724307723; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=EOgK6J8p2w/S1o1OKH/zY9ss8HubkQJlyV2xM6D/lbA=; b=UtifeCcp/oa2uQct6G1lQm8v47VmUZis9UTggiEsH2n/FbcnaAztEgCH4cECFaIRNX JKtCjkLUDQkLsqmLnF9HqfUMhejIsQjNVWfjow47f10+VxPgcGQnzG3aR+hyom0NoBbe W11YP8ZGaFQ8GybCHhuqF8eEvIfRD/FrhMFyKjTkNxo+tLg+0X8fA/gfFOGAX6OIhaSz MZ6C5xJ6PqiR3AU/tYIGitqIEclSujV/M1MpQp6b541R0OyyM4q07NHX35zjDjQguUkF xIVGxPct5jY3iK63BK0bafeT7oDVgoh2wK0oEm1yEoArfOEjVTea6RjAdSgozO9zH9QV EV6A== X-Forwarded-Encrypted: i=1; AJvYcCV2yuln/Cu5rtrIfP2dqpyVY/1JjIJY35o+LRZrMiIZZqXbq1/ycCmYnf9rMKz3sVoCWG7m2LJjMnqvWpuSc1hGCVo= X-Gm-Message-State: AOJu0YxpmdTcHvWAyYEXJAZHlYf34QklcJqBZ1UyNrdZqbKstCROSs0K jQeLkl8w45OW87wqpXOlrLiBR514XX8srlxcP0dkgfjJRuzrsSKYVw6gpCPl1xA= X-Google-Smtp-Source: AGHT+IH4sQrqFjAznFSRnjLbAId9X2L98DtewLKQ2JpwY8wc33BQwNeSahwx/nG4PdH+FEoxz6fpnA== X-Received: by 2002:a05:600c:1551:b0:426:62c5:4742 with SMTP id 5b1f17b1804b1-429dd22f39bmr38404145e9.7.1723702922723; Wed, 14 Aug 2024 23:22:02 -0700 (PDT) Received: from localhost (109-81-83-166.rct.o2.cz. [109.81.83.166]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-429e7e1c46fsm9842485e9.39.2024.08.14.23.22.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Aug 2024 23:22:02 -0700 (PDT) Date: Thu, 15 Aug 2024 08:22:00 +0200 From: Michal Hocko To: Yafang Shao Cc: Christoph Hellwig , akpm@linux-foundation.org, viro@zeniv.linux.org.uk, brauner@kernel.org, jack@suse.cz, david@fromorbit.com, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, Kent Overstreet Subject: Re: [PATCH 1/2] mm: Add memalloc_nowait_{save,restore} Message-ID: References: <20240812090525.80299-1-laoar.shao@gmail.com> <20240812090525.80299-2-laoar.shao@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 9FAF5140010 X-Stat-Signature: 5rt1gq9g484k47ne9hsigj1owng5ddff X-HE-Tag: 1723702924-57199 X-HE-Meta: U2FsdGVkX19eBoyNnoLI5eMiegO4qWUrX+gJU/KzKr6AQ/hv8lP2+FbOUrhto0di8emfuh5YAo2mCWG5qXLXKFzRjcwz7FjB19yDRviezf2ik58MVTZcZfVSROSkFTzmc2DCtX0HsehSsg6SluypTPftVQ2Bkxk/4HRCsIJDGgp0DcEpT7pxYp2hjWPrHSrjisgqRcWW8eOW4B7twwq4G9HdjpDgYAxP7PLHbj2Zh8M5Rs9nh24cGbE0ARfkZtE9Dui4aU7x+nez7q7IdHefNx1n0ELF4IApQ0lMhTyd82HmkdHhtEJ14qEnAk3HwIghzIYKFU9fzVPjrbjI3UCz5Eh3d+e7qHMdQQIqkQf62qPLSzz2KwkjBhIkGeCMR1RaL0GL7RoJ3XPlNv5O4C9OtjZKjALaNoNd1Ae6r/F5bgWnevcEWpq2GsfZAqJ1UiaR7bIDOlKPpcpbgpfjUAfL8dvHbcHY5p9rdKFNPDCLQrlhTKoW3pTbdLl3Uz9YcuTqA/VoZ7vKwwHmJFNlI4TCJ7kv6BHRpSQ7Wkb519aD247eI1f1nNgRb3T5jSfFXw1ZpyydpiuWR0ebv9dr6mBS5yk0fe3CBF9115hjyxpkoq6A4MQSkCMXFLMTzvqayA55n0RtncD8bfhXg2xXP96K6/svMnbWoWID+G+lfaTvtFPc/SkWvivcu5ZwVcDxhlsnm1xd9gYmndjFkIYFvo5IjhGoLeYh2m84ENfS7vCc1ufGzfv1ulIO7O5YbZi5cG0i6yjWBCJMjUnMcehGOmB+UuhkIQMwGxgftllNBGcer5vGrfq6RVV9+T/aqAL2l4+zOpz49r+KbBKfJ21neNX7NvEuEuLKcIbk2PZZ8UaJXBpAb2Uf+vZSN8UYI4ReuaooeY62nB3OOsJcItO9rIUL/DssRKmlzo4Uj7CAOpHW+oEZTfQrFLcM/5jIy6+cixpnefe/E8q1FV2MpNyq78F L8wG8ObX 07TzfouBUll65E5fC3aJrTZVW0wswFSGx8QO3L+M1LRnEZ3biMSl8regf/XxmnzZ0Ntae9u3NVZJDUHnRKBqhubwwPMaVTjHsh4nQJ6IRWJgwwCrFIThYShkggnP34Ovc1wjbd8obzA0EIyb4TMJXEctGBRlo0aVqznlUXT5esgw3bwza+Hm1UOhKW/UpTiYk022ZZbSR64Pgo4hAyTq2iFYlQ7+VN1yq4Qs8f4SCpCtS9luqJkWbuF9mkf5XDlDlWTn+nUAkzzw24IyKcbJFjsioPHVch+64zO+NWdGOWOuTKQ/GSnAUFqNOcJG276PGiHf03IQKcuhz2zYkr1sJljkCD52oppgIZ83x4nBfnH9kcgOE7mRszBrPJI3Y9xooHo6L5O4+xQrFMaUoml/NhZDVOkdXPvvpMdpOrFXrKgS+l66ROztSwDx1XOEOdmIAGRh3/v0Gfd9d4n35qIwugIT4NH3H9frfoZyJ8VHh/2FoQ8XxjjpZDCzi1Ul4dYVJaQX1FqkChRTcE0DPMZEAKXz6/lLVxjkdaPOI7qPKHfe3BCbWZ9Bv9FMwy7GpX0ogliBF 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 15-08-24 11:26:09, Yafang Shao wrote: > On Wed, Aug 14, 2024 at 8:43 PM Michal Hocko wrote: [...] > > > If that's the case, I believe we should at least consider adding the > > > following code change to the kernel: > > > > We already do have that > > /* > > * All existing users of the __GFP_NOFAIL are blockable, so warn > > * of any new users that actually require GFP_NOWAIT > > */ > > if (WARN_ON_ONCE_GFP(!can_direct_reclaim, gfp_mask)) > > goto fail; > > I don't see a reason to place the `goto fail;` above the > `__alloc_pages_cpuset_fallback(gfp_mask, order, ALLOC_MIN_RESERVE, ac);` > line. Since we've already woken up kswapd, it should be acceptable to > allocate memory from ALLOC_MIN_RESERVE temporarily. Why not consider > implementing the following changes instead? > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 9ecf99190ea2..598d4df829cd 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -4386,13 +4386,6 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned > int order, > * we always retry > */ > if (gfp_mask & __GFP_NOFAIL) { > - /* > - * All existing users of the __GFP_NOFAIL are blockable, so warn > - * of any new users that actually require GFP_NOWAIT > - */ > - if (WARN_ON_ONCE_GFP(!can_direct_reclaim, gfp_mask)) > - goto fail; > - > /* > * PF_MEMALLOC request from this context is rather bizarre > * because we cannot reclaim anything and only can loop waiting > @@ -4419,6 +4412,14 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned > int order, > if (page) > goto got_pg; > > + /* > + * All existing users of the __GFP_NOFAIL are blockable, so warn > + * of any new users that actually require GFP_NOWAIT > + */ > + if (WARN_ON_ONCE_GFP(!can_direct_reclaim, gfp_mask)) { > + goto fail; > + } > + > cond_resched(); > goto retry; > } How does this solve anything. It will still eventually fail the NOFAIL allocation. It might happen slightly later but that doesn't change the fact it will _fail_. I have referenced a discussion why that is not really desireable and why Barry wants that addressed. We have added that WARN_ON_ONCE because we have assumed that people do understand that NOFAIL without reclaim is just too much to ask. We were wrong there was one user in the kernel. That one was not too hard to find out because you can _grep_ for those flags. Scoped APIs make that impossible! > > But Barry has patches to turn that into BUG because failing NOFAIL > > allocations is not cool and cause unexpected failures. Have a look at > > https://lore.kernel.org/all/20240731000155.109583-1-21cnbao@gmail.com/ > > > > > > I am really > > > > surprised that we even have PF_MEMALLOC_NORECLAIM in the first place! > > > > > > There's use cases for it. > > > > Right but there are certain constrains that we need to worry about to > > have a maintainable code. Scope allocation contrains are really a good > > feature when that has a well defined semantic. E.g. NOFS, NOIO or > > NOMEMALLOC (although this is more self inflicted injury exactly because > > PF_MEMALLOC had a "use case"). NOWAIT scope semantic might seem a good > > feature but it falls appart on nested NOFAIL allocations! So the flag is > > usable _only_ if you fully control the whole scoped context. Good luck > > with that long term! This is fragile, hard to review and even harder to > > keep working properly. The flag would have been Nacked on that ground. > > But nobody asked... > > It's already implemented, and complaints won't resolve the issue. How > about making the following change to provide a warning when this new > flag is used incorrectly? How does this solve anything at all? It will warn you that your code is incorrect and what next? Are you going to remove GFP_NOFAIL from the nested allocation side? NOFAIL is a strong requirement and it is not used nilly willy. There must have been a very good reason to use it. Are you going to drop the scope? Let me repeat, nested NOFAIL allocations will BUG_ON on failure. Your warning might catch those users slightly earlier when allocation succeed but that doesn't make those crashes impossible. PF_MEMALLOC_NORECLAIM might be already merged but this concept is inherently fragile and should be reverted rather than finding impossible ways around it. And it should be done before this spreads outside of bcachefs. -- Michal Hocko SUSE Labs