Re: [PATCH] slab.h: Avoid using & for logical and of booleans

Re: [PATCH] slab.h: Avoid using & for logical and of booleans

[Andrew Morton]

On Mon,  5 Nov 2018 12:40:00 -0800 Bart Van Assche <bvanassche@acm.org> wrote:

> This patch suppresses the following sparse warning:
> 
> ./include/linux/slab.h:332:43: warning: dubious: x & !y
> 
> ...
>
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -329,7 +329,7 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
>  	 * If an allocation is both __GFP_DMA and __GFP_RECLAIMABLE, return
>  	 * KMALLOC_DMA and effectively ignore __GFP_RECLAIMABLE
>  	 */
> -	return type_dma + (is_reclaimable & !is_dma) * KMALLOC_RECLAIM;
> +	return type_dma + is_reclaimable * !is_dma * KMALLOC_RECLAIM;
>  }
>  
>  /*

I suppose so.

That function seems too clever for its own good :(.  I wonder if these
branch-avoiding tricks are really worthwhile.

Re: [PATCH] slab.h: Avoid using & for logical and of booleans

[Bart Van Assche]

On Mon, 2018-11-05 at 13:13 -0800, Andrew Morton wrote:
> On Mon,  5 Nov 2018 12:40:00 -0800 Bart Van Assche <bvanassche@acm.org> wrote:
> 
> > This patch suppresses the following sparse warning:
> > 
> > ./include/linux/slab.h:332:43: warning: dubious: x & !y
> > 
> > ...
> > 
> > --- a/include/linux/slab.h
> > +++ b/include/linux/slab.h
> > @@ -329,7 +329,7 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
> >  	 * If an allocation is both __GFP_DMA and __GFP_RECLAIMABLE, return
> >  	 * KMALLOC_DMA and effectively ignore __GFP_RECLAIMABLE
> >  	 */
> > -	return type_dma + (is_reclaimable & !is_dma) * KMALLOC_RECLAIM;
> > +	return type_dma + is_reclaimable * !is_dma * KMALLOC_RECLAIM;
> >  }
> >  
> >  /*
> 
> I suppose so.
> 
> That function seems too clever for its own good :(.  I wonder if these
> branch-avoiding tricks are really worthwhile.

>From what I have seen in gcc disassembly it seems to me like gcc uses the
cmov instruction to implement e.g. the ternary operator (?:). So I think none
of the cleverness in kmalloc_type() is really necessary to avoid conditional
branches. I think this function would become much more readable when using a
switch statement or when rewriting it as follows (untested):

 static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
 {
-	int is_dma = 0;
-	int type_dma = 0;
-	int is_reclaimable;
-
-#ifdef CONFIG_ZONE_DMA
-	is_dma = !!(flags & __GFP_DMA);
-	type_dma = is_dma * KMALLOC_DMA;
-#endif
-
-	is_reclaimable = !!(flags & __GFP_RECLAIMABLE);
-
 	/*
 	 * If an allocation is both __GFP_DMA and __GFP_RECLAIMABLE, return
 	 * KMALLOC_DMA and effectively ignore __GFP_RECLAIMABLE
 	 */
-	return type_dma + (is_reclaimable & !is_dma) * KMALLOC_RECLAIM;
+	static const enum kmalloc_cache_type flags_to_type[2][2] = {
+		{ 0,		KMALLOC_RECLAIM },
+		{ KMALLOC_DMA,	KMALLOC_DMA },
+	};
+#ifdef CONFIG_ZONE_DMA
+	bool is_dma = !!(flags & __GFP_DMA);
+#endif
+	bool is_reclaimable = !!(flags & __GFP_RECLAIMABLE);
+
+	return flags_to_type[is_dma][is_reclaimable];
 }

Re: [PATCH] slab.h: Avoid using & for logical and of booleans

[Rasmus Villemoes]

