* Re: [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported
2021-05-10 15:02 ` [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported Hyeonggon Yoo
@ 2021-05-10 15:15 ` Christoph Lameter
2021-05-10 15:21 ` Vlastimil Babka
2021-05-11 3:09 ` Hyeonggon Yoo
2021-05-10 15:19 ` Vlastimil Babka
2021-05-10 15:44 ` [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported Matthew Wilcox
2 siblings, 2 replies; 19+ messages in thread
From: Christoph Lameter @ 2021-05-10 15:15 UTC (permalink / raw)
To: Hyeonggon Yoo
Cc: Vlastimil Babka, Matthew Wilcox, penberg, rientjes,
iamjoonsoo.kim, linux-mm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 235 bytes --]
I guess this needs to be reviewed and tested by the users of architectures
that can use large MAXORDER pages such as powerpc and Itanium.
On Tue, 11 May 2021, Hyeonggon Yoo wrote:
> updated patch. let me know if something is wrong!
>
[-- Attachment #2: Type: text/x-diff, Size: 2527 bytes --]
>From 8fe7ecdfb0f5bd5b08771512303d72f1c6447362 Mon Sep 17 00:00:00 2001
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Date: Mon, 10 May 2021 23:57:34 +0900
Subject: [PATCH] mm: kmalloc_index: make compiler break when size is not
supported
currently when size is not supported by kmalloc_index, compiler will not
break. so changed BUG to BUILD_BUG_ON_MSG to make compiler break if size is
wrong. this is done in compile time.
also removed code that allocates more than 32MB because current
implementation supports only up to 32MB.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
---
include/linux/slab.h | 7 +++++--
mm/slab_common.c | 7 +++----
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 0c97d788762c..fd0c7229d105 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -346,6 +346,9 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
* 1 = 65 .. 96 bytes
* 2 = 129 .. 192 bytes
* n = 2^(n-1)+1 .. 2^n
+ *
+ * Note: you don't need to optimize kmalloc_index because it's evaluated
+ * in compile-time.
*/
static __always_inline unsigned int kmalloc_index(size_t size)
{
@@ -382,8 +385,8 @@ static __always_inline unsigned int kmalloc_index(size_t size)
if (size <= 8 * 1024 * 1024) return 23;
if (size <= 16 * 1024 * 1024) return 24;
if (size <= 32 * 1024 * 1024) return 25;
- if (size <= 64 * 1024 * 1024) return 26;
- BUG();
+
+ BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
/* Will never be reached. Needed because the compiler may complain */
return -1;
diff --git a/mm/slab_common.c b/mm/slab_common.c
index f8833d3e5d47..39d4eca8cf9b 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -745,8 +745,8 @@ struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags)
/*
* kmalloc_info[] is to make slub_debug=,kmalloc-xx option work at boot time.
- * kmalloc_index() supports up to 2^26=64MB, so the final entry of the table is
- * kmalloc-67108864.
+ * kmalloc_index() supports up to 2^25=32MB, so the final entry of the table is
+ * kmalloc-33554432.
*/
const struct kmalloc_info_struct kmalloc_info[] __initconst = {
INIT_KMALLOC_INFO(0, 0),
@@ -774,8 +774,7 @@ const struct kmalloc_info_struct kmalloc_info[] __initconst = {
INIT_KMALLOC_INFO(4194304, 4M),
INIT_KMALLOC_INFO(8388608, 8M),
INIT_KMALLOC_INFO(16777216, 16M),
- INIT_KMALLOC_INFO(33554432, 32M),
- INIT_KMALLOC_INFO(67108864, 64M)
+ INIT_KMALLOC_INFO(33554432, 32M)
};
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported
2021-05-10 15:15 ` Christoph Lameter
@ 2021-05-10 15:21 ` Vlastimil Babka
2021-05-11 3:09 ` Hyeonggon Yoo
1 sibling, 0 replies; 19+ messages in thread
From: Vlastimil Babka @ 2021-05-10 15:21 UTC (permalink / raw)
To: Christoph Lameter, Hyeonggon Yoo
Cc: Matthew Wilcox, penberg, rientjes, iamjoonsoo.kim, linux-mm,
linux-kernel
On 5/10/21 5:15 PM, Christoph Lameter wrote:
> I guess this needs to be reviewed and tested by the users of architectures
> that can use large MAXORDER pages such as powerpc and Itanium.
AFAICS large MAX_ORDER should matter as KMALLOC_SHIFT_HIGH will be always capped
to 25. But sure, let the bots complain if something is wrong. I'm more
interested if we shake out some compiler that can't do the compile-time
evaluation properly and we didn't know until now.
> On Tue, 11 May 2021, Hyeonggon Yoo wrote:
>
>> updated patch. let me know if something is wrong!
>>
>
>
> 0001-mm-kmalloc_index-make-compiler-break-when-size-is-no.patch
>
>>From 8fe7ecdfb0f5bd5b08771512303d72f1c6447362 Mon Sep 17 00:00:00 2001
> From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> Date: Mon, 10 May 2021 23:57:34 +0900
> Subject: [PATCH] mm: kmalloc_index: make compiler break when size is not
> supported
>
> currently when size is not supported by kmalloc_index, compiler will not
> break. so changed BUG to BUILD_BUG_ON_MSG to make compiler break if size is
> wrong. this is done in compile time.
>
> also removed code that allocates more than 32MB because current
> implementation supports only up to 32MB.
>
> Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> ---
> include/linux/slab.h | 7 +++++--
> mm/slab_common.c | 7 +++----
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 0c97d788762c..fd0c7229d105 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -346,6 +346,9 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
> * 1 = 65 .. 96 bytes
> * 2 = 129 .. 192 bytes
> * n = 2^(n-1)+1 .. 2^n
> + *
> + * Note: you don't need to optimize kmalloc_index because it's evaluated
> + * in compile-time.
> */
> static __always_inline unsigned int kmalloc_index(size_t size)
> {
> @@ -382,8 +385,8 @@ static __always_inline unsigned int kmalloc_index(size_t size)
> if (size <= 8 * 1024 * 1024) return 23;
> if (size <= 16 * 1024 * 1024) return 24;
> if (size <= 32 * 1024 * 1024) return 25;
> - if (size <= 64 * 1024 * 1024) return 26;
> - BUG();
> +
> + BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
>
> /* Will never be reached. Needed because the compiler may complain */
> return -1;
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index f8833d3e5d47..39d4eca8cf9b 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -745,8 +745,8 @@ struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags)
>
> /*
> * kmalloc_info[] is to make slub_debug=,kmalloc-xx option work at boot time.
> - * kmalloc_index() supports up to 2^26=64MB, so the final entry of the table is
> - * kmalloc-67108864.
> + * kmalloc_index() supports up to 2^25=32MB, so the final entry of the table is
> + * kmalloc-33554432.
> */
> const struct kmalloc_info_struct kmalloc_info[] __initconst = {
> INIT_KMALLOC_INFO(0, 0),
> @@ -774,8 +774,7 @@ const struct kmalloc_info_struct kmalloc_info[] __initconst = {
> INIT_KMALLOC_INFO(4194304, 4M),
> INIT_KMALLOC_INFO(8388608, 8M),
> INIT_KMALLOC_INFO(16777216, 16M),
> - INIT_KMALLOC_INFO(33554432, 32M),
> - INIT_KMALLOC_INFO(67108864, 64M)
> + INIT_KMALLOC_INFO(33554432, 32M)
> };
>
> /*
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported
2021-05-10 15:15 ` Christoph Lameter
2021-05-10 15:21 ` Vlastimil Babka
@ 2021-05-11 3:09 ` Hyeonggon Yoo
1 sibling, 0 replies; 19+ messages in thread
From: Hyeonggon Yoo @ 2021-05-11 3:09 UTC (permalink / raw)
To: Christoph Lameter
Cc: Vlastimil Babka, Matthew Wilcox, penberg, rientjes,
iamjoonsoo.kim, linux-mm, linux-kernel
On Mon, May 10, 2021 at 05:15:23PM +0200, Christoph Lameter wrote:
> I guess this needs to be reviewed and tested by the users of architectures
> that can use large MAXORDER pages such as powerpc and Itanium.
as Vlastimil Babka said, I think it's fine too. because
KMALLOC_SHIFT_HIGH can't be bigger than 25 regardless of MAX_ORDER
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported
2021-05-10 15:02 ` [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported Hyeonggon Yoo
2021-05-10 15:15 ` Christoph Lameter
@ 2021-05-10 15:19 ` Vlastimil Babka
2021-05-10 15:38 ` Hyeonggon Yoo
2021-05-11 2:59 ` [PATCH v3] mm: change run-time assertion in kmalloc_index() to compile-time Hyeonggon Yoo
2021-05-10 15:44 ` [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported Matthew Wilcox
2 siblings, 2 replies; 19+ messages in thread
From: Vlastimil Babka @ 2021-05-10 15:19 UTC (permalink / raw)
To: Hyeonggon Yoo
Cc: Matthew Wilcox, cl, penberg, rientjes, iamjoonsoo.kim, linux-mm,
linux-kernel
On 5/10/21 5:02 PM, Hyeonggon Yoo wrote:
> updated patch. let me know if something is wrong!
>
>
> 0001-mm-kmalloc_index-make-compiler-break-when-size-is-no.patch
>
> From 8fe7ecdfb0f5bd5b08771512303d72f1c6447362 Mon Sep 17 00:00:00 2001
> From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> Date: Mon, 10 May 2021 23:57:34 +0900
> Subject: [PATCH] mm: kmalloc_index: make compiler break when size is not
> supported
I'd rephrase the subject:
mm, slub: change run-time assertion in kmalloc_index() to compile-time
> currently when size is not supported by kmalloc_index, compiler will not
> break.
"... compiler will generate a run-time BUG() while a compile-time error is also
possible, and better"
> so changed BUG to BUILD_BUG_ON_MSG to make compiler break if size is
> wrong. this is done in compile time.
>
> also removed code that allocates more than 32MB because current
> implementation supports only up to 32MB.
>
> Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> ---
> include/linux/slab.h | 7 +++++--
> mm/slab_common.c | 7 +++----
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 0c97d788762c..fd0c7229d105 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -346,6 +346,9 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
> * 1 = 65 .. 96 bytes
> * 2 = 129 .. 192 bytes
> * n = 2^(n-1)+1 .. 2^n
> + *
> + * Note: you don't need to optimize kmalloc_index because it's evaluated
"there's no need to..."
> + * in compile-time.
> */
> static __always_inline unsigned int kmalloc_index(size_t size)
> {
> @@ -382,8 +385,8 @@ static __always_inline unsigned int kmalloc_index(size_t size)
> if (size <= 8 * 1024 * 1024) return 23;
> if (size <= 16 * 1024 * 1024) return 24;
> if (size <= 32 * 1024 * 1024) return 25;
> - if (size <= 64 * 1024 * 1024) return 26;
> - BUG();
> +
> + BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
>
> /* Will never be reached. Needed because the compiler may complain */
> return -1;
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index f8833d3e5d47..39d4eca8cf9b 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -745,8 +745,8 @@ struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags)
>
> /*
> * kmalloc_info[] is to make slub_debug=,kmalloc-xx option work at boot time.
> - * kmalloc_index() supports up to 2^26=64MB, so the final entry of the table is
> - * kmalloc-67108864.
> + * kmalloc_index() supports up to 2^25=32MB, so the final entry of the table is
> + * kmalloc-33554432.
kmalloc-32M
> */
> const struct kmalloc_info_struct kmalloc_info[] __initconst = {
> INIT_KMALLOC_INFO(0, 0),
> @@ -774,8 +774,7 @@ const struct kmalloc_info_struct kmalloc_info[] __initconst = {
> INIT_KMALLOC_INFO(4194304, 4M),
> INIT_KMALLOC_INFO(8388608, 8M),
> INIT_KMALLOC_INFO(16777216, 16M),
> - INIT_KMALLOC_INFO(33554432, 32M),
> - INIT_KMALLOC_INFO(67108864, 64M)
> + INIT_KMALLOC_INFO(33554432, 32M)
> };
>
> /*
>
Thanks
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported
2021-05-10 15:19 ` Vlastimil Babka
@ 2021-05-10 15:38 ` Hyeonggon Yoo
2021-05-11 8:36 ` Vlastimil Babka
2021-05-11 2:59 ` [PATCH v3] mm: change run-time assertion in kmalloc_index() to compile-time Hyeonggon Yoo
1 sibling, 1 reply; 19+ messages in thread
From: Hyeonggon Yoo @ 2021-05-10 15:38 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Matthew Wilcox, cl, penberg, rientjes, iamjoonsoo.kim, linux-mm,
linux-kernel
On Mon, May 10, 2021 at 05:19:58PM +0200, Vlastimil Babka wrote:
> I'd rephrase the subject:
> mm, slub: change run-time assertion in kmalloc_index() to compile-time
>
> "... compiler will generate a run-time BUG() while a compile-time error is also
> possible, and better"
> "there's no need to..."
> kmalloc-32M
Vlastimil Babka and Christoph Lameter, thank you for reviewing the patch.
I'm not familiar with kernel community yet. should I send patch v3 again,
or can you update it directly?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported
2021-05-10 15:38 ` Hyeonggon Yoo
@ 2021-05-11 8:36 ` Vlastimil Babka
2021-05-11 8:37 ` Vlastimil Babka
0 siblings, 1 reply; 19+ messages in thread
From: Vlastimil Babka @ 2021-05-11 8:36 UTC (permalink / raw)
To: Hyeonggon Yoo
Cc: Matthew Wilcox, cl, penberg, rientjes, iamjoonsoo.kim, linux-mm,
linux-kernel
On 5/10/21 5:38 PM, Hyeonggon Yoo wrote:
> On Mon, May 10, 2021 at 05:19:58PM +0200, Vlastimil Babka wrote:
>
>> I'd rephrase the subject:
>> mm, slub: change run-time assertion in kmalloc_index() to compile-time
>>
>
>> "... compiler will generate a run-time BUG() while a compile-time error is also
>> possible, and better"
>
>> "there's no need to..."
>> kmalloc-32M
>
>
> Vlastimil Babka and Christoph Lameter, thank you for reviewing the patch.
>
> I'm not familiar with kernel community yet. should I send patch v3 again,
> or can you update it directly?
I think it would be best if you sent v3, the way you did with v1 - inline. With
v2 it looked like a mail body with you message and patch as attachment. We want
the testing bots to pick it up and they might work only with inline patch. Thanks.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported
2021-05-11 8:36 ` Vlastimil Babka
@ 2021-05-11 8:37 ` Vlastimil Babka
2021-05-11 9:14 ` Hyeonggon Yoo
0 siblings, 1 reply; 19+ messages in thread
From: Vlastimil Babka @ 2021-05-11 8:37 UTC (permalink / raw)
To: Hyeonggon Yoo
Cc: Matthew Wilcox, cl, penberg, rientjes, iamjoonsoo.kim, linux-mm,
linux-kernel
On 5/11/21 10:36 AM, Vlastimil Babka wrote:
> On 5/10/21 5:38 PM, Hyeonggon Yoo wrote:
>> On Mon, May 10, 2021 at 05:19:58PM +0200, Vlastimil Babka wrote:
>>
>>> I'd rephrase the subject:
>>> mm, slub: change run-time assertion in kmalloc_index() to compile-time
>>>
>>
>>> "... compiler will generate a run-time BUG() while a compile-time error is also
>>> possible, and better"
>>
>>> "there's no need to..."
>>> kmalloc-32M
>>
>>
>> Vlastimil Babka and Christoph Lameter, thank you for reviewing the patch.
>>
>> I'm not familiar with kernel community yet. should I send patch v3 again,
>> or can you update it directly?
>
> I think it would be best if you sent v3, the way you did with v1 - inline. With
> v2 it looked like a mail body with you message and patch as attachment. We want
> the testing bots to pick it up and they might work only with inline patch. Thanks.
Ah, you already did. Good.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported
2021-05-11 8:37 ` Vlastimil Babka
@ 2021-05-11 9:14 ` Hyeonggon Yoo
0 siblings, 0 replies; 19+ messages in thread
From: Hyeonggon Yoo @ 2021-05-11 9:14 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Matthew Wilcox, cl, penberg, rientjes, iamjoonsoo.kim, linux-mm,
linux-kernel, 42.hyeyoo
On Tue, May 11, 2021 at 10:37:20AM +0200, Vlastimil Babka wrote:
> >
> > I think it would be best if you sent v3, the way you did with v1 - inline. With
> > v2 it looked like a mail body with you message and patch as attachment. We want
> > the testing bots to pick it up and they might work only with inline patch. Thanks.
>
> Ah, you already did. Good.
>
OK, I will not send patch as an attachment later. it's first time for me
to hear about 'testing bots'. is there a documentation about it?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] mm: change run-time assertion in kmalloc_index() to compile-time
2021-05-10 15:19 ` Vlastimil Babka
2021-05-10 15:38 ` Hyeonggon Yoo
@ 2021-05-11 2:59 ` Hyeonggon Yoo
1 sibling, 0 replies; 19+ messages in thread
From: Hyeonggon Yoo @ 2021-05-11 2:59 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Matthew Wilcox, cl, penberg, rientjes, iamjoonsoo.kim, linux-mm,
linux-kernel
From c2b8fc4144f2c44b0ac5957a06207c385cc94d15 Mon Sep 17 00:00:00 2001
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Date: Tue, 11 May 2021 11:53:23 +0900
Subject: [PATCH] mm, slub: change run-time assertion in kmalloc_index() to
compile-time
currently when size is not supported by kmalloc_index, compiler will
generate a run-time BUG() while compile-time error is also possible,
and better. so changed BUG to BUILD_BUG_ON_MSG to make compile-time
check possible.
also removed code that allocates more than 32MB because current
implementation supports only up to 32MB.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
---
include/linux/slab.h | 7 +++++--
mm/slab_common.c | 7 +++----
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 6d454886bcaf..df1937309df2 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -345,6 +345,9 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
* 1 = 65 .. 96 bytes
* 2 = 129 .. 192 bytes
* n = 2^(n-1)+1 .. 2^n
+ *
+ * Note: there's no need to optimize kmalloc_index because it's evaluated
+ * in compile-time.
*/
static __always_inline unsigned int kmalloc_index(size_t size)
{
@@ -381,8 +384,8 @@ static __always_inline unsigned int kmalloc_index(size_t size)
if (size <= 8 * 1024 * 1024) return 23;
if (size <= 16 * 1024 * 1024) return 24;
if (size <= 32 * 1024 * 1024) return 25;
- if (size <= 64 * 1024 * 1024) return 26;
- BUG();
+
+ BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
/* Will never be reached. Needed because the compiler may complain */
return -1;
diff --git a/mm/slab_common.c b/mm/slab_common.c
index fe8b68482670..97664bbe8147 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1192,8 +1192,8 @@ struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags)
/*
* kmalloc_info[] is to make slub_debug=,kmalloc-xx option work at boot time.
- * kmalloc_index() supports up to 2^26=64MB, so the final entry of the table is
- * kmalloc-67108864.
+ * kmalloc_index() supports up to 2^25=32MB, so the final entry of the table is
+ * kmalloc-32M.
*/
const struct kmalloc_info_struct kmalloc_info[] __initconst = {
INIT_KMALLOC_INFO(0, 0),
@@ -1221,8 +1221,7 @@ const struct kmalloc_info_struct kmalloc_info[] __initconst = {
INIT_KMALLOC_INFO(4194304, 4M),
INIT_KMALLOC_INFO(8388608, 8M),
INIT_KMALLOC_INFO(16777216, 16M),
- INIT_KMALLOC_INFO(33554432, 32M),
- INIT_KMALLOC_INFO(67108864, 64M)
+ INIT_KMALLOC_INFO(33554432, 32M)
};
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported
2021-05-10 15:02 ` [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported Hyeonggon Yoo
2021-05-10 15:15 ` Christoph Lameter
2021-05-10 15:19 ` Vlastimil Babka
@ 2021-05-10 15:44 ` Matthew Wilcox
2021-05-11 3:03 ` Hyeonggon Yoo
2 siblings, 1 reply; 19+ messages in thread
From: Matthew Wilcox @ 2021-05-10 15:44 UTC (permalink / raw)
To: Hyeonggon Yoo
Cc: Vlastimil Babka, cl, penberg, rientjes, iamjoonsoo.kim, linux-mm,
linux-kernel
On Tue, May 11, 2021 at 12:02:30AM +0900, Hyeonggon Yoo wrote:
> @@ -382,8 +385,8 @@ static __always_inline unsigned int kmalloc_index(size_t size)
> if (size <= 8 * 1024 * 1024) return 23;
> if (size <= 16 * 1024 * 1024) return 24;
> if (size <= 32 * 1024 * 1024) return 25;
> - if (size <= 64 * 1024 * 1024) return 26;
> - BUG();
> +
> + BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
we're being encouraged to use static_assert() these days.
https://en.cppreference.com/w/c/language/_Static_assert
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported
2021-05-10 15:44 ` [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported Matthew Wilcox
@ 2021-05-11 3:03 ` Hyeonggon Yoo
2021-05-11 8:33 ` Vlastimil Babka
0 siblings, 1 reply; 19+ messages in thread
From: Hyeonggon Yoo @ 2021-05-11 3:03 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Vlastimil Babka, cl, penberg, rientjes, iamjoonsoo.kim, linux-mm,
linux-kernel
On Mon, May 10, 2021 at 04:44:09PM +0100, Matthew Wilcox wrote:
> On Tue, May 11, 2021 at 12:02:30AM +0900, Hyeonggon Yoo wrote:
> > @@ -382,8 +385,8 @@ static __always_inline unsigned int kmalloc_index(size_t size)
> > if (size <= 8 * 1024 * 1024) return 23;
> > if (size <= 16 * 1024 * 1024) return 24;
> > if (size <= 32 * 1024 * 1024) return 25;
> > - if (size <= 64 * 1024 * 1024) return 26;
> > - BUG();
> > +
> > + BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
>
> we're being encouraged to use static_assert() these days.
> https://en.cppreference.com/w/c/language/_Static_assert
>
can you tell me difference between static_assert and BUILD_BUG_ON?
it seems that mm subsystem does not make use of it now.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported
2021-05-11 3:03 ` Hyeonggon Yoo
@ 2021-05-11 8:33 ` Vlastimil Babka
0 siblings, 0 replies; 19+ messages in thread
From: Vlastimil Babka @ 2021-05-11 8:33 UTC (permalink / raw)
To: Hyeonggon Yoo, Matthew Wilcox
Cc: cl, penberg, rientjes, iamjoonsoo.kim, linux-mm, linux-kernel
On 5/11/21 5:03 AM, Hyeonggon Yoo wrote:
> On Mon, May 10, 2021 at 04:44:09PM +0100, Matthew Wilcox wrote:
>> On Tue, May 11, 2021 at 12:02:30AM +0900, Hyeonggon Yoo wrote:
>> > @@ -382,8 +385,8 @@ static __always_inline unsigned int kmalloc_index(size_t size)
>> > if (size <= 8 * 1024 * 1024) return 23;
>> > if (size <= 16 * 1024 * 1024) return 24;
>> > if (size <= 32 * 1024 * 1024) return 25;
>> > - if (size <= 64 * 1024 * 1024) return 26;
>> > - BUG();
>> > +
>> > + BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
>>
>> we're being encouraged to use static_assert() these days.
>> https://en.cppreference.com/w/c/language/_Static_assert
>>
>
> can you tell me difference between static_assert and BUILD_BUG_ON?
> it seems that mm subsystem does not make use of it now.
Some difference is explained in include/linux/build_bug.h near static_assert
definition.
But importantly it seems it's not possible to place static_assert(false) in
place of the BUG() because it will trigger despite not being reachable.
BUILD_BUG_ON_MSG(1 "..."); worked as expected for me.
^ permalink raw reply [flat|nested] 19+ messages in thread