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 E3A8BCD37B1 for ; Tue, 3 Sep 2024 22:39:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 75EDE8D01EA; Tue, 3 Sep 2024 18:39:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 70E898D01E4; Tue, 3 Sep 2024 18:39:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5AF848D01EA; Tue, 3 Sep 2024 18:39:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 3BECD8D01E4 for ; Tue, 3 Sep 2024 18:39:51 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id C4487140B1E for ; Tue, 3 Sep 2024 22:39:50 +0000 (UTC) X-FDA: 82524895740.22.58DFBF8 Received: from mail-pl1-f172.google.com (mail-pl1-f172.google.com [209.85.214.172]) by imf16.hostedemail.com (Postfix) with ESMTP id D816D180011 for ; Tue, 3 Sep 2024 22:39:48 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=TSR7p98P; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf16.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.214.172 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1725403082; 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=lJzEDZ6Y41Cwm+nItw4ApEDZvLrm6Zf0G+Ovt54oSKM=; b=kIjStTR5s+6ud/Nf+mbmcIRudvlfWauc0NtSmNEWkXgGdoHkNQXjMdlCmILD7jF4xlA7lR /pS9WJbH+Ta1YeNAirYhWwlRU7HkfvEjuLiERNh+8beejGi7gUrIQ4HzChnLl2WmCQY18y plc+qKBZvYePUXf3+v2sPoC/l5Hw4QA= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725403082; a=rsa-sha256; cv=none; b=CqRaHdibBACuJbHsICfHunv8OYysSA0m/HTa5lOJrVAMZcl2UyesIR4DQyx+QRzlwy5i5f CRp6vssNnSp8KgaYrp1JeCVq//umwDwf2QrxkiMKjlAQWs7hgmo4/3+/zGd/BQESXaKUWg BvYvKc3HMI979IUhfKer0QlvcKYABQo= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=TSR7p98P; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf16.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.214.172 as permitted sender) smtp.mailfrom=21cnbao@gmail.com Received: by mail-pl1-f172.google.com with SMTP id d9443c01a7336-2053616fa36so37567805ad.0 for ; Tue, 03 Sep 2024 15:39:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1725403187; x=1726007987; darn=kvack.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=lJzEDZ6Y41Cwm+nItw4ApEDZvLrm6Zf0G+Ovt54oSKM=; b=TSR7p98PMiilwJZ2OUzSd3P+4TGkjoJ7nrl6Kw6yGDXJDPkSnCXfmt4A8psMkBDMXA yvBz3oXilFOrkiZdnGjHAtguYOuU/x5ICGVeIXofc4SZO6+1r4xYgpvXPwVDWblYc24l 0T/skJE0wTB6gm3ZBJnSdP1hT6z+zL1gNQof8JNDl/560B9lgnyFVUguFz1bk7Ht1vGv 61NbtJ22q9lsJCHpDvy1EvIHLmVvQYx2dH0MAYw7L52VwgH9S0on1j7VGW0g2KSSPN/O Xbdnikaby0z8uCLLHuDakQLglLbCSqGpdx0ct0bZBHaKDeO03rgX1wEFmfXj/0Hj0boL qiIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725403187; x=1726007987; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=lJzEDZ6Y41Cwm+nItw4ApEDZvLrm6Zf0G+Ovt54oSKM=; b=jXDKSoz36exaIhLWjbsT0aaNa5ogde3cEJUab9ah/Mj3w/l+ZfZO5eeUr+LomN7KZ7 3/u1/YDu1GeW8JEzUEAPkKQdSw55buULcebzj9Vmph49Cwj3vkcuAGKdZcB0fTRKyUOA nFB8VkU0csJGNo1paPEXsJS9T2H1bKf3n43SQIv3a0QimlutV0WmD5b2+pK+9HpF81z2 Y6tz/pUD4gV06pxkGQ/htXMWW/uq9jUDtWoAuqh2vKQJxFkx7HEbUnrWshO+NNNPEoOQ ey+YR/u7OhT/Akc2k0aOlHQYmYnFcKVctkH8Kpj15WR/nFbw/IxZnWReeB0DwRcQPNqM qUKQ== X-Forwarded-Encrypted: i=1; AJvYcCVlZvd9+rZTJ/cBIDMd4PWQ1ohFiZNoYqkNl+4G6ttsCq+l2zaTzgD/d/kmqozUIYUeNQxYwJNj/w==@kvack.org X-Gm-Message-State: AOJu0Yytz5NilQkcOnz/3K133PyYbsXud50qW3NBPSO2qhTWS6HAsXbW 5dzWkS36/2+K5+fWzvGGkRgrz2CCM8/8EKYiw7jzbnqltSlsD38F X-Google-Smtp-Source: AGHT+IF4JZ9uttpCEjhGk+KRVirtmc6UBRZKXBbOBrpMJWbwoF0ZbezJXFzPRsbM0ocaAmPCoeH3ig== X-Received: by 2002:a17:903:2b04:b0:205:51ee:b81e with SMTP id d9443c01a7336-20551eebb28mr131826195ad.15.1725403187460; Tue, 03 Sep 2024 15:39:47 -0700 (PDT) Received: from Barrys-MBP.hub ([2407:7000:8942:5500:3dfb:fdbe:2749:2520]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-206aea55789sm3007825ad.217.2024.09.03.15.39.40 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Tue, 03 Sep 2024 15:39:47 -0700 (PDT) From: Barry Song <21cnbao@gmail.com> To: mhocko@suse.com Cc: 21cnbao@gmail.com, 42.hyeyoo@gmail.com, akpm@linux-foundation.org, cl@linux.com, david@redhat.com, hailong.liu@oppo.com, hch@infradead.org, iamjoonsoo.kim@lge.com, laoar.shao@gmail.com, linux-mm@kvack.org, penberg@kernel.org, rientjes@google.com, roman.gushchin@linux.dev, torvalds@linux-foundation.org, urezki@gmail.com, v-songbaohua@oppo.com, vbabka@suse.cz, virtualization@lists.linux.dev Subject: Re: [PATCH v4 3/3] mm: warn about illegal __GFP_NOFAIL usage in a more appropriate location and manner Date: Wed, 4 Sep 2024 10:39:35 +1200 Message-Id: <20240903223935.1697-1-21cnbao@gmail.com> X-Mailer: git-send-email 2.39.3 (Apple Git-146) In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: D816D180011 X-Stat-Signature: 71yk8oatz9dnh4j1odge4o1yw3ird1ch X-Rspam-User: X-HE-Tag: 1725403188-153681 X-HE-Meta: U2FsdGVkX190sVAstHhUcBmgFT7pE2fljXCKQQtO2hkK7vswvLmAwgeWqrTuWfvh09TsqdfUHlYMxJ4CkBduM5JiBijbjMr8+mGB0vV2f1iq9JbzR7tTsERRc0ftBVjV9SMFTOKkwWHJK0w/DdlFua7h0Di6FYf31c/m9pPaUDnts7I4aQ7a3pGUAjiPWPXEKM6mqJBqZqgyX2lqNK+4p9lcNluO/FupjsLFRIrBntRzsBROAf6T3MbuNKd5rwRn/wdWifR8sajGDYykJfN7XnTgjuBIE7xZg0eI/2plcHcLXiM3p7dyofF8jCdrjk/8P7Rlm9mAJfNnEWGux9UYU/nDg+YLGLknpHRfgBRyYB9XgYbdD8zWiwZP7inurpGKprlZJMGHHagNdok6vG22N8cY+3uM1Id8l324GDh4e18WvmWmPvc8DNypOagrtpx2Ovb+t2qYpo2vsk4IDLX7jFfvBHmQsMC0bFv9qeYm7nB+xXULthXJw9RIUdUCtee96opKlnm/wjuIWg6dVRjCIxxXDxItKpcLngeEdEdWRbilUy2ATANXAkP5Q4sNi4u/SNNU+H8zxj38Mxjf4MfjEs4Phql58twHEK10nN6vzoa78gYcyEJa+w5amPcO3z6kV4v2NoPaxRjbcf3Ohp2LLMs6QGZQPUVvuf0MK3IDgSASzE49T8W8UxfLSFto7edS1FSgNnKPUmsNo2UpeRnVzf+WjTioc8Bgk5mkzvSDCaqVMjtNVEOUw8VZ086jCf1NXalIOxQgA3W59+Z2BfM0iJR/wGO3gXXDDxI6zR6gJRhYDemUmUjyzypm88dqpsC1I/QLpdTXSZx4F4Aq9i9v1/cHRI5x2APdCZ7FvcDM0Iz4h3Fj1KIL+ffbjslyDAvaX6GGF0yiNaMwx/jZmnDtCKrtpNuo2ch1bb54KkuWUcTeLkAOBTZfTTzTVRweJWpH2S6g16NoLdvHb1M2qDD ocDnZqrw UFhHo4IASjj0Dp2QL5g79qLRYrsvcbF8oVfNLH+m4WmW/x/+sSM8viyRrMfNpf0v6dcz5zlfc5E8TTuABBjFE32nRAqDOlsApafng9L4K5Es+nWZIeAkIXww47oeIroZStRgaKVqd+lr3hFIIgt7PckrOHkGRNb9YpT71Yibhs/kAPinOfLIXFVX1ZdZerSwcFBSMwf116gllrT65vG+2Xz/T41H8THf1UabMOghnK6jTb/vz4OSymfec8QOWUP2T3vuqvZpLZ8lWg3PONNjTcwFRJD2HtFMZY1B9FDNuRnM/AmlAKNgwKK9p0G2OSeh+Fzzy7/8ncsOPq2FkN0Sg0qj0bHou96PHaOEUsBoqZpOv6V1vosy1cG9aZ08kjSrDMl629DoEaYtOX/eDr2bjK5vp187Fdt90cxQFm90V72+0TIxfJAU+bGduVux2JHViD9oBVr8c4RoJrNk8meHg2AAJNxCBb/mjM35iXkH9IxVdy1qHVblYS7r2Rsv/ZlNTg8WWYFr9+s0/mPrVxlF45N5Bys2wEeep7mnMbiumAg3W1/EQNk7wb2bdQp9TfZTg0cJFByU2jNl/aVIrfvFlTapth/zM/q2gBKSpbDj8xtrBjrBMq1v6pem0BeMspdj4cLRj5WSsCk1obN0HFo1Ly+ZHUw== 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 Mon, Sep 2, 2024 at 7:58 PM Michal Hocko wrote: > > On Sat 31-08-24 08:28:23, Barry Song wrote: > > From: Barry Song > > > > Three points for this change: > > > > 1. We should consolidate all warnings in one place. Currently, the > >    order > 1 warning is in the hotpath, while others are in less > >    likely scenarios. Moving all warnings to the slowpath will reduce > >    the overhead for order > 1 and increase the visibility of other > >    warnings. > > > > 2. We currently have two warnings for order: one for order > 1 in > >    the hotpath and another for order > costly_order in the laziest > >    path. I suggest standardizing on order > 1 since it’s been in > >    use for a long time. > > > > 3. We don't need to check for __GFP_NOWARN in this case. __GFP_NOWARN > >    is meant to suppress allocation failure reports, but here we're > >    dealing with bug detection, not allocation failures. So replace > >    WARN_ON_ONCE_GFP by WARN_ON_ONCE. > > > > Suggested-by: Vlastimil Babka > > Signed-off-by: Barry Song > > Acked-by: Michal Hocko > > Updating the doc about order > 1 sounds like it would still fall into > the scope of this patch. I don not think we absolutely have to document > each unsupported gfp flags combination for GFP_NOFAIL but the order is a > good addition with a note that kvmalloc should be used instead in such a > case. Hi Andrew, If there are no objections from Michal and David, could you please squash the following: >From fc7a2a49e8d0811d706d13d2080393274f316806 Mon Sep 17 00:00:00 2001 From: Barry Song Date: Wed, 4 Sep 2024 10:26:19 +1200 Subject: [PATCH] mm: also update the doc for __GFP_NOFAIL with order > 1 Obviously we only support order <= 1 __GFP_NOFAIL allocation and if someone wants larger memory, they should consider using kvmalloc() instead. Suggested-by: David Hildenbrand Suggested-by: Michal Hocko Signed-off-by: Barry Song --- include/linux/gfp_types.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/linux/gfp_types.h b/include/linux/gfp_types.h index 4a1fa7706b0c..65db9349f905 100644 --- a/include/linux/gfp_types.h +++ b/include/linux/gfp_types.h @@ -253,7 +253,8 @@ enum { * used only when there is no reasonable failure policy) but it is * definitely preferable to use the flag rather than opencode endless * loop around allocator. - * Using this flag for costly allocations is _highly_ discouraged. + * Allocating pages from the buddy with __GFP_NOFAIL and order > 1 is + * not supported. Please consider using kvmalloc() instead. */ #define __GFP_IO ((__force gfp_t)___GFP_IO) #define __GFP_FS ((__force gfp_t)___GFP_FS) -- 2.34.1 > > > --- > >  mm/page_alloc.c | 50 ++++++++++++++++++++++++------------------------- > >  1 file changed, 25 insertions(+), 25 deletions(-) > > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > index c81ee5662cc7..e790b4227322 100644 > > --- a/mm/page_alloc.c > > +++ b/mm/page_alloc.c > > @@ -3033,12 +3033,6 @@ struct page *rmqueue(struct zone *preferred_zone, > >  { > >       struct page *page; > > > > -     /* > > -      * We most definitely don't want callers attempting to > > -      * allocate greater than order-1 page units with __GFP_NOFAIL. > > -      */ > > -     WARN_ON_ONCE((gfp_flags & __GFP_NOFAIL) && (order > 1)); > > - > >       if (likely(pcp_allowed_order(order))) { > >               page = rmqueue_pcplist(preferred_zone, zone, order, > >                                      migratetype, alloc_flags); > > @@ -4175,6 +4169,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order, > >  { > >       bool can_direct_reclaim = gfp_mask & __GFP_DIRECT_RECLAIM; > >       bool can_compact = gfp_compaction_allowed(gfp_mask); > > +     bool nofail = gfp_mask & __GFP_NOFAIL; > >       const bool costly_order = order > PAGE_ALLOC_COSTLY_ORDER; > >       struct page *page = NULL; > >       unsigned int alloc_flags; > > @@ -4187,6 +4182,25 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order, > >       unsigned int zonelist_iter_cookie; > >       int reserve_flags; > > > > +     if (unlikely(nofail)) { > > +             /* > > +              * We most definitely don't want callers attempting to > > +              * allocate greater than order-1 page units with __GFP_NOFAIL. > > +              */ > > +             WARN_ON_ONCE(order > 1); > > +             /* > > +              * Also we don't support __GFP_NOFAIL without __GFP_DIRECT_RECLAIM, > > +              * otherwise, we may result in lockup. > > +              */ > > +             WARN_ON_ONCE(!can_direct_reclaim); > > +             /* > > +              * PF_MEMALLOC request from this context is rather bizarre > > +              * because we cannot reclaim anything and only can loop waiting > > +              * for somebody to do a work for us. > > +              */ > > +             WARN_ON_ONCE(current->flags & PF_MEMALLOC); > > +     } > > + > >  restart: > >       compaction_retries = 0; > >       no_progress_loops = 0; > > @@ -4404,29 +4418,15 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order, > >        * Make sure that __GFP_NOFAIL request doesn't leak out and make sure > >        * we always retry > >        */ > > -     if (gfp_mask & __GFP_NOFAIL) { > > +     if (unlikely(nofail)) { > >               /* > > -              * All existing users of the __GFP_NOFAIL are blockable, so warn > > -              * of any new users that actually require GFP_NOWAIT > > +              * Lacking direct_reclaim we can't do anything to reclaim memory, > > +              * we disregard these unreasonable nofail requests and still > > +              * return NULL > >                */ > > -             if (WARN_ON_ONCE_GFP(!can_direct_reclaim, gfp_mask)) > > +             if (!can_direct_reclaim) > >                       goto fail; > > > > -             /* > > -              * PF_MEMALLOC request from this context is rather bizarre > > -              * because we cannot reclaim anything and only can loop waiting > > -              * for somebody to do a work for us > > -              */ > > -             WARN_ON_ONCE_GFP(current->flags & PF_MEMALLOC, gfp_mask); > > - > > -             /* > > -              * non failing costly orders are a hard requirement which we > > -              * are not prepared for much so let's warn about these users > > -              * so that we can identify them and convert them to something > > -              * else. > > -              */ > > -             WARN_ON_ONCE_GFP(costly_order, gfp_mask); > > - > >               /* > >                * Help non-failing allocations by giving some access to memory > >                * reserves normally used for high priority non-blocking > > -- > > 2.34.1 > > -- > Michal Hocko > SUSE Labs Thanks Barry