On 2018-11-05 22:48, Bart Van Assche wrote:
> On Mon, 2018-11-05 at 13:13 -0800, Andrew Morton wrote:
>> On Mon,  5 Nov 2018 12:40:00 -0800 Bart Van Assche <bvanassche@acm.org> wrote:
>>
>>> This patch suppresses the following sparse warning:
>>>
>>> ./include/linux/slab.h:332:43: warning: dubious: x & !y
>>>
>>> ...
>>>
>>> --- a/include/linux/slab.h
>>> +++ b/include/linux/slab.h
>>> @@ -329,7 +329,7 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
>>>  	 * If an allocation is both __GFP_DMA and __GFP_RECLAIMABLE, return
>>>  	 * KMALLOC_DMA and effectively ignore __GFP_RECLAIMABLE
>>>  	 */
>>> -	return type_dma + (is_reclaimable & !is_dma) * KMALLOC_RECLAIM;
>>> +	return type_dma + is_reclaimable * !is_dma * KMALLOC_RECLAIM;
>>>  }
>>>  
>>>  /*
>>
>> I suppose so.
>>
>> That function seems too clever for its own good :(.  I wonder if these
>> branch-avoiding tricks are really worthwhile.
> 
> From what I have seen in gcc disassembly it seems to me like gcc uses the
> cmov instruction to implement e.g. the ternary operator (?:). So I think none
> of the cleverness in kmalloc_type() is really necessary to avoid conditional
> branches. I think this function would become much more readable when using a
> switch statement or when rewriting it as follows (untested):
> 
>  static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
>  {
> -	int is_dma = 0;
> -	int type_dma = 0;
> -	int is_reclaimable;
> -
> -#ifdef CONFIG_ZONE_DMA
> -	is_dma = !!(flags & __GFP_DMA);
> -	type_dma = is_dma * KMALLOC_DMA;
> -#endif
> -
> -	is_reclaimable = !!(flags & __GFP_RECLAIMABLE);
> -
>  	/*
>  	 * If an allocation is both __GFP_DMA and __GFP_RECLAIMABLE, return
>  	 * KMALLOC_DMA and effectively ignore __GFP_RECLAIMABLE
>  	 */
> -	return type_dma + (is_reclaimable & !is_dma) * KMALLOC_RECLAIM;
> +	static const enum kmalloc_cache_type flags_to_type[2][2] = {
> +		{ 0,		KMALLOC_RECLAIM },
> +		{ KMALLOC_DMA,	KMALLOC_DMA },
> +	};
> +#ifdef CONFIG_ZONE_DMA
> +	bool is_dma = !!(flags & __GFP_DMA);
> +#endif
> +	bool is_reclaimable = !!(flags & __GFP_RECLAIMABLE);
> +
> +	return flags_to_type[is_dma][is_reclaimable];
>  }
> 

Won't that pessimize the cases where gfp is a constant to actually do
the table lookup, and add 16 bytes to every translation unit?

Another option is to add a fake KMALLOC_DMA_RECLAIM so the
kmalloc_caches[] array has size 4, then assign the same dma
kmalloc_cache pointer to [2][i] and [3][i] (so that costs perhaps a
dozen pointers in .data), and then just compute kmalloc_type() as

((flags & __GFP_RECLAIMABLE) >> someshift) | ((flags & __GFP_DMA) >>
someothershift).

Perhaps one could even shuffle the GFP flags so the two shifts are the same.

Rasmus

Re: [PATCH] slab.h: Avoid using & for logical and of booleans

[Bart Van Assche]

On Mon, 2018-11-05 at 23:14 +0100, Rasmus Villemoes wrote:
> Won't that pessimize the cases where gfp is a constant to actually do
> the table lookup, and add 16 bytes to every translation unit?
> 
> Another option is to add a fake KMALLOC_DMA_RECLAIM so the
> kmalloc_caches[] array has size 4, then assign the same dma
> kmalloc_cache pointer to [2][i] and [3][i] (so that costs perhaps a
> dozen pointers in .data), and then just compute kmalloc_type() as
> 
> ((flags & __GFP_RECLAIMABLE) >> someshift) | ((flags & __GFP_DMA) >>
> someothershift).
> 
> Perhaps one could even shuffle the GFP flags so the two shifts are the same.

