From: Andrew Morton <akpm@linux-foundation.org>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: David Laight <David.Laight@ACULAB.COM>,
"'Bart Van Assche'" <bvanassche@acm.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Mel Gorman <mgorman@techsingularity.net>,
Christoph Lameter <cl@linux.com>, Roman Gushchin <guro@fb.com>,
"Darryl T. Agostinelli" <dagostinelli@gmail.com>
Subject: Re: [PATCH] slab.h: Avoid using & for logical and of booleans
Date: Fri, 9 Nov 2018 11:00:19 -0800 [thread overview]
Message-ID: <20181109110019.c82fba8125d4e2891fbe4a6c@linux-foundation.org> (raw)
In-Reply-To: <cbc1fc52-dc8c-aa38-8f29-22da8bcd91c1@suse.cz>
On Fri, 9 Nov 2018 09:12:09 +0100 Vlastimil Babka <vbabka@suse.cz> wrote:
> Multiple people have reported the following sparse warning:
>
> ./include/linux/slab.h:332:43: warning: dubious: x & !y
>
> The minimal fix would be to change the logical & to boolean &&, which emits the
> same code, but Andrew has suggested that the branch-avoiding tricks are maybe
> not worthwile. David Laight provided a nice comparison of disassembly of
> multiple variants, which shows that the current version produces a 4 deep
> dependency chain, and fixing the sparse warning by changing logical and to
> multiplication emits an IMUL, making it even more expensive.
>
> The code as rewritten by this patch yielded the best disassembly, with a single
> predictable branch for the most common case, and a ternary operator for the
> rest, which gcc seems to compile without a branch or cmov by itself.
>
> The result should be more readable, without a sparse warning and probably also
> faster for the common case.
>
> Reported-by: Bart Van Assche <bvanassche@acm.org>
> Reported-by: Darryl T. Agostinelli <dagostinelli@gmail.com>
> Suggested-by: Andrew Morton <akpm@linux-foundation.org>
> Suggested-by: David Laight <David.Laight@ACULAB.COM>
> Fixes: 1291523f2c1d ("mm, slab/slub: introduce kmalloc-reclaimable caches")
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> include/linux/slab.h | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 918f374e7156..18c6920c2803 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -304,6 +304,8 @@ enum kmalloc_cache_type {
> KMALLOC_RECLAIM,
> #ifdef CONFIG_ZONE_DMA
> KMALLOC_DMA,
> +#else
> + KMALLOC_DMA = KMALLOC_NORMAL,
> #endif
> NR_KMALLOC_TYPES
> };
I don't think this works correctly. Resetting KMALLOC_DMA to 0 will
cause NR_KMALLOC_TYPES to have value 1.
enum foo {
a = 0,
b,
c = 0,
d
}
main()
{
printf("%d %d %d %d\n", a, b, c, d);
}
akpm3> ./a.out
0 1 0 1
next prev parent reply other threads:[~2018-11-09 19:00 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-05 20:40 Bart Van Assche
2018-11-05 21:13 ` Andrew Morton
2018-11-05 21:48 ` Bart Van Assche
2018-11-05 22:14 ` Rasmus Villemoes
2018-11-05 22:40 ` Bart Van Assche
2018-11-05 22:48 ` Alexander Duyck
2018-11-06 0:01 ` Bart Van Assche
2018-11-06 0:11 ` Alexander Duyck
2018-11-06 0:32 ` Bart Van Assche
2018-11-06 17:20 ` Alexander Duyck
2018-11-06 17:48 ` Bart Van Assche
2018-11-06 18:17 ` Alexander Duyck
2018-11-06 9:45 ` William Kucharski
2018-11-06 8:40 ` Vlastimil Babka
2018-11-06 10:08 ` David Laight
2018-11-06 10:22 ` Vlastimil Babka
2018-11-06 11:07 ` David Laight
2018-11-06 12:51 ` Vlastimil Babka
2018-11-07 10:41 ` David Laight
2018-11-09 8:12 ` Vlastimil Babka
2018-11-09 19:00 ` Andrew Morton [this message]
2018-11-09 19:16 ` Vlastimil Babka
2018-11-09 19:47 ` Darryl T. Agostinelli
2018-11-09 21:31 ` Vlastimil Babka
2018-11-12 9:55 ` David Laight
2018-11-13 18:22 ` Vlastimil Babka
2018-11-21 13:22 ` Vlastimil Babka
2018-11-19 11:04 ` Pavel Machek
2018-11-19 12:51 ` Vlastimil Babka
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181109110019.c82fba8125d4e2891fbe4a6c@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=David.Laight@ACULAB.COM \
--cc=bvanassche@acm.org \
--cc=cl@linux.com \
--cc=dagostinelli@gmail.com \
--cc=guro@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@techsingularity.net \
--cc=vbabka@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome