Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <ebb3c657-c914-7d71-c4c9-6e8fe2d2e0c0@bytedance.com>
Date:   Wed, 11 May 2022 10:36:18 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.9.0
Subject: Re: [PATCH 1/2] mm: fix missing handler for __GFP_NOWARN
Content-Language: en-US
To:     Andrew Morton <akpm@linux-foundation.org>
Cc:     akinobu.mita@gmail.com, vbabka@suse.cz, gregkh@linuxfoundation.org,
        jirislaby@kernel.org, rostedt@goodmis.org,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org
References: <20220510113809.80626-1-zhengqi.arch@bytedance.com>
 <20220510115922.350a496ca8b91686c1758282@linux-foundation.org>
 <c44769b3-a132-63a5-fd40-4d483497dff2@bytedance.com>
 <20220510193215.14ed7e3fb70857738e10c0a2@linux-foundation.org>
From:   Qi Zheng <zhengqi.arch@bytedance.com>
In-Reply-To: <20220510193215.14ed7e3fb70857738e10c0a2@linux-foundation.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 2022/5/11 10:32 AM, Andrew Morton wrote:
> On Wed, 11 May 2022 10:19:48 +0800 Qi Zheng <zhengqi.arch@bytedance.com> wrote:
> 
>>
>> ,,,
>>>> --- a/mm/internal.h
>>>> +++ b/mm/internal.h
>>>> @@ -35,6 +35,17 @@ struct folio_batch;
>>>>    /* Do not use these with a slab allocator */
>>>>    #define GFP_SLAB_BUG_MASK (__GFP_DMA32|__GFP_HIGHMEM|~__GFP_BITS_MASK)
>>>>    
>>>> +#define WARN_ON_ONCE_GFP(cond, gfp)	({				\
>>>> +	static bool __section(".data.once") __warned;			\
>>>> +	int __ret_warn_once = !!(cond);					\
>>>> +									\
>>>> +	if (unlikely(!(gfp & __GFP_NOWARN) && __ret_warn_once && !__warned)) { \
>>>> +		__warned = true;					\
>>>> +		WARN_ON(1);						\
>>>> +	}								\
>>>> +	unlikely(__ret_warn_once);					\
>>>> +})
>>>
>>> I don't think WARN_ON_ONCE_GFP is a good name for this.  But
>>> WARN_ON_ONCE_IF_NOT_GFP_NOWARN is too long :(
>>>
>>> WARN_ON_ONCE_NOWARN might be better.  No strong opinion here, really.
>>
>> I've thought about WARN_ON_ONCE_NOWARN, but I feel a little weird
>> putting 'WARN' and 'NOWARN' together, how about WARN_ON_ONCE_IF_ALLOWED?
> 
> I dunno.  WARN_ON_ONCE_GFP isn't too bad I suppose.  Add a comment over
> the definition explaining it?

OK, I will add a comment to it.

> 
>>>
>>>> @@ -4902,8 +4906,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>>>>    	 * We also sanity check to catch abuse of atomic reserves being used by
>>>>    	 * callers that are not in atomic context.
>>>>    	 */
>>>> -	if (WARN_ON_ONCE((gfp_mask & (__GFP_ATOMIC|__GFP_DIRECT_RECLAIM)) ==
>>>> -				(__GFP_ATOMIC|__GFP_DIRECT_RECLAIM)))
>>>> +	if (WARN_ON_ONCE_GFP((gfp_mask & (__GFP_ATOMIC|__GFP_DIRECT_RECLAIM)) ==
>>>> +				(__GFP_ATOMIC|__GFP_DIRECT_RECLAIM), gfp_mask))
>>>>    		gfp_mask &= ~__GFP_ATOMIC;
>>>>    
>>>>    retry_cpuset:
>>>
>>> I dropped this hunk - Neil's "mm: discard __GFP_ATOMIC"
>>> (https://lkml.kernel.org/r/163712397076.13692.4727608274002939094@noble.neil.brown.name)
>>> deleted this code.
>>>
>>
>> This series is based on v5.18-rc5, I will rebase it to the latest next
>> branch and check if there are any missing WARN_ON_ONCEs that are not
>> being handled.
> 
> Against git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm branch
> mm-unstable, please.  That ends up in linux-next, with a delay.

OK, will do.

-- 
Thanks,
Qi