How about this version, still untested? My compiler is able to evaluate
the switch expression if the argument is constant.

 static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
 {
-	int is_dma = 0;
-	int type_dma = 0;
-	int is_reclaimable;
+	unsigned int dr = !!(flags & __GFP_RECLAIMABLE);
 
 #ifdef CONFIG_ZONE_DMA
-	is_dma = !!(flags & __GFP_DMA);
-	type_dma = is_dma * KMALLOC_DMA;
+	dr |= !!(flags & __GFP_DMA) << 1;
 #endif
 
-	is_reclaimable = !!(flags & __GFP_RECLAIMABLE);
-
 	/*
 	 * If an allocation is both __GFP_DMA and __GFP_RECLAIMABLE, return
 	 * KMALLOC_DMA and effectively ignore __GFP_RECLAIMABLE
 	 */
-	return type_dma + (is_reclaimable & !is_dma) * KMALLOC_RECLAIM;
+	switch (dr) {
+	default:
+	case 0:
+		return 0;
+	case 1:
+		return KMALLOC_RECLAIM;
+	case 2:
+	case 3:
+		return KMALLOC_DMA;
+	}
 }
 
Bart.

Re: [PATCH] slab.h: Avoid using & for logical and of booleans

[Alexander Duyck]

On Mon, Nov 5, 2018 at 2:41 PM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On Mon, 2018-11-05 at 23:14 +0100, Rasmus Villemoes wrote:
> > Won't that pessimize the cases where gfp is a constant to actually do
> > the table lookup, and add 16 bytes to every translation unit?
> >
> > Another option is to add a fake KMALLOC_DMA_RECLAIM so the
> > kmalloc_caches[] array has size 4, then assign the same dma
> > kmalloc_cache pointer to [2][i] and [3][i] (so that costs perhaps a
> > dozen pointers in .data), and then just compute kmalloc_type() as
> >
> > ((flags & __GFP_RECLAIMABLE) >> someshift) | ((flags & __GFP_DMA) >>
> > someothershift).
> >
> > Perhaps one could even shuffle the GFP flags so the two shifts are the same.
>
> How about this version, still untested? My compiler is able to evaluate
> the switch expression if the argument is constant.
>
>  static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
>  {
> -       int is_dma = 0;
> -       int type_dma = 0;
> -       int is_reclaimable;
> +       unsigned int dr = !!(flags & __GFP_RECLAIMABLE);
>
>  #ifdef CONFIG_ZONE_DMA
> -       is_dma = !!(flags & __GFP_DMA);
> -       type_dma = is_dma * KMALLOC_DMA;
> +       dr |= !!(flags & __GFP_DMA) << 1;
>  #endif
>
> -       is_reclaimable = !!(flags & __GFP_RECLAIMABLE);
> -
>         /*
>          * If an allocation is both __GFP_DMA and __GFP_RECLAIMABLE, return
>          * KMALLOC_DMA and effectively ignore __GFP_RECLAIMABLE
>          */
> -       return type_dma + (is_reclaimable & !is_dma) * KMALLOC_RECLAIM;
> +       switch (dr) {
> +       default:
> +       case 0:
> +               return 0;
> +       case 1:
> +               return KMALLOC_RECLAIM;
> +       case 2:
> +       case 3:
> +               return KMALLOC_DMA;
> +       }
>  }
>
> Bart.

Doesn't this defeat the whole point of the code which I thought was to
avoid conditional jumps and branches? Also why would you bother with
the "dr" value when you could just mask the flags value and switch on
that directly?

Re: [PATCH] slab.h: Avoid using & for logical and of booleans

[Bart Van Assche]

On Mon, 2018-11-05 at 14:48 -0800, Alexander Duyck wrote:
> On Mon, Nov 5, 2018 at 2:41 PM Bart Van Assche <bvanassche@acm.org> wrote:
> > How about this version, still untested? My compiler is able to evaluate
> > the switch expression if the argument is constant.
> > 
> >  static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
> >  {
> > -       int is_dma = 0;
> > -       int type_dma = 0;
> > -       int is_reclaimable;
> > +       unsigned int dr = !!(flags & __GFP_RECLAIMABLE);
> > 
> >  #ifdef CONFIG_ZONE_DMA
> > -       is_dma = !!(flags & __GFP_DMA);
> > -       type_dma = is_dma * KMALLOC_DMA;
> > +       dr |= !!(flags & __GFP_DMA) << 1;
> >  #endif
> > 
> > -       is_reclaimable = !!(flags & __GFP_RECLAIMABLE);
> > -
> >         /*
> >          * If an allocation is both __GFP_DMA and __GFP_RECLAIMABLE, return
> >          * KMALLOC_DMA and effectively ignore __GFP_RECLAIMABLE
> >          */
> > -       return type_dma + (is_reclaimable & !is_dma) * KMALLOC_RECLAIM;
> > +       switch (dr) {
> > +       default:
> > +       case 0:
> > +               return 0;
> > +       case 1:
> > +               return KMALLOC_RECLAIM;
> > +       case 2:
> > +       case 3:
> > +               return KMALLOC_DMA;
> > +       }
> >  }
>
> Doesn't this defeat the whole point of the code which I thought was to
> avoid conditional jumps and branches? Also why would you bother with
> the "dr" value when you could just mask the flags value and switch on
> that directly?

