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 610D5C52D6F for ; Wed, 21 Aug 2024 12:41:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B48D26B00F6; Wed, 21 Aug 2024 08:41:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AF8756B0112; Wed, 21 Aug 2024 08:41:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 973166B0113; Wed, 21 Aug 2024 08:41:37 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 744D36B00F6 for ; Wed, 21 Aug 2024 08:41:37 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 17DC080C20 for ; Wed, 21 Aug 2024 12:41:37 +0000 (UTC) X-FDA: 82476213834.29.CCF35D2 Received: from mail-vs1-f41.google.com (mail-vs1-f41.google.com [209.85.217.41]) by imf13.hostedemail.com (Postfix) with ESMTP id 4B9272001D for ; Wed, 21 Aug 2024 12:41:35 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=bycgNBQw; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf13.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.217.41 as permitted sender) smtp.mailfrom=laoar.shao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724244006; 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=cLZMJpmpzGt5GhCbFn0EpW7Uj9M6X8hFyELkG7gj5uU=; b=OwmVcYEl399jesKnFv8ZkM3tKEmA0MuC3WeSNPOmdnf+SbxovwxOyp+PzFSBH/WNBhwrim TfKbwtLe9tN+BtU1jtqIVQnfRx7ipYgoeS6KrQ51dq7cYeVs6O0IPJMSUzAR/BDgZqiqxj MGUZ0LIk8LUEuRFU/XqATWsVdBQNl5Y= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724244006; a=rsa-sha256; cv=none; b=yyqs5q9DlYtZbBcsjqNd14qmva41nZucRnsbghUT/P5FNc3gjQSQhqRkt1yZ+1xtze25YV uHXj5351diYPp7TYQnfOg1xCqyz4IzrPpt+zDFw+YzcPb4lzNwNDBXee8FYtf3n6nL5uJN Y11OP73G46rof7L3mQESbrMqMVk9UAE= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=bycgNBQw; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf13.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.217.41 as permitted sender) smtp.mailfrom=laoar.shao@gmail.com Received: by mail-vs1-f41.google.com with SMTP id ada2fe7eead31-498c4f251ffso730703137.3 for ; Wed, 21 Aug 2024 05:41:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724244094; x=1724848894; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=cLZMJpmpzGt5GhCbFn0EpW7Uj9M6X8hFyELkG7gj5uU=; b=bycgNBQwuQOZD24XCaeeMcQIWN+wme5bvbFqf9gicDa5hGVUzHg0/Bpp0pKALR9AaA WWz3gcOVwCACJoaqnfq4LSVMreI3wiuMYVmwjhqgPUGbwIh10Z7tDjq8It7YxJlFKMbM rsYbnxZ0ZE95ULjKB+2zhqvyy+8++wePdXXYtG9GdWOal4Uq6Vtvl89vJb61yyVxcD6M Fc2+EErc/Nr9HX2UCnwTMATSy5qA747SJ+Er/Tf3hhuqoB7JwOKElgNjz5zpWIrg6UpP fktD24UrY8/G6pmMgNoAezSqZLZUNF5QnTWZsDqxVqAkQQLqRlN3aJx4fqIfNQXSGXMP r9QQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724244094; x=1724848894; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=cLZMJpmpzGt5GhCbFn0EpW7Uj9M6X8hFyELkG7gj5uU=; b=nCzPAEqZaDkaXlOhQNm2MeZ0RZvPOxZWzJeqmy4ijEwnWkQ8b6lkXZHwH/xjwdj+4w nR/0Og0a5bPBDNQc5VAI5561ZxlIUcvcoGxSJMngV4nfXy73ffhHpZwe3Wrkwq843cFs Qp+zrnnl+nWMJtsiRDAttvmZq5oVdyfceKhS2wQqr8dJ+UrNRSqtp+mYtmMckIs3MsyL 9rq4PKmNY49tAxgoXkS67wLTGDxVMovFVMffjqt8BAOEnvSAFW4KduEr/5kE9ROJi3Yc AKk2p1yNpSWD2JbjZkHgTnGL8fDjgtq/xwFpsfecByiyjRucjmmQOEusopsEGM4xN4rI T6dA== X-Forwarded-Encrypted: i=1; AJvYcCVxTRmJBx4Lpy500rmlCV9PWpC2hRdDK3KuihJ/TOwDiNyCGAPqCjRz2Y8BbyQ6fbmJWnh24zb+nQ==@kvack.org X-Gm-Message-State: AOJu0Yz/7V1qkeuAVVlvSKIgqXTH3QF10A8rDYtZgbof4T0XIs0t3Fjs Wj97jQvwqkcgKCg4/z5L2omevxoPV00EKn9WiX6Kts2/wXMIfGqD8QWoohtmhqDY7ahiSu3X+It vF4Ry0/a7c7Fb4J24efNJ8nyoc2U= X-Google-Smtp-Source: AGHT+IGnM2JOP3BDTAlgVEWy9Jwa8Xn67U8sTX4goz/aFENoIfFfkW/zg10z5jA+HbAXH3KlXwA39DrlPe0lgrnjkTA= X-Received: by 2002:a05:6102:26c8:b0:497:6bb5:398a with SMTP id ada2fe7eead31-498d2eb7a78mr2537587137.7.1724244094122; Wed, 21 Aug 2024 05:41:34 -0700 (PDT) MIME-Version: 1.0 References: <20240817062449.21164-1-21cnbao@gmail.com> <7050deab-e99c-4c83-b7b9-b5dad42f4e95@redhat.com> In-Reply-To: From: Yafang Shao Date: Wed, 21 Aug 2024 20:40:56 +0800 Message-ID: Subject: Re: [PATCH v3 0/4] mm: clarify nofail memory allocation To: Linus Torvalds Cc: David Hildenbrand , Barry Song <21cnbao@gmail.com>, akpm@linux-foundation.org, linux-mm@kvack.org, 42.hyeyoo@gmail.com, cl@linux.com, hailong.liu@oppo.com, hch@infradead.org, iamjoonsoo.kim@lge.com, mhocko@suse.com, penberg@kernel.org, rientjes@google.com, roman.gushchin@linux.dev, urezki@gmail.com, v-songbaohua@oppo.com, vbabka@suse.cz, virtualization@lists.linux.dev Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 4B9272001D X-Stat-Signature: t8c4jacikyn3zrs8hnantz1jn13h4qus X-Rspam-User: X-HE-Tag: 1724244095-323634 X-HE-Meta: U2FsdGVkX18ytvIZcLUbMI1v7lbmsFPp7BTU+stzl8wXM/ClsHx0VNYLCdshHS5bZ7Dx7qbWniDP3Hof7PfnDUOSYVTgX0n5rlga0QALLEDpFRWzTYhtUg5vPaIXosU+nYXEhJ2jSgaVUHYeQf+xSdkGpg8JM+mST8WFNhYixt/ypMgaE11pKSboK6LkQg5UFTEvfHWeKY4c07g1p2EU5NiQQZXlSHmvxENa011OBKYZ+f4MZsEw2z2xS6CC5M7eayGrfhiCfA46FRXFRv7g5tLqU3DfEXPAvNE7Niy9Ao0SiZxq9a1EgMmuxsHDHLm3ndHntc4hQvVeajaOpVidff+1kWWh+kp+v1Yap/sBl2mfLB7kxEQnGkM0St/t8lQb2hqS6KXZpcK4ZhRIF3RUDvxh5gruCi8jNbuu2Z06qncuIMbju9696HJ28HMwWKqMgg5AieWyxM1i05uKUSGHrYdJw8n4t6oix652xCZcTidIFABqKc5NtWqNhPOmKQU3a4Mnr529T2P/nfZVD/R+JDefI0Id3tKssBryNJGJUxru9wXByOci2jbRvTH4++o1Po/M+3iUrIQJraMqFwOoeH6EgWZsjskOa2GqeaEIHNwbuuF32ThTMC6Q32HcpfKr4fI7Kj2WM+0WtJWYHYtuxipebiH5U0uHYiuvyS3cyzdJ8gl5guokD2ES11nKKLeMnROxckgP97Zr0a1hVvqezFuFNuN+9wGoSbZnFpPPd2JVODzwYP6aZq1h917/XKAE0fdNr0+vXSYkC6IhBLwS4tCamG+Ne7QhTfpgd4EVTFvt97xeD8w93OKIDcnycUbR1AlR1laHtwbpwnnPJNW+O9K3duxENWkc9/qc0SdynInJT3p1IMagRkDtD7xG3dYvLRK+JbEpolO/Mm+abm82LWq5It6yTc45XGE9wDnS9/A92Xtv2d2CF4s4OGqZa8NkxQkpTkKqc7Sgg/jKwxQ uYZetDAx SGxOeXHulnQgYoC4BD4cvM5rOfbu/RTqGFeTbcgdTjIFK6WjOReuFOGz4kwpSRKgGwAfzPTHtusSTMed8ohnYMoYRVpJVzYlwdAEecV50eLVPgbStZg82HiXvyZP2ztBVnhDgGHRSihTvRAq0NvgCxkoBDCXRWSspvaFjqGXDb+3T2J/aopso5lvNEOyDIgKKPqRBDZUgaVRGEFpiswpcGnMLlcUIEKTY0WLXB4Bb4A86Vn90nYcY/paTLKSZApOZHiAtcKvk/RjjFmWx/i4YqZvAFP95NlUurfvnQJTi0MXDwXGHGa/s3NMHHWI8iCS8yqrwWQfezJm6f2IJHcxiiT9JOSieHMzDykwH4ILhXkcJ5wTnCNzDDQFH2B9d1YQQ1TO8KaOtsgHw036+gIl4wtTO80zoVH5wuk0UiVVmI+2W+1k+OdBFs+JjXPmg4C4RE8cBDn970M4Ia8cDAh21SjTisV04nNboydkii/iOvk+fZ2AmX5JPXIrmyrf8mY2EC8hk0s6DBIWiCuvsWyEXPjorQe3Vgsc/BAER 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 Tue, Aug 20, 2024 at 12:05=E2=80=AFAM Linus Torvalds wrote: > > On Mon, 19 Aug 2024 at 06:02, David Hildenbrand wrote: > > > > > > If we must still fail a nofail allocation, we should trigger a BUG ra= ther > > > than exposing NULL dereferences to callers who do not check the retur= n > > > value. > > > > I am not convinced that BUG_ON is the right tool here to save the world= , > > but I see how we arrived here. > > I think the thing to do is to just add a > > WARN_ON_ONCE((flags & __GFP_NOFAIL) && bad_nofail_alloc(oder, flags)= ); > > or similar, where that bad_nofail_alloc() checks that the allocation > order is small and that the flags are sane for a NOFAIL allocation. > > Because no, BUG_ON() is *never* the answer. The answer is to make sure > nobody ever sets NOFAIL in situations where the allocation can fail > and there is no way forward. > > A BUG_ON() will quite likely just make things worse. You're better off > with a WARN_ON() and letting the caller just oops. > > Honestly, I'm perfectly fine with just removing that stupid useless > flag entirely. The flag goes back to 2003 and was introduced in > 2.5.69, and was meant to be for very particular uses that otherwise > just looped waiting for memory. > > Back in 2.5.69, there was exactly one user: the jbd journal code, that > did a buffer head allocation with GFP_NOFAIL. By 2.6.0 that had > expanded by another user in XFS, and even that one had a comment > saying that it needed to be narrowed down. And in fact, by the 2.6.12 > release, that XFS use had been removed, but the jbd journal had grown > another jbd_kmalloc case for transaction data. So at the beginning of > the git archives, we had exactly *one* user (with two places). > > *THAT* is the kind of use that the flag was meant for: small > allocations required to make forward progress in writeout during > memory pressure. > > It has then expanded and is now a problem. The cases using GFP_NOFAIL > for things like vmalloc() - which is by definition not a small > allocation - should be just removed as outright bugs. One potential approach could be to rename GFP_NOFAIL to GFP_NOFAIL_FOR_SMALL_ALLOC, specifically for smaller allocations, and to clear this flag for larger allocations. However, the challenge lies in determining what constitutes a 'small' allocation. > > Note that we had this comment back in 2010: > > * __GFP_NOFAIL: The VM implementation _must_ retry infinitely: the calle= r > * cannot handle allocation failures. This modifier is deprecated and no= new > * users should be added. > > and then it was softened in 2015 to the current > > * __GFP_NOFAIL: The VM implementation _must_ retry infinitely: the calle= r > * cannot handle allocation failures. New users should be evaluated caref= ully > ... > > and clearly that "evaluated carefully" actually never happened, so the > new comment is just garbage. > > I wonder how many modern users of GFP_NOFAIL are simply due to > over-eager allocation failure injection testing, and then people added > GFP_NOFAIL just because it shut up the mindless random allocation > failures. > > I mean, we have a __GFP_NOFAIL in rhashtable_init() - which can > actually return an error just fine, but there was this crazy worry > about the IPC layer initialization failing: > > https://lore.kernel.org/all/20180523172500.anfvmjtumww65ief@linux-n805= / > > Things like that, where people just added mindless "theoretical > concerns" issues, or possibly had some error injection module that > inserted impossible failures. > > I do NOT want those things to become BUG_ON()'s. It's better to just > return NULL with a "bogus GFP_NOFAIL" warning, and have the oops > happen in the actual bad place that did an invalid allocation. > > Because the blame should go *there*, and it should not even remotely > look like "oh, the MM code failed". No. The caller was garbage. > > So no. No MM BUG_ON code. > > Linus > -- Regards Yafang