All of lore.kernel.org
 help / color / mirror / Atom feed
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.

  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: link
Be 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.