Storing the relevant bits of 'flags' in the 'dr' variable avoids that the
bit selection expressions have to be repeated and allows to use a switch
statement instead of multiple if / else statements.

Most kmalloc() calls pass a constant to the gfp argument. That allows the
compiler to evaluate kmalloc_type() at compile time. So the conditional jumps
and branches only appear when the gfp argument is not a constant. What makes
you think it is important to optimize for that case?

Bart.

Re: [PATCH] slab.h: Avoid using & for logical and of booleans

[Alexander Duyck]

On Mon, Nov 5, 2018 at 4:01 PM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On Mon, 2018-11-05 at 14:48 -0800, Alexander Duyck wrote:
> > On Mon, Nov 5, 2018 at 2:41 PM Bart Van Assche <bvanassche@acm.org> wrote:
> > > How about this version, still untested? My compiler is able to evaluate
> > > the switch expression if the argument is constant.
> > >
> > >  static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
> > >  {
> > > -       int is_dma = 0;
> > > -       int type_dma = 0;
> > > -       int is_reclaimable;
> > > +       unsigned int dr = !!(flags & __GFP_RECLAIMABLE);
> > >
> > >  #ifdef CONFIG_ZONE_DMA
> > > -       is_dma = !!(flags & __GFP_DMA);
> > > -       type_dma = is_dma * KMALLOC_DMA;
> > > +       dr |= !!(flags & __GFP_DMA) << 1;
> > >  #endif
> > >
> > > -       is_reclaimable = !!(flags & __GFP_RECLAIMABLE);
> > > -
> > >         /*
> > >          * If an allocation is both __GFP_DMA and __GFP_RECLAIMABLE, return
> > >          * KMALLOC_DMA and effectively ignore __GFP_RECLAIMABLE
> > >          */
> > > -       return type_dma + (is_reclaimable & !is_dma) * KMALLOC_RECLAIM;
> > > +       switch (dr) {
> > > +       default:
> > > +       case 0:
> > > +               return 0;
> > > +       case 1:
> > > +               return KMALLOC_RECLAIM;
> > > +       case 2:
> > > +       case 3:
> > > +               return KMALLOC_DMA;
> > > +       }
> > >  }
> >
> > Doesn't this defeat the whole point of the code which I thought was to
> > avoid conditional jumps and branches? Also why would you bother with
> > the "dr" value when you could just mask the flags value and switch on
> > that directly?
>
> Storing the relevant bits of 'flags' in the 'dr' variable avoids that the
> bit selection expressions have to be repeated and allows to use a switch
> statement instead of multiple if / else statements.

Really they shouldn't have to be repeated. You essentially have just 3
cases. 0, __GFP_RECLAIMABLE, and the default case.

> Most kmalloc() calls pass a constant to the gfp argument. That allows the
> compiler to evaluate kmalloc_type() at compile time. So the conditional jumps
> and branches only appear when the gfp argument is not a constant. What makes
> you think it is important to optimize for that case?
>
> Bart.

I didn't really think it was all that important to optimize, but I
thought that was what you were trying to maintain with the earlier
patch since it was converting things to a table lookup.

If we really don't care then why even bother with the switch statement
anyway? It seems like you could just do one ternary operator and be
done with it. Basically all you need is:
return (defined(CONFIG_ZONE_DMA) && (flags & __GFP_DMA)) ? KMALLOC_DMA :
        (flags & __GFP_RECLAIMABLE) ? KMALLOC_RECLAIM : 0;

Why bother with all the extra complexity of the switch statement?

Thanks.

