리눅스 커널에 커밋 해보자
리눅스 커널에 커밋해보자! 리눅스 커널에 커밋한다는 건 정말 멋진 일이다. 하지만 나는 아직 쪼렙이기 때문에 커널의 중요한 파트에 기여할 수는 없다. 하지만 리눅스 커널에 커밋하기 위해서
hyeyoo.com
올해 3월에 썼던 글, drivers/staging에서 코딩 스타일을 고쳐서 보내봤다.
후기 먼저 적어보자면..
지금까지 기여했던 건 staging 디렉토리에서 코딩 스타일을 고치거나, 주석의 오타를 고치는 정도의 trivial한 패치였다면 이번에는 약간의 (?) discussion을 거쳐서 기여했다. 패치 자체는 대단한 내용은 아니다. 하지만 과정이 너무 재미있었고 좋은 경험이어서 자세하게 기록을 남겨본다.
이전까진 오픈소스가 모두에게 열려있다는 걸 머릿속으로만 알고있었는데, 이번에 정말 몸소 체감하게 됐다. 이름도 얼굴도 처음 보는 사람인데도 친절하게 리뷰해주고, 서로 의견을 나누는 게 정말 신기하게 느껴졌다. 그리고 모두에게 열려있는 만큼 왜 이 패치가 필요한지 명확하게 설명하고 설득하는 과정이 필요하다.
그리고 같이 의견을 나누는 개발자들이 Oracle, Intel, SUSE 이런 곳에서 일하는 정상급 개발자임을 생각해보면, 해당 프로젝트랑 커리어만 잘 연결한다면 오픈소스가 개발자로서 성장하기에 정말 좋겠다는 생각이 든다. (꼭 리눅스 커널이 아니더라도 그렇다.)
기여하게 된 과정
kmalloc 분석
시작은 메모리 할당 파트에서 kmalloc의 구현부를 보다가 의문이 들었다.
static __always_inline void *kmalloc(size_t size, gfp_t flags)
{
if (__builtin_constant_p(size)) {
#ifndef CONFIG_SLOB
unsigned int index;
#endif
if (size > KMALLOC_MAX_CACHE_SIZE)
return kmalloc_large(size, flags);
#ifndef CONFIG_SLOB
index = kmalloc_index(size);
if (!index)
return ZERO_SIZE_PTR;
return kmem_cache_alloc_trace(
kmalloc_caches[kmalloc_type(flags)][index],
flags, size);
#endif
}
return __kmalloc(size, flags);
} 우선 kmalloc은 size가 상수인지 체크한다. ( if (__builtin_constant_p(size)) 부분) size가 상수가 아니라면 __kmalloc이 호출되는데 이 함수는 더미 함수로 항상 실패한다. size가 상수라면 먼저 size가 KMALLOC_MAX_CACHE_SIZE를 넘는지 보는데, size > KMALLOC_MAX_CACHE_SIZE인 경우에는 페이지 단위의 할당을 하고, 그렇지 않으면 (size가 작으면) 슬랩 할당자를 호출한다.
kmalloc의 슬랩 할당자는 크기별로 나뉘어져있는데, 전역으로 kmalloc_caches라는 전역 변수가 존재한다. 이차원 배열이고, kmalloc의 타입 별로, 메모리의 크기 별로 캐시를 따로 선언한다. 이때 어떤 캐시를 사용할지는 index를 통해 결정한다.
struct kmem_cache *
kmalloc_caches[NR_KMALLOC_TYPES][KMALLOC_SHIFT_HIGH + 1] __ro_after_init =
{ /* initialization for https://bugs.llvm.org/show_bug.cgi?id=42570 */ }; 여기서 KMALLOC_SHIFT_HIGH는 최대 할당 가능한 캐시의 크기와 관련이 있다. 주석을 보면 kmalloc은 슬랩 할당자로 최대 32MB까지를 지원한다고 나와있다. KMALLOC_SHIFT_HIGH의 최댓 값은 정의상 25이며, 따라서 2^25 = 32MB까지 지원한다.
#ifdef CONFIG_SLAB
/*
* The largest kmalloc size supported by the SLAB allocators is
* 32 megabyte (2^25) or the maximum allocatable page order if that is
* less than 32 MB.
*
* WARNING: Its not easy to increase this value since the allocators have
* to do various tricks to work around compiler limitations in order to
* ensure proper constant folding.
*/
#define KMALLOC_SHIFT_HIGH ((MAX_ORDER + PAGE_SHIFT - 1) <= 25 ? \
(MAX_ORDER + PAGE_SHIFT - 1) : 25)
#define KMALLOC_SHIFT_MAX KMALLOC_SHIFT_HIGH
#ifndef KMALLOC_SHIFT_LOW
#define KMALLOC_SHIFT_LOW 5
#endif
#endif 사실 아까 본 KMALLOC_MAX_CACHE_SIZE도 정의를 살펴보면 (1UL << KMALLOC_SHIFT_HIGH) == 32MB이다.
#define KMALLOC_MAX_CACHE_SIZE (1UL << KMALLOC_SHIFT_HIGH) 자, 여기까진 코드가 매우 정상이다. 그럼 어디가 문제인가? 아까 살펴봤듯이 kmalloc에서는 작은 사이즈의 메모리를 할당할 때는 사이즈 별로 적당한 index를 구해서 kmalloc_caches 으로부터 알맞은 캐시를 찾아 해당 캐시로부터 메모리를 할당한다. 그럼 이 index를 구하는 로직을 보자.
/*
* Figure out which kmalloc slab an allocation of a certain size
* belongs to.
* 0 = zero alloc
* 1 = 65 .. 96 bytes
* 2 = 129 .. 192 bytes
* n = 2^(n-1)+1 .. 2^n
*/
static __always_inline unsigned int kmalloc_index(size_t size)
{
if (!size)
return 0;
if (size <= KMALLOC_MIN_SIZE)
return KMALLOC_SHIFT_LOW;
if (KMALLOC_MIN_SIZE <= 32 && size > 64 && size <= 96)
return 1;
if (KMALLOC_MIN_SIZE <= 64 && size > 128 && size <= 192)
return 2;
if (size <= 8) return 3;
if (size <= 16) return 4;
if (size <= 32) return 5;
if (size <= 64) return 6;
if (size <= 128) return 7;
if (size <= 256) return 8;
if (size <= 512) return 9;
if (size <= 1024) return 10;
if (size <= 2 * 1024) return 11;
if (size <= 4 * 1024) return 12;
if (size <= 8 * 1024) return 13;
if (size <= 16 * 1024) return 14;
if (size <= 32 * 1024) return 15;
if (size <= 64 * 1024) return 16;
if (size <= 128 * 1024) return 17;
if (size <= 256 * 1024) return 18;
if (size <= 512 * 1024) return 19;
if (size <= 1024 * 1024) return 20;
if (size <= 2 * 1024 * 1024) return 21;
if (size <= 4 * 1024 * 1024) return 22;
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();
/* Will never be reached. Needed because the compiler may complain */
return -1;
} kmalloc_index는 inline 함수이며, size별로 적절한 인덱스를 리턴한다. 그리고 size가 너무 큰 경우에는 BUG()를 호출한다. 그런데 마지막 if문에서는 size가 64MB보다 작거나 같은지 확인한다. 그런데 아까 봤던 코드 상에서는 32MB까지 지원하는데 좀 이상했다. 물론, 저 라인은 도달이 불가능하다. 왜냐하면 kmalloc의 if (size > KMALLOC_MAX_CACHE_SIZE) 부분 때문이다. 하지만 나중에 코드를 수정하다보면 문제가 생길 수 있을 것 같았다.
이메일을 통한 discussion
처음에 적당히 패치를 작성해서 get_maintainer.pl에 따라서 메인테이너랑 메일링 리스트에 메일을 보냈다.
From 5ec7c87d324a74ad12444d326181a7c368e0155e Mon Sep 17 00:00:00 2001
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Date: Sun, 9 May 2021 07:00:20 +0900
Subject: [PATCH] mm: kmalloc_index: remove case when size is more than 32MB
the return value of kmalloc_index is used as index of kmalloc_caches,
which is defined as:
struct kmem_cache *
kmalloc_caches[NR_KMALLOC_TYPES][KMALLOC_SHIFT_HIGH + 1]
and KMALLOC_SHIFT_HIGH is defined as:
#define KMALLOC_SHIFT_HIGH ((MAX_ORDER + PAGE_SHIFT - 1) <= 25 ? \
(MAX_ORDER + PAGE_SHIFT - 1) : 25)
KMALLOC_SHIFT_HIGH is maximum 25 by its definition. thus index of
kmalloc_caches cannot be 26. so this case should be removed.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
---
include/linux/slab.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 0c97d788762c..4694b1db4cb2 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -382,7 +382,6 @@ 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();
/* Will never be reached. Needed because the compiler may complain */
--
2.25.1Date Sun, 9 May 2021 00:19:40 +0100
From Matthew Wilcox <willy@infradead.org>
Subject Re: [PATCH] mm: kmalloc_index: remove case when size is more than 32MB
On Sun, May 09, 2021 at 07:13:28AM +0900, Hyeonggon Yoo wrote:
> the return value of kmalloc_index is used as index of kmalloc_caches,
it doesn't matter. every few weeks somebody posts a patch to "optimise"
kmalloc_index, failing to appreciate that it's only ever run at compile
time because it's all under __builtin_constant_p().그랬더니 willy라는 사람이 최적화를 할 필요는 없다고 한다. __builtin_constant_p라서 컴파일 할 때 다 최적화가 된다고 한다. 오, 이건 몰랐던 거다. 근데 내 패치는 최적화 패치가 아니다.
On 5/9/21 7:33 AM, Hyeonggon Yoo wrote:
> On Sun, May 09, 2021 at 12:19:40AM +0100, Matthew Wilcox wrote:
>> On Sun, May 09, 2021 at 07:13:28AM +0900, Hyeonggon Yoo wrote:
>> > the return value of kmalloc_index is used as index of kmalloc_caches,
>>
>> it doesn't matter. every few weeks somebody posts a patch to "optimise"
>> kmalloc_index, failing to appreciate that it's only ever run at compile
>> time because it's all under __builtin_constant_p().
>
> Oh thanks, I didn't know about __builtin_constant_p.
>
> But I was not optimizing kmalloc_index. isn't it confusing that
> kmalloc_caches alllows maximum size of 32MB, and kmalloc_index allows
> maximum size of 64MB?
>
> and even if the code I removed is never reached because 64MB is always
> bigger than KMALLOC_MAX_CACHE_SIZE, it will cause an error if reached.
KMALLOC_MAX_CACHE_SIZE depends on KMALLOC_SHIFT_HIGH
size of kmalloc_caches array depends on KMALLOC_SHIFT_HIGH
So I don't an easy way how it could become reachable while causing the index to
overflow - if someone increased KMALLOC_SHIFT_HIGH from 25 to 26, all should be
fine, AFAICS.
The problem would be if someone increased it to 27, then we might suddenly get a
BUG() in kmalloc_index(). We should probably replace that BUG() with
BUILD_BUG_ON(1) to catch that at compile time. Hopefully no supported compiler
will break because it's not able to do the proper compile-time evaluation - but
if it does, at least we would know.
So I would accept the patch if it also changed BUG() to e.g. BUILD_BUG_ON_MSG(1,
"unexpected size in kmalloc_index()");
and expanded the function's comment that this is always compile-time evaluated
and thus no attempts at "optimizing" the code should be made.
그러고 나서 메인테이너한테 답장이 왔다. 지금은 크게 문제가 안되는데, 나중에 KMALLOC_SHIFT_HIGH를 27로 바꾸면 문제가 될 수 있다고 한다. 그리고 BUG()는 run-time assertion이라, 이걸 BUILD_BUG_ON으로 바꿔서 compile-time assertion을 하는게 어떻냐고 한다. 그리고 위에서 willy가 말한 것처럼 kmalloc_index를 최적화 하려는 사람이 많았나보다. 그래서 최적화할 필요가 없다고 주석도 달아달라고 한다.
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그래서 메인테이너가 이야기한 부분이랑, 다른 코드에서 kmalloc_index가 64MB까지 지원한다고 생각하고 잘못 짠 코드도 수정해서 다시 보냈다.
Date: Mon, 10 May 2021 17:19:58 +0200
From: Vlastimil Babka <vbabka@suse.cz>
To: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Cc: Matthew Wilcox <willy@infradead.org>, cl@linux.com, penberg@kernel.org, rientjes@google.com,
iamjoonsoo.kim@lge.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] mm: kmalloc_index: make compiler break when size is not supported
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.0
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그랬더니 친절하게 커밋 메시지랑 주석에서 어색한 부분을 수정해주었다. 다시봐도 넘나 친절한 것..
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이게 마지막 패치다.
Date: Tue, 11 May 2021 20:08:15 +0200
From: Vlastimil Babka <vbabka@suse.cz>
To: Hyeonggon Yoo <42.hyeyoo@gmail.com>, akpm@linux-foundation.org, iamjoonsoo.kim@lge.com,
rientjes@google.com, penberg@kernel.org, cl@linux.com
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] mm, slub: change run-time assertion in kmalloc_index() to compile-time
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.0
On 5/11/21 7:34 PM, Hyeonggon Yoo wrote:
> 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>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
Thanks!
> ---
> 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)
> };
>
> /*
>
Reviewed-by: 를 찍어줬다. 예스! 그러고 얼마 안가서 -mm 트리에 추가됐다는 메일이 왔다. -mm -> linux->next -> mainline 순서로 패치가 이동하는 것 같다.
Date: Tue, 11 May 2021 14:03:27 -0700
From: akpm@linux-foundation.org
To: 42.hyeyoo@gmail.com, cl@linux.com, iamjoonsoo.kim@lge.com, mm-commits@vger.kernel.org,
penberg@kernel.org, rientjes@google.com, vbabka@suse.cz
Subject: + mm-slub-change-run-time-assertion-in-kmalloc_index-to-compile-time.patch added to -mm
tree
User-Agent: s-nail v14.8.16
The patch titled
Subject: mm, slub: change run-time assertion in kmalloc_index() to compile-time
has been added to the -mm tree. Its filename is
mm-slub-change-run-time-assertion-in-kmalloc_index-to-compile-time.patch
This patch should soon appear at
https://ozlabs.org/~akpm/mmots/broken-out/mm-slub-change-run-time-assertion-in-kmalloc_index-to-c
+ompile-time.patch
and later at
https://ozlabs.org/~akpm/mmotm/broken-out/mm-slub-change-run-time-assertion-in-kmalloc_index-to-c
+ompile-time.patch
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next and is updated
there every 3-4 working days
------------------------------------------------------
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Subject: 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 change BUG to BUILD_BUG_ON_MSG to make compile-time check
possible.
Also remove code that allocates more than 32MB because current
implementation supports only up to 32MB.
Link: https://lkml.kernel.org/r/20210511173448.GA54466@hyeyoo
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
include/linux/slab.h | 7 +++++--
mm/slab_common.c | 7 +++----
2 files changed, 8 insertions(+), 6 deletions(-)
--- a/include/linux/slab.h~mm-slub-change-run-time-assertion-in-kmalloc_index-to-compile-time
+++ a/include/linux/slab.h
@@ -346,6 +346,9 @@ static __always_inline enum kmalloc_cach
* 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)
{
@@ -382,8 +385,8 @@ static __always_inline unsigned int kmal
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;
--- a/mm/slab_common.c~mm-slub-change-run-time-assertion-in-kmalloc_index-to-compile-time
+++ a/mm/slab_common.c
@@ -755,8 +755,8 @@ struct kmem_cache *kmalloc_slab(size_t s
/*
* 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),
@@ -784,8 +784,7 @@ const struct kmalloc_info_struct kmalloc
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)
};
/*
_
Patches currently in -mm which might be from 42.hyeyoo@gmail.com are
mm-slub-change-run-time-assertion-in-kmalloc_index-to-compile-time.patch
mm-fix-typos-and-grammar-error-in-comments.patch
좀 길어서 요약했는데, 전체 내용은 아래 링크에서 확인할 수 있다.
'Kernel' 카테고리의 다른 글
| [Linux Kernel] 주소와 메모리 공간 (2) | 2021.05.15 |
|---|---|
| [Linux Kernel] 고민과 공부 방향 정리 (5) | 2021.05.12 |
| [Linux Kernel] 메모리 할당 (0) | 2021.05.08 |
| [Linux Kernel] 시간과 타이머 (1) | 2021.05.04 |
| [Linux Kernel] 인터럽트와 후반부 처리의 개념 (0) | 2021.04.28 |
댓글