From: Hyeonggon Yoo <42.hyeyoo@gmail.com> To: Vlastimil Babka <vbabka@suse.cz> Cc: kernel test robot <lkp@intel.com>, kbuild-all@lists.01.org, Linux Memory Management List <linux-mm@kvack.org>, Andrew Morton <akpm@linux-foundation.org> Subject: Some logical errors on my words, but I still wonder... Date: Thu, 10 Jun 2021 14:17:02 +0900 [thread overview] Message-ID: <20210610051702.GA5630@hyeyoo> (raw) In-Reply-To: <20210608184501.GA5505@hyeyoo> On Wed, Jun 09, 2021 at 03:45:01AM +0900, Hyeonggon Yoo wrote: > static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node) > { > > if ( ( > __builtin_constant_p( > !!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))) > ) > ? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) > : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 601, }; (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) > { > unsigned int i = __kmalloc_index(size, true); > > > they are so ugly but the point is: > > > > It seems that gcc evaluates > > > > > > __builtin_constant_p( > > > !!(builtin_constant_p(size) > > > && size <= KMALLOC_MAX_CACHE_SIZE) > > > ) > > > as true. > > > > > > mm.. when the size passed to bpf_map_kmalloc_node is not constant, > > > (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false. > > > then __builtin_constant_p(!!false) is true. So it calls kmalloc_index. > > > There were some logical errors on my words. I still think the compiler evaluate it as true, but that is not reason why kmalloc_node calls __kmalloc_index for non-constant size. I wonder why kmalloc_node called __kmalloc_index(size, size_is_constant=true) for non-constant size. Thanks, Hyeonggon > > > what change should be made? > > > just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition > > > doesn't make sense, because kmalloc_node is not working as expected. > some evidence to add: > > there are 4 calls of bpf_map_kmalloc_node. > if you comment out two calls of bpf_map_kmalloc_node with non-constant > (in line 168, 508), it doesn't make an error. So I thought > it was problem when non-constant size was passed. > > And if "kmalloc_index makes problem with non-constant size" is > true, then it is caller's fault because it is not allowed (except > in __kmalloc_index) > > kmalloc_node shouldn't call kmalloc_index if the size was not > constant.
WARNING: multiple messages have this Message-ID (diff)
From: Hyeonggon Yoo <42.hyeyoo@gmail.com> To: kbuild-all@lists.01.org Subject: Some logical errors on my words, but I still wonder... Date: Thu, 10 Jun 2021 14:17:02 +0900 [thread overview] Message-ID: <20210610051702.GA5630@hyeyoo> (raw) In-Reply-To: <20210608184501.GA5505@hyeyoo> [-- Attachment #1: Type: text/plain, Size: 2366 bytes --] On Wed, Jun 09, 2021 at 03:45:01AM +0900, Hyeonggon Yoo wrote: > static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node) > { > > if ( ( > __builtin_constant_p( > !!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))) > ) > ? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) > : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 601, }; (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) > { > unsigned int i = __kmalloc_index(size, true); > > > they are so ugly but the point is: > > > > It seems that gcc evaluates > > > > > > __builtin_constant_p( > > > !!(builtin_constant_p(size) > > > && size <= KMALLOC_MAX_CACHE_SIZE) > > > ) > > > as true. > > > > > > mm.. when the size passed to bpf_map_kmalloc_node is not constant, > > > (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false. > > > then __builtin_constant_p(!!false) is true. So it calls kmalloc_index. > > > There were some logical errors on my words. I still think the compiler evaluate it as true, but that is not reason why kmalloc_node calls __kmalloc_index for non-constant size. I wonder why kmalloc_node called __kmalloc_index(size, size_is_constant=true) for non-constant size. Thanks, Hyeonggon > > > what change should be made? > > > just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition > > > doesn't make sense, because kmalloc_node is not working as expected. > some evidence to add: > > there are 4 calls of bpf_map_kmalloc_node. > if you comment out two calls of bpf_map_kmalloc_node with non-constant > (in line 168, 508), it doesn't make an error. So I thought > it was problem when non-constant size was passed. > > And if "kmalloc_index makes problem with non-constant size" is > true, then it is caller's fault because it is not allowed (except > in __kmalloc_index) > > kmalloc_node shouldn't call kmalloc_index if the size was not > constant.
next prev parent reply other threads:[~2021-06-10 5:17 UTC|newest] Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-06-05 6:10 [linux-next:master 7012/7430] include/linux/compiler_types.h:328:38: error: call to '__compiletime_assert_183' declared with attribute error: unexpected size in kmalloc_index() kernel test robot 2021-06-05 6:10 ` kernel test robot 2021-06-06 11:08 ` Hyeonggon Yoo 2021-06-06 11:08 ` Hyeonggon Yoo 2021-06-07 11:40 ` Vlastimil Babka 2021-06-07 11:40 ` Vlastimil Babka 2021-06-07 12:25 ` Hyeonggon Yoo 2021-06-07 12:25 ` Hyeonggon Yoo 2021-06-07 15:27 ` Vlastimil Babka 2021-06-07 15:27 ` Vlastimil Babka 2021-06-07 15:49 ` Hyeonggon Yoo 2021-06-07 15:49 ` Hyeonggon Yoo 2021-06-08 7:57 ` Vlastimil Babka 2021-06-08 7:57 ` Vlastimil Babka 2021-06-08 17:05 ` Hyeonggon Yoo 2021-06-08 17:05 ` Hyeonggon Yoo 2021-06-08 17:27 ` Vlastimil Babka 2021-06-08 17:27 ` Vlastimil Babka 2021-06-08 18:45 ` Hyeonggon Yoo 2021-06-08 18:45 ` Hyeonggon Yoo 2021-06-08 18:50 ` Hyeonggon Yoo 2021-06-08 18:50 ` Hyeonggon Yoo 2021-06-10 5:17 ` Hyeonggon Yoo [this message] 2021-06-10 5:17 ` Some logical errors on my words, but I still wonder Hyeonggon Yoo 2021-06-10 11:43 ` [linux-next:master 7012/7430] include/linux/compiler_types.h:328:38: error: call to '__compiletime_assert_183' declared with attribute error: unexpected size in kmalloc_index() Vlastimil Babka 2021-06-10 11:43 ` Vlastimil Babka 2021-06-11 8:58 ` Hyeonggon Yoo 2021-06-11 10:27 ` Vlastimil Babka 2021-06-11 11:56 ` Hyeonggon Yoo 2021-06-11 23:59 ` Vlastimil Babka 2021-06-11 23:59 ` Vlastimil Babka 2021-06-12 0:19 ` Hyeonggon Yoo 2021-06-12 0:19 ` Hyeonggon Yoo 2021-06-14 9:26 ` [PATCH FIX -next] " Vlastimil Babka 2021-06-14 9:26 ` Vlastimil Babka 2021-06-11 16:56 ` Nathan Chancellor 2021-06-11 16:56 ` Nathan Chancellor 2021-06-12 0:31 ` Hyeonggon Yoo 2021-06-12 0:31 ` Hyeonggon Yoo
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=20210610051702.GA5630@hyeyoo \ --to=42.hyeyoo@gmail.com \ --cc=akpm@linux-foundation.org \ --cc=kbuild-all@lists.01.org \ --cc=linux-mm@kvack.org \ --cc=lkp@intel.com \ --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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.