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 5A920C10F1A for ; Thu, 9 May 2024 03:09:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E53136B008C; Wed, 8 May 2024 23:09:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DDCC86B0093; Wed, 8 May 2024 23:09:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C7CB86B0095; Wed, 8 May 2024 23:09:22 -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 A72E96B008C for ; Wed, 8 May 2024 23:09:22 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 2C468120D49 for ; Thu, 9 May 2024 03:09:22 +0000 (UTC) X-FDA: 82097376564.29.4B07156 Received: from mail-vk1-f171.google.com (mail-vk1-f171.google.com [209.85.221.171]) by imf01.hostedemail.com (Postfix) with ESMTP id 6116D40009 for ; Thu, 9 May 2024 03:09:20 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=cWwvXAiA; spf=pass (imf01.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.171 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1715224160; a=rsa-sha256; cv=none; b=78gir35EHBASU2Xtzky9mPDYoPLCKK7s5DSY2f4UPKGKESZhhY7GqIjt5HflYlYapMzfIh UdSDPID+Q6qMkc9r3tCoUKbuVsfZlKmkDIsAVCC2JkJUAXBYcv0cAUFvU7WX56YepnVL78 sAGav2iFMtidxO0841bZ46KwiyxgfXA= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=cWwvXAiA; spf=pass (imf01.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.171 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1715224160; 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=a8ZuJSLHVy2XSy/nvCPgDIRow5iMVn6G7A/A6pwXhWw=; b=i3Xtrc/ZxFoYJbC2lWRZEJseSBIwntgshwobXZ2PaDJurWsfJfQk31eYU9Hu+KkJ01h3Yx WXtSoIGXqSO9mQ25KSLelXt8D2P5lehyi10CqcXDGl8ByVCmgdGnWjgkKCfWx5VWBBFAad Cw0RmLjlLEDK8iOOYO/rjKwS8RxRkGs= Received: by mail-vk1-f171.google.com with SMTP id 71dfb90a1353d-4df29c84f60so259869e0c.1 for ; Wed, 08 May 2024 20:09:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1715224159; x=1715828959; 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=a8ZuJSLHVy2XSy/nvCPgDIRow5iMVn6G7A/A6pwXhWw=; b=cWwvXAiAdIlJvki7rAi4Lz2ORbj63oEkvcBxTy0I4rfcWcMVwgAboinFMsuF0RlcsL kUHm6NfSCI1L4h3A3WnKtYeShWA2W/QOCyluEvQxZ87eBKDd9WwA9HmHyr4lp2CMrJby PRdYpVaxuzjbVm6epao+B78NB8vay4wTQqZHROvsd7Rl/Pr1mKFcdoMP8sNiZV1oKkSD TRFiycrp+Mavn1pYcQ/xOKyBhokrclCEafOLHxNyz5vc25RLh1UP2NH7AW/njGSfWkLo 3+1menlICsd+Y6bthft9Wg1quE9NODXsMxTt2jzvis0U2YHxc8g1xEUKD/vduN6VZeT9 pHHg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715224159; x=1715828959; 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=a8ZuJSLHVy2XSy/nvCPgDIRow5iMVn6G7A/A6pwXhWw=; b=eyqh76oTqJCxOdrOtzdQ2mOw2Yl08nCY/qZI24/m9IyJoAsh2KgWlSH//lQeKMqqYV 20tkDuwD3RzNlVhZZPZfdTsfH9eU1xeHp5v+MkNofIKru3RdzQIBjS1hE83kj7TRHENw knPTrk1u11XN6e+p9huAzS6dyrWQ9r3kjgQVHK6X93Wt6O1zEAx29uGST2+RKKWA57KV cMullW/fRKTSKk4uYJ8IBQFlwfxnO5T0Xup5Ok4oh5Fp9kIwWx3U79vPGEYX64stfmb/ 9jSB7qRUmfOKuZBqLMeKRm9YIy3jA3SHoJTMSrUmS8wNPHVRaFqHAWT0Ixrj2Eulb4Ba bTTQ== X-Forwarded-Encrypted: i=1; AJvYcCVujDVIK5PK7JY1UDEz3ltL/Dcs39aUizlhKBLtQbVhJa0Txks+qw902/hvCpNdFtbnMwMaK1+H/+mbbnB4NxGdPus= X-Gm-Message-State: AOJu0Ywo8QWcwf4c3S6oy4bIPq1zJuYCC/vxXz59SIds45cgpOuJzyVm bZdVOUxI5KBT/usvXOHhUODue7S2l8zh6zeBxX38Ki9V1inAHaaytX8Lu/tAK/7sp1TGsZUPtiz pEWtin2jYMSF4zH1nul6hpdGbkpg= X-Google-Smtp-Source: AGHT+IHRZyXY+v7E9ZZWepqn/PVV+z+GqkL5ZnKAwS01Bth0M8FbhZovpaubSh7sb6BiNkrZz7IZGylCy/46KttPpxA= X-Received: by 2002:a05:6122:917:b0:4da:bae9:4449 with SMTP id 71dfb90a1353d-4df78f09a5fmr1344260e0c.4.1715224159442; Wed, 08 May 2024 20:09:19 -0700 (PDT) MIME-Version: 1.0 References: <20240508125808.28882-1-hailong.liu@oppo.com> <20d782ad-c059-4029-9c75-0ef278c98d81@linux.alibaba.com> In-Reply-To: <20d782ad-c059-4029-9c75-0ef278c98d81@linux.alibaba.com> From: Barry Song <21cnbao@gmail.com> Date: Thu, 9 May 2024 15:09:07 +1200 Message-ID: Subject: Re: [RFC PATCH] mm/vmalloc: fix vmalloc which may return null if called with __GFP_NOFAIL To: Gao Xiang Cc: hailong.liu@oppo.com, akpm@linux-foundation.org, urezki@gmail.com, hch@infradead.org, lstoakes@gmail.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, xiang@kernel.org, chao@kernel.org, Oven Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: b5r15ys48ob4iee8ihkrzcyy9scj8tr1 X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 6116D40009 X-HE-Tag: 1715224160-524262 X-HE-Meta: U2FsdGVkX18B1TuQlykdcBNXuLodrAiJ8E3Zx+7MAPSpKfyByvciXx6BMZs/qoztMSUCczzpycnCvz+5FnO85AMQtBCGXg7IZ2SlgFYyQ+81soFaUqgLCKzXxor3XbliNStXgbxvT0SqBQiExdLvHyxnLEotgehe3StKtF9mgHSc2V46NBeDM7nRqpABePKkSEs5bpkSMiKNRCXqaPbknKdTK9EjfwKcahCYtkwdv+6JNsR6Tuj3AkqkLL+m3FUlrCgYQlDeQoh+8VgvmvjvF51XW9m94VVgBew3vrRIEl1t/k8U6DB4/lL/zk2++qAC8Glj1z1JSBLENkWjlgsByS154MSlsQQzCAH+7TXq3kVBlPMq1oBT3BASL2tQ09d30YpVR3MxpRh8IRcbvP/jyH/qQQva7ZIIyy3XJv7XNjmeP/yJtu7xGngrVdhQncozQ1maVDB3GubnfC0ngLmq+9GxRfChBeyfj3DilLAne/qf+27HuDN5T0/395kgOh7kJUI1emjukwuy12zc619rpIomiZGsx+c18AnQ6T79UeRDcIs+VhzY52C814wP4DiJOjMkERDnu+txAEgwcOHeWyefySFAB2ULTQtIeVbicW5/9jFHpHloT2IyC3+FCcgLeSUvmNUYJrREgSBEPoLr5GMbiHS7ajqyqEpqPrMM5DVXYkw4JKg0qXlnIksPOcyphn97uuB2FhMAireGYBi2Bq2s+gJ927QyolOb818HKL9wCHDrrqFS/P9ubueuFx95fyTXhvIPTaf+nALTdj0JA0UWV7xlOletjIcZjgjPP8GZa0DMwCpb/Xih83DRz2A+Vgyl+hXklkoTR4xwM8rfoTTKORoSHrz0y6bovqiD/W95CO7Ewy3Ge1dejMcZ/Pl7HW+m1b9rWuF7ZITqu6r7b4/iYlUB5srscK6cx5qGhmtsm4P8uRhNjbl9pdPgmhIcXRSh8sssCiFHH3MvIaV 07Ypc/jP pCazgjEF3CQnkpX1FVVIX2WLpShremVwu2R9+QvjOLyNM2MYnRdU8R1oVizgkd85EujXqH6nPmi9FPDqVI86Vn4UbuTKRuYcn8IFyc/J0YyZj1/yA1B3tXePiwQZ+yAGadPmIdST6HVR7Yg4k1wsR12pix15Jqs4/Z8JxSfkNPkTcQj/nIckX5RzHJVXQb0i6KmoQOoZyIyPoew4Std0gKO8H8hHcQmzbqEHu0gLnbkKJCYJ3+c5m4vAOW2snLE1LxJH25jjd4to2TUorgjRxbNoZ/iHwGVebNBcbV5Dz86pXqo08n1EFwBa4eexBNUMPiBQEqgkG1rp+QnrlA7+uflEVhYa2TX3bonXbAtMWhWdWraro01nCXdQLLMWuG6AiBj+wX+WAjausASIwFxvwLGZwbFNwH+sC8AIMYV/LMexkttHyEhEZXag0Q6UjCrQquD4c/hHg3nXzpCmx6DJ7RuuOCWLOZ8/B/2MkNC18ZmO7lFlzgVswr+nzdptZtE+mFAlKaVfaoVQtEnGSdn0rCXWG7Ksw43s5vImf 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, May 9, 2024 at 2:39=E2=80=AFPM Gao Xiang wrote: > > Hi, > > On 2024/5/9 10:20, Barry Song wrote: > > On Thu, May 9, 2024 at 12:58=E2=80=AFAM wrote: > >> > >> From: "Hailong.Liu" > >> > >> Commit a421ef303008 ("mm: allow !GFP_KERNEL allocations for kvmalloc") > >> includes support for __GFP_NOFAIL, but it presents a conflict with > >> commit dd544141b9eb ("vmalloc: back off when the current task is > >> OOM-killed"). A possible scenario is as belows: > >> > >> process-a > >> kvcalloc(n, m, GFP_KERNEL | __GFP_NOFAIL) > >> __vmalloc_node_range() > >> __vmalloc_area_node() > >> vm_area_alloc_pages() > >> --> oom-killer send SIGKILL to process-a > >> if (fatal_signal_pending(current)) break; > >> --> return NULL; > >> > >> to fix this, do not check fatal_signal_pending() in vm_area_alloc_page= s() > >> if __GFP_NOFAIL set. > >> > >> Reported-by: Oven > >> Signed-off-by: Hailong.Liu > >> --- > >> mm/vmalloc.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/mm/vmalloc.c b/mm/vmalloc.c > >> index 6641be0ca80b..2f359d08bf8d 100644 > >> --- a/mm/vmalloc.c > >> +++ b/mm/vmalloc.c > >> @@ -3560,7 +3560,7 @@ vm_area_alloc_pages(gfp_t gfp, int nid, > >> > >> /* High-order pages or fallback path if "bulk" fails. */ > >> while (nr_allocated < nr_pages) { > >> - if (fatal_signal_pending(current)) > >> + if (!(gfp & __GFP_NOFAIL) && fatal_signal_pending(curr= ent)) > >> break; > > > > why not !nofail ? > > > > This seems a correct fix, but it undermines the assumption made in > > commit dd544141b9eb > > ("vmalloc: back off when the current task is OOM-killed") > > > > " > > This may trigger some hidden problems, when caller does not handle > > vmalloc failures, or when rollaback after failed vmalloc calls own > > vmallocs inside. However all of these scenarios are incorrect: vm= alloc > > does not guarantee successful allocation, it has never been called= with > > __GFP_NOFAIL and threfore either should not be used for any rollba= cks or > > should handle such errors correctly and not lead to critical failu= res. > > " > > > > If a significant kvmalloc operation is performed with the NOFAIL flag, = it risks > > reverting the fix intended to address the OOM-killer issue in commit > > dd544141b9eb. > > Should we indeed permit the NOFAIL flag for large kvmalloc allocations? > > Just from my perspective, I don't really care about kmalloc, vmalloc > or kvmalloc (__GFP_NOFAIL). I even don't care if it returns three > order-0 pages or a high-order page. I just would like to need a > virtual consecutive buffer (even it works slowly.) with __GFP_NOFAIL. > > Because in some cases, writing fallback code may be tough and hard to > test if such fallback path is correct since it only triggers in extreme > workloads, and even such buffers are just used in a very short lifetime. > Also see other FS discussion of __GFP_NOFAIL, e.g. > https://lore.kernel.org/all/ZcUQfzfQ9R8X0s47@tiehlicka/ > > In the worst cases, it usually just needs < 5 order-0 pages (for many > cases it only needs one page), but with kmalloc it will trigger WARN > if it occurs to > order-1 allocation. as I mentioned before. > > With my limited understanding I don't see why it could any problem with > kvmalloc(__GFP_NOFAIL) since it has no difference of kmalloc(GFP_NOFAIL) > with order-0 allocation. I completely understand that you're not concerned about the origin of the memory, such as whether it's organized by all zero-order pages. However, in the eve= nt that someone else allocates a large memory, like several megabytes with the NOFAIL flag, commit dd544141b9eb aims to halt the allocation before success if the process being allocated is targeted for termination of OOM-killer. With the current patch, we miss the opportunity for early allocation termination. However, if the size of the kvmalloc() is small, as in your case, I believe it should be perfectly fine. but do we have any way to prevent large size allocation = with NOFAIL? > > > Thanks, > Gao XIang Thanks Barry