Re: Question on ___GFP_NOLOCKDEP - Was: Re: [PATCH 1/1] Remove hardcoding of ___GFP_xxx bitmasks

[Vlastimil Babka <vbabka@suse.cz>]

On 04/27/2017 03:35 PM, Michal Hocko wrote:
> On Thu 27-04-17 15:16:47, Igor Stoppa wrote:
>> On 26/04/17 18:29, Igor Stoppa wrote:
>>
>>> On 26/04/17 17:47, Michal Hocko wrote:
>>
>> [...]
>>
>>>> Also the current mm tree has ___GFP_NOLOCKDEP which is not addressed
>>>> here so I suspect you have based your change on the Linus tree.
>>
>>> I used your tree from kernel.org
>>
>> I found it, I was using master, instead of auto-latest (is it correct?)
> 
> yes
> 
>> But now I see something that I do not understand (apologies if I'm
>> asking something obvious).
>>
>> First there is:
>>
>> [...]
>> #define ___GFP_WRITE		0x800000u
>> #define ___GFP_KSWAPD_RECLAIM	0x1000000u
>> #ifdef CONFIG_LOCKDEP
>> #define ___GFP_NOLOCKDEP	0x4000000u
>> #else
>> #define ___GFP_NOLOCKDEP	0
>> #endif
>>
>> Then:
>>
>> /* Room for N __GFP_FOO bits */
>> #define __GFP_BITS_SHIFT (25 + IS_ENABLED(CONFIG_LOCKDEP))
>>
>>
>>
>> Shouldn't it be either:
>> ___GFP_NOLOCKDEP	0x2000000u
> 
> Yes it should. At the time when this patch was written this value was
> used. Later I've removed __GFP_OTHER by 41b6167e8f74 ("mm: get rid of
> __GFP_OTHER_NODE") and forgot to refresh this one. Thanks for noticing
> this.
> 
> Andrew, could you fold the following in please?
> ---
> From 8dc9c917af215f659bb990fa48ae7b4753027c19 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Thu, 27 Apr 2017 15:28:10 +0200
> Subject: [PATCH] lockdep-allow-to-disable-reclaim-lockup-detection-fix
> 
> Igor Stoppa has noticed that __GFP_NOLOCKDEP can use a lower bit. At the
> time lockdep-allow-to-disable-reclaim-lockup-detection was written we
> still had __GFP_OTHER_NODE but I have removed it in 41b6167e8f74 ("mm:
> get rid of __GFP_OTHER_NODE") and forgot to lower the bit value.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>

Ping, I have noticed (at least in the mmotm-2017-05-08-16-30 git tag)
there's still 0x4000000u ?

> ---
>  include/linux/gfp.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 2b1a44f5bdb6..a89d37e8b387 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -41,7 +41,7 @@ struct vm_area_struct;
>  #define ___GFP_WRITE		0x800000u
>  #define ___GFP_KSWAPD_RECLAIM	0x1000000u
>  #ifdef CONFIG_LOCKDEP
> -#define ___GFP_NOLOCKDEP	0x4000000u
> +#define ___GFP_NOLOCKDEP	0x2000000u
>  #else
>  #define ___GFP_NOLOCKDEP	0
>  #endif
>