- Alex

Re: [PATCH] slab.h: Avoid using & for logical and of booleans

[Bart Van Assche]

On Mon, 2018-11-05 at 16:11 -0800, Alexander Duyck wrote:
> If we really don't care then why even bother with the switch statement
> anyway? It seems like you could just do one ternary operator and be
> done with it. Basically all you need is:
> return (defined(CONFIG_ZONE_DMA) && (flags & __GFP_DMA)) ? KMALLOC_DMA :
>         (flags & __GFP_RECLAIMABLE) ? KMALLOC_RECLAIM : 0;
> 
> Why bother with all the extra complexity of the switch statement?

I don't think that defined() can be used in a C expression. Hence the
IS_ENABLED() macro. If you fix that, leave out four superfluous parentheses,
test your patch, post that patch and cc me then I will add my Reviewed-by.

Bart.

Re: [PATCH] slab.h: Avoid using & for logical and of booleans

[Alexander Duyck]

On Mon, Nov 5, 2018 at 4:32 PM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On Mon, 2018-11-05 at 16:11 -0800, Alexander Duyck wrote:
> > If we really don't care then why even bother with the switch statement
> > anyway? It seems like you could just do one ternary operator and be
> > done with it. Basically all you need is:
> > return (defined(CONFIG_ZONE_DMA) && (flags & __GFP_DMA)) ? KMALLOC_DMA :
> >         (flags & __GFP_RECLAIMABLE) ? KMALLOC_RECLAIM : 0;
> >
> > Why bother with all the extra complexity of the switch statement?
>
> I don't think that defined() can be used in a C expression. Hence the
> IS_ENABLED() macro. If you fix that, leave out four superfluous parentheses,
> test your patch, post that patch and cc me then I will add my Reviewed-by.
>
> Bart.

Actually the defined macro is used multiple spots in if statements
throughout the kernel.

The reason for IS_ENABLED is to address the fact that we can be
dealing with macros that indicate if they are built in or a module
since those end up being two different defines depending on if you
select 'y' or 'm'.

Thanks.

- Alex

Re: [PATCH] slab.h: Avoid using & for logical and of booleans

[Bart Van Assche]

On Tue, 2018-11-06 at 09:20 -0800, Alexander Duyck wrote:
> On Mon, Nov 5, 2018 at 4:32 PM Bart Van Assche <bvanassche@acm.org> wrote:
> > 
> > On Mon, 2018-11-05 at 16:11 -0800, Alexander Duyck wrote:
> > > If we really don't care then why even bother with the switch statement
> > > anyway? It seems like you could just do one ternary operator and be
> > > done with it. Basically all you need is:
> > > return (defined(CONFIG_ZONE_DMA) && (flags & __GFP_DMA)) ? KMALLOC_DMA :
> > >         (flags & __GFP_RECLAIMABLE) ? KMALLOC_RECLAIM : 0;
> > > 
> > > Why bother with all the extra complexity of the switch statement?
> > 
> > I don't think that defined() can be used in a C expression. Hence the
> > IS_ENABLED() macro. If you fix that, leave out four superfluous parentheses,
> > test your patch, post that patch and cc me then I will add my Reviewed-by.
> 
> Actually the defined macro is used multiple spots in if statements
> throughout the kernel.

The only 'if (defined(' matches I found in the kernel tree that are not
preprocessor statements occur in Perl code. Maybe I overlooked something?

> The reason for IS_ENABLED is to address the fact that we can be
> dealing with macros that indicate if they are built in or a module
> since those end up being two different defines depending on if you
> select 'y' or 'm'.

>From Documentation/process/coding-style.rst:

Within code, where possible, use the IS_ENABLED macro to convert a Kconfig
symbol into a C boolean expression, and use it in a normal C conditional:

.. code-block:: c

	if (IS_ENABLED(CONFIG_SOMETHING)) {
		...
	}

Bart.

Re: [PATCH] slab.h: Avoid using & for logical and of booleans

[Alexander Duyck]

On Tue, Nov 6, 2018 at 9:48 AM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On Tue, 2018-11-06 at 09:20 -0800, Alexander Duyck wrote:
> > On Mon, Nov 5, 2018 at 4:32 PM Bart Van Assche <bvanassche@acm.org> wrote:
> > >
> > > On Mon, 2018-11-05 at 16:11 -0800, Alexander Duyck wrote:
> > > > If we really don't care then why even bother with the switch statement
> > > > anyway? It seems like you could just do one ternary operator and be
> > > > done with it. Basically all you need is:
> > > > return (defined(CONFIG_ZONE_DMA) && (flags & __GFP_DMA)) ? KMALLOC_DMA :
> > > >         (flags & __GFP_RECLAIMABLE) ? KMALLOC_RECLAIM : 0;
> > > >
> > > > Why bother with all the extra complexity of the switch statement?
> > >
> > > I don't think that defined() can be used in a C expression. Hence the
> > > IS_ENABLED() macro. If you fix that, leave out four superfluous parentheses,
> > > test your patch, post that patch and cc me then I will add my Reviewed-by.
> >
> > Actually the defined macro is used multiple spots in if statements
> > throughout the kernel.
>
> The only 'if (defined(' matches I found in the kernel tree that are not
> preprocessor statements occur in Perl code. Maybe I overlooked something?

You may be right. I think I was thinking of "__is_defined", not "defined".

> > The reason for IS_ENABLED is to address the fact that we can be
> > dealing with macros that indicate if they are built in or a module
> > since those end up being two different defines depending on if you
> > select 'y' or 'm'.
>
> From Documentation/process/coding-style.rst:
>
> Within code, where possible, use the IS_ENABLED macro to convert a Kconfig
> symbol into a C boolean expression, and use it in a normal C conditional:
>
> .. code-block:: c
>
>         if (IS_ENABLED(CONFIG_SOMETHING)) {
>                 ...
>         }
>
> Bart.

Right. Part of the reason for suggesting that is that depending on how
you define "CONFIG_SOMETHING" it can actually be defined as
"CONFIG_SOMETHING" or "CONFIG_SOMETHING_MODULE".  I was operating
under the assumption that CONFIG_ZONE_DMA wasn't ever going to be
built as a module.

Thanks.

- Alex

Re: [PATCH] slab.h: Avoid using & for logical and of booleans

[William Kucharski]

> On Nov 5, 2018, at 14:13, Andrew Morton <akpm@linux-foundation.org> wrote:
> 
>> On Mon,  5 Nov 2018 12:40:00 -0800 Bart Van Assche <bvanassche@acm.org> wrote:
>> -    return type_dma + (is_reclaimable & !is_dma) * KMALLOC_RECLAIM;
>> +    return type_dma + is_reclaimable * !is_dma * KMALLOC_RECLAIM;
>> }
>> 
>> /*
> 
> I suppose so.
> 
> That function seems too clever for its own good :(.  I wonder if these
> branch-avoiding tricks are really worthwhile.

At the very least I'd like to see some comments added as to why that approach was taken for the sake of future maintainers.

William Kucharski

Re: [PATCH] slab.h: Avoid using & for logical and of booleans

[Vlastimil Babka]

On 11/13/18 7:22 PM, Vlastimil Babka wrote:
> On 11/12/18 10:55 AM, David Laight wrote:
>> From: Vlastimil Babka [mailto:vbabka@suse.cz]
>>> Sent: 09 November 2018 19:16
>> ...
>>> This? Not terribly elegant, but I don't see a nicer way right now...
>>
>> Maybe just have two copies of the function body?
>>
>>  static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
>> {
>> #ifndef CONFIG_ZONE_DMA
>> 	return flags & __GFP_RECLAIMABLE ? KMALLOC_RECLAIM : KMALLOC_NORMAL;
>> #else
>> 	if (likely((flags & (__GFP_DMA | __GFP_RECLAIMABLE)) == 0))
>> 		return KMALLOC_NORMAL;
>> 	return flags & __GFP_DMA ? KMALLOC_DMA : KMALLOC_RECLAIM;
>> #endif
>> }
> 
> OK that's probably the most straightforward to follow, thanks.
> Note that for CONFIG_ZONE_DMA=n the result is identical to original code and
> all other attempts. flags & __GFP_DMA is converted to 1/0 index without branches
> or cmovs or whatnot.

Ping? Seems like people will report duplicates until the sparse warning
is gone in mainline...

Also CC linux-mm which was somehow lost.


----8<----