* [PATCH] slab: Fix too strict alignment check in create_cache()
2024-11-20 12:46 [PATCH] slab: Fix too strict alignment check in create_cache() Geert Uytterhoeven
@ 2024-11-20 12:49 ` Geert Uytterhoeven
2024-11-20 15:00 ` Guenter Roeck
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2024-11-20 12:49 UTC (permalink / raw)
To: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton, Vlastimil Babka, Roman Gushchin, Hyeonggon Yoo,
Jens Axboe, Pavel Begunkov, Mike Rapoport, Christian Brauner,
Guenter Roeck, Kees Cook, Jann Horn
Cc: linux-mm, io-uring, linux-m68k, linux-kernel, Geert Uytterhoeven
On m68k, where the minimum alignment of unsigned long is 2 bytes:
Kernel panic - not syncing: __kmem_cache_create_args: Failed to create slab 'io_kiocb'. Error -22
CPU: 0 UID: 0 PID: 1 Comm: swapper Not tainted 6.12.0-atari-03776-g7eaa1f99261a #1783
Stack from 0102fe5c:
0102fe5c 00514a2b 00514a2b ffffff00 00000001 0051f5ed 00425e78 00514a2b
0041eb74 ffffffea 00000310 0051f5ed ffffffea ffffffea 00601f60 00000044
0102ff20 000e7a68 0051ab8e 004383b8 0051f5ed ffffffea 000000b8 00000007
01020c00 00000000 000e77f0 0041e5f0 005f67c0 0051f5ed 000000b6 0102fef4
00000310 0102fef4 00000000 00000016 005f676c 0060a34c 00000010 00000004
00000038 0000009a 01000000 000000b8 005f668e 0102e000 00001372 0102ff88
Call Trace: [<00425e78>] dump_stack+0xc/0x10
[<0041eb74>] panic+0xd8/0x26c
[<000e7a68>] __kmem_cache_create_args+0x278/0x2e8
[<000e77f0>] __kmem_cache_create_args+0x0/0x2e8
[<0041e5f0>] memset+0x0/0x8c
[<005f67c0>] io_uring_init+0x54/0xd2
The minimal alignment of an integral type may differ from its size,
hence is not safe to assume that an arbitrary freeptr_t (which is
basically an unsigned long) is always aligned to 4 or 8 bytes.
As nothing seems to require the additional alignment, it is safe to fix
this by relaxing the check to the actual minimum alignment of freeptr_t.
Fixes: aaa736b186239b7d ("io_uring: specify freeptr usage for SLAB_TYPESAFE_BY_RCU io_kiocb cache")
Fixes: d345bd2e9834e2da ("mm: add kmem_cache_create_rcu()")
Reported-by: Guenter Roeck <[email protected]>
Closes: https://lore.kernel.org/[email protected]
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
mm/slab_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 893d320599151845..f2f201d865c108bd 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -230,7 +230,7 @@ static struct kmem_cache *create_cache(const char *name,
if (args->use_freeptr_offset &&
(args->freeptr_offset >= object_size ||
!(flags & SLAB_TYPESAFE_BY_RCU) ||
- !IS_ALIGNED(args->freeptr_offset, sizeof(freeptr_t))))
+ !IS_ALIGNED(args->freeptr_offset, __alignof(freeptr_t))))
goto out;
err = -ENOMEM;
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] slab: Fix too strict alignment check in create_cache()
2024-11-20 12:46 [PATCH] slab: Fix too strict alignment check in create_cache() Geert Uytterhoeven
2024-11-20 12:49 ` Geert Uytterhoeven
@ 2024-11-20 15:00 ` Guenter Roeck
2024-11-20 15:01 ` Jens Axboe
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2024-11-20 15:00 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton, Vlastimil Babka, Roman Gushchin, Hyeonggon Yoo,
Jens Axboe, Pavel Begunkov, Mike, Rapoport, Christian Brauner,
Kees Cook, Jann Horn, linux-mm, io-uring, linux-m68k,
linux-kernel
On Wed, Nov 20, 2024 at 01:46:21PM +0100, Geert Uytterhoeven wrote:
> On m68k, where the minimum alignment of unsigned long is 2 bytes:
>
> Kernel panic - not syncing: __kmem_cache_create_args: Failed to create slab 'io_kiocb'. Error -22
> CPU: 0 UID: 0 PID: 1 Comm: swapper Not tainted 6.12.0-atari-03776-g7eaa1f99261a #1783
> Stack from 0102fe5c:
> 0102fe5c 00514a2b 00514a2b ffffff00 00000001 0051f5ed 00425e78 00514a2b
> 0041eb74 ffffffea 00000310 0051f5ed ffffffea ffffffea 00601f60 00000044
> 0102ff20 000e7a68 0051ab8e 004383b8 0051f5ed ffffffea 000000b8 00000007
> 01020c00 00000000 000e77f0 0041e5f0 005f67c0 0051f5ed 000000b6 0102fef4
> 00000310 0102fef4 00000000 00000016 005f676c 0060a34c 00000010 00000004
> 00000038 0000009a 01000000 000000b8 005f668e 0102e000 00001372 0102ff88
> Call Trace: [<00425e78>] dump_stack+0xc/0x10
> [<0041eb74>] panic+0xd8/0x26c
> [<000e7a68>] __kmem_cache_create_args+0x278/0x2e8
> [<000e77f0>] __kmem_cache_create_args+0x0/0x2e8
> [<0041e5f0>] memset+0x0/0x8c
> [<005f67c0>] io_uring_init+0x54/0xd2
>
> The minimal alignment of an integral type may differ from its size,
> hence is not safe to assume that an arbitrary freeptr_t (which is
> basically an unsigned long) is always aligned to 4 or 8 bytes.
>
> As nothing seems to require the additional alignment, it is safe to fix
> this by relaxing the check to the actual minimum alignment of freeptr_t.
>
> Fixes: aaa736b186239b7d ("io_uring: specify freeptr usage for SLAB_TYPESAFE_BY_RCU io_kiocb cache")
> Fixes: d345bd2e9834e2da ("mm: add kmem_cache_create_rcu()")
> Reported-by: Guenter Roeck <[email protected]>
> Closes: https://lore.kernel.org/[email protected]
> Signed-off-by: Geert Uytterhoeven <[email protected]>
On m68k:
Tested-by: Guenter Roeck <[email protected]>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] slab: Fix too strict alignment check in create_cache()
2024-11-20 12:46 [PATCH] slab: Fix too strict alignment check in create_cache() Geert Uytterhoeven
2024-11-20 12:49 ` Geert Uytterhoeven
2024-11-20 15:00 ` Guenter Roeck
@ 2024-11-20 15:01 ` Jens Axboe
2024-11-20 15:03 ` Vlastimil Babka
2024-11-21 10:19 ` Christian Brauner
4 siblings, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2024-11-20 15:01 UTC (permalink / raw)
To: Geert Uytterhoeven, Christoph Lameter, Pekka Enberg,
David Rientjes, Joonsoo Kim, Andrew Morton, Vlastimil Babka,
Roman Gushchin, Hyeonggon Yoo, Pavel Begunkov, Mike Rapoport,
Christian Brauner, Guenter Roeck, Kees Cook, Jann Horn
Cc: linux-mm, io-uring, linux-m68k, linux-kernel
On 11/20/24 5:49 AM, Geert Uytterhoeven wrote:
> On m68k, where the minimum alignment of unsigned long is 2 bytes:
>
> Kernel panic - not syncing: __kmem_cache_create_args: Failed to create slab 'io_kiocb'. Error -22
> CPU: 0 UID: 0 PID: 1 Comm: swapper Not tainted 6.12.0-atari-03776-g7eaa1f99261a #1783
> Stack from 0102fe5c:
> 0102fe5c 00514a2b 00514a2b ffffff00 00000001 0051f5ed 00425e78 00514a2b
> 0041eb74 ffffffea 00000310 0051f5ed ffffffea ffffffea 00601f60 00000044
> 0102ff20 000e7a68 0051ab8e 004383b8 0051f5ed ffffffea 000000b8 00000007
> 01020c00 00000000 000e77f0 0041e5f0 005f67c0 0051f5ed 000000b6 0102fef4
> 00000310 0102fef4 00000000 00000016 005f676c 0060a34c 00000010 00000004
> 00000038 0000009a 01000000 000000b8 005f668e 0102e000 00001372 0102ff88
> Call Trace: [<00425e78>] dump_stack+0xc/0x10
> [<0041eb74>] panic+0xd8/0x26c
> [<000e7a68>] __kmem_cache_create_args+0x278/0x2e8
> [<000e77f0>] __kmem_cache_create_args+0x0/0x2e8
> [<0041e5f0>] memset+0x0/0x8c
> [<005f67c0>] io_uring_init+0x54/0xd2
>
> The minimal alignment of an integral type may differ from its size,
> hence is not safe to assume that an arbitrary freeptr_t (which is
> basically an unsigned long) is always aligned to 4 or 8 bytes.
>
> As nothing seems to require the additional alignment, it is safe to fix
> this by relaxing the check to the actual minimum alignment of freeptr_t.
>
> Fixes: aaa736b186239b7d ("io_uring: specify freeptr usage for SLAB_TYPESAFE_BY_RCU io_kiocb cache")
> Fixes: d345bd2e9834e2da ("mm: add kmem_cache_create_rcu()")
> Reported-by: Guenter Roeck <[email protected]>
> Closes: https://lore.kernel.org/[email protected]
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> mm/slab_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 893d320599151845..f2f201d865c108bd 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -230,7 +230,7 @@ static struct kmem_cache *create_cache(const char *name,
> if (args->use_freeptr_offset &&
> (args->freeptr_offset >= object_size ||
> !(flags & SLAB_TYPESAFE_BY_RCU) ||
> - !IS_ALIGNED(args->freeptr_offset, sizeof(freeptr_t))))
> + !IS_ALIGNED(args->freeptr_offset, __alignof(freeptr_t))))
> goto out;
>
> err = -ENOMEM;
This looks much better, thanks.
Reviewed-by: Jens Axboe <[email protected]>
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] slab: Fix too strict alignment check in create_cache()
2024-11-20 12:46 [PATCH] slab: Fix too strict alignment check in create_cache() Geert Uytterhoeven
` (2 preceding siblings ...)
2024-11-20 15:01 ` Jens Axboe
@ 2024-11-20 15:03 ` Vlastimil Babka
2024-11-20 15:14 ` Guenter Roeck
2024-11-20 17:50 ` Christoph Lameter (Ampere)
2024-11-21 10:19 ` Christian Brauner
4 siblings, 2 replies; 12+ messages in thread
From: Vlastimil Babka @ 2024-11-20 15:03 UTC (permalink / raw)
To: Geert Uytterhoeven, Christoph Lameter, Pekka Enberg,
David Rientjes, Joonsoo Kim, Andrew Morton, Roman Gushchin,
Hyeonggon Yoo, Jens Axboe, Pavel Begunkov, Mike Rapoport,
Christian Brauner, Guenter Roeck, Kees Cook, Jann Horn
Cc: linux-mm, io-uring, linux-m68k, linux-kernel
On 11/20/24 13:49, Geert Uytterhoeven wrote:
> On m68k, where the minimum alignment of unsigned long is 2 bytes:
>
> Kernel panic - not syncing: __kmem_cache_create_args: Failed to create slab 'io_kiocb'. Error -22
> CPU: 0 UID: 0 PID: 1 Comm: swapper Not tainted 6.12.0-atari-03776-g7eaa1f99261a #1783
> Stack from 0102fe5c:
> 0102fe5c 00514a2b 00514a2b ffffff00 00000001 0051f5ed 00425e78 00514a2b
> 0041eb74 ffffffea 00000310 0051f5ed ffffffea ffffffea 00601f60 00000044
> 0102ff20 000e7a68 0051ab8e 004383b8 0051f5ed ffffffea 000000b8 00000007
> 01020c00 00000000 000e77f0 0041e5f0 005f67c0 0051f5ed 000000b6 0102fef4
> 00000310 0102fef4 00000000 00000016 005f676c 0060a34c 00000010 00000004
> 00000038 0000009a 01000000 000000b8 005f668e 0102e000 00001372 0102ff88
> Call Trace: [<00425e78>] dump_stack+0xc/0x10
> [<0041eb74>] panic+0xd8/0x26c
> [<000e7a68>] __kmem_cache_create_args+0x278/0x2e8
> [<000e77f0>] __kmem_cache_create_args+0x0/0x2e8
> [<0041e5f0>] memset+0x0/0x8c
> [<005f67c0>] io_uring_init+0x54/0xd2
>
> The minimal alignment of an integral type may differ from its size,
> hence is not safe to assume that an arbitrary freeptr_t (which is
> basically an unsigned long) is always aligned to 4 or 8 bytes.
>
> As nothing seems to require the additional alignment, it is safe to fix
> this by relaxing the check to the actual minimum alignment of freeptr_t.
>
> Fixes: aaa736b186239b7d ("io_uring: specify freeptr usage for SLAB_TYPESAFE_BY_RCU io_kiocb cache")
> Fixes: d345bd2e9834e2da ("mm: add kmem_cache_create_rcu()")
> Reported-by: Guenter Roeck <[email protected]>
> Closes: https://lore.kernel.org/[email protected]
> Signed-off-by: Geert Uytterhoeven <[email protected]>
Thanks, will add it to slab pull for 6.13.
> ---
> mm/slab_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 893d320599151845..f2f201d865c108bd 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -230,7 +230,7 @@ static struct kmem_cache *create_cache(const char *name,
> if (args->use_freeptr_offset &&
> (args->freeptr_offset >= object_size ||
> !(flags & SLAB_TYPESAFE_BY_RCU) ||
> - !IS_ALIGNED(args->freeptr_offset, sizeof(freeptr_t))))
> + !IS_ALIGNED(args->freeptr_offset, __alignof(freeptr_t))))
Seems only bunch of places uses __alignof but many use __alignoff__ and this
also is what seems to be documented?
> goto out;
>
> err = -ENOMEM;
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] slab: Fix too strict alignment check in create_cache()
2024-11-20 15:03 ` Vlastimil Babka
@ 2024-11-20 15:14 ` Guenter Roeck
2024-11-20 15:44 ` Vlastimil Babka
2024-11-20 17:50 ` Christoph Lameter (Ampere)
1 sibling, 1 reply; 12+ messages in thread
From: Guenter Roeck @ 2024-11-20 15:14 UTC (permalink / raw)
To: Vlastimil Babka, Geert Uytterhoeven, Christoph Lameter,
Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton,
Roman Gushchin, Hyeonggon Yoo, Jens Axboe, Pavel Begunkov,
Mike Rapoport, Christian Brauner, Kees Cook, Jann Horn
Cc: linux-mm, io-uring, linux-m68k, linux-kernel
On 11/20/24 07:03, Vlastimil Babka wrote:
> On 11/20/24 13:49, Geert Uytterhoeven wrote:
>> On m68k, where the minimum alignment of unsigned long is 2 bytes:
>>
>> Kernel panic - not syncing: __kmem_cache_create_args: Failed to create slab 'io_kiocb'. Error -22
>> CPU: 0 UID: 0 PID: 1 Comm: swapper Not tainted 6.12.0-atari-03776-g7eaa1f99261a #1783
>> Stack from 0102fe5c:
>> 0102fe5c 00514a2b 00514a2b ffffff00 00000001 0051f5ed 00425e78 00514a2b
>> 0041eb74 ffffffea 00000310 0051f5ed ffffffea ffffffea 00601f60 00000044
>> 0102ff20 000e7a68 0051ab8e 004383b8 0051f5ed ffffffea 000000b8 00000007
>> 01020c00 00000000 000e77f0 0041e5f0 005f67c0 0051f5ed 000000b6 0102fef4
>> 00000310 0102fef4 00000000 00000016 005f676c 0060a34c 00000010 00000004
>> 00000038 0000009a 01000000 000000b8 005f668e 0102e000 00001372 0102ff88
>> Call Trace: [<00425e78>] dump_stack+0xc/0x10
>> [<0041eb74>] panic+0xd8/0x26c
>> [<000e7a68>] __kmem_cache_create_args+0x278/0x2e8
>> [<000e77f0>] __kmem_cache_create_args+0x0/0x2e8
>> [<0041e5f0>] memset+0x0/0x8c
>> [<005f67c0>] io_uring_init+0x54/0xd2
>>
>> The minimal alignment of an integral type may differ from its size,
>> hence is not safe to assume that an arbitrary freeptr_t (which is
>> basically an unsigned long) is always aligned to 4 or 8 bytes.
>>
>> As nothing seems to require the additional alignment, it is safe to fix
>> this by relaxing the check to the actual minimum alignment of freeptr_t.
>>
>> Fixes: aaa736b186239b7d ("io_uring: specify freeptr usage for SLAB_TYPESAFE_BY_RCU io_kiocb cache")
>> Fixes: d345bd2e9834e2da ("mm: add kmem_cache_create_rcu()")
>> Reported-by: Guenter Roeck <[email protected]>
>> Closes: https://lore.kernel.org/[email protected]
>> Signed-off-by: Geert Uytterhoeven <[email protected]>
>
> Thanks, will add it to slab pull for 6.13.
>
>> ---
>> mm/slab_common.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>> index 893d320599151845..f2f201d865c108bd 100644
>> --- a/mm/slab_common.c
>> +++ b/mm/slab_common.c
>> @@ -230,7 +230,7 @@ static struct kmem_cache *create_cache(const char *name,
>> if (args->use_freeptr_offset &&
>> (args->freeptr_offset >= object_size ||
>> !(flags & SLAB_TYPESAFE_BY_RCU) ||
>> - !IS_ALIGNED(args->freeptr_offset, sizeof(freeptr_t))))
>> + !IS_ALIGNED(args->freeptr_offset, __alignof(freeptr_t))))
>
> Seems only bunch of places uses __alignof but many use __alignoff__ and this
> also is what seems to be documented?
__alignoff__ -> __alignof__
Guenter
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] slab: Fix too strict alignment check in create_cache()
2024-11-20 15:14 ` Guenter Roeck
@ 2024-11-20 15:44 ` Vlastimil Babka
2024-11-20 15:50 ` Geert Uytterhoeven
0 siblings, 1 reply; 12+ messages in thread
From: Vlastimil Babka @ 2024-11-20 15:44 UTC (permalink / raw)
To: Guenter Roeck, Geert Uytterhoeven, Christoph Lameter,
Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton,
Roman Gushchin, Hyeonggon Yoo, Jens Axboe, Pavel Begunkov,
Mike Rapoport, Christian Brauner, Kees Cook, Jann Horn
Cc: linux-mm, io-uring, linux-m68k, linux-kernel
On 11/20/24 16:14, Guenter Roeck wrote:
> On 11/20/24 07:03, Vlastimil Babka wrote:
>> On 11/20/24 13:49, Geert Uytterhoeven wrote:
>>> On m68k, where the minimum alignment of unsigned long is 2 bytes:
>>>
>>> Kernel panic - not syncing: __kmem_cache_create_args: Failed to create slab 'io_kiocb'. Error -22
>>> CPU: 0 UID: 0 PID: 1 Comm: swapper Not tainted 6.12.0-atari-03776-g7eaa1f99261a #1783
>>> Stack from 0102fe5c:
>>> 0102fe5c 00514a2b 00514a2b ffffff00 00000001 0051f5ed 00425e78 00514a2b
>>> 0041eb74 ffffffea 00000310 0051f5ed ffffffea ffffffea 00601f60 00000044
>>> 0102ff20 000e7a68 0051ab8e 004383b8 0051f5ed ffffffea 000000b8 00000007
>>> 01020c00 00000000 000e77f0 0041e5f0 005f67c0 0051f5ed 000000b6 0102fef4
>>> 00000310 0102fef4 00000000 00000016 005f676c 0060a34c 00000010 00000004
>>> 00000038 0000009a 01000000 000000b8 005f668e 0102e000 00001372 0102ff88
>>> Call Trace: [<00425e78>] dump_stack+0xc/0x10
>>> [<0041eb74>] panic+0xd8/0x26c
>>> [<000e7a68>] __kmem_cache_create_args+0x278/0x2e8
>>> [<000e77f0>] __kmem_cache_create_args+0x0/0x2e8
>>> [<0041e5f0>] memset+0x0/0x8c
>>> [<005f67c0>] io_uring_init+0x54/0xd2
>>>
>>> The minimal alignment of an integral type may differ from its size,
>>> hence is not safe to assume that an arbitrary freeptr_t (which is
>>> basically an unsigned long) is always aligned to 4 or 8 bytes.
>>>
>>> As nothing seems to require the additional alignment, it is safe to fix
>>> this by relaxing the check to the actual minimum alignment of freeptr_t.
>>>
>>> Fixes: aaa736b186239b7d ("io_uring: specify freeptr usage for SLAB_TYPESAFE_BY_RCU io_kiocb cache")
>>> Fixes: d345bd2e9834e2da ("mm: add kmem_cache_create_rcu()")
>>> Reported-by: Guenter Roeck <[email protected]>
>>> Closes: https://lore.kernel.org/[email protected]
>>> Signed-off-by: Geert Uytterhoeven <[email protected]>
>>
>> Thanks, will add it to slab pull for 6.13.
>>
>>> ---
>>> mm/slab_common.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>>> index 893d320599151845..f2f201d865c108bd 100644
>>> --- a/mm/slab_common.c
>>> +++ b/mm/slab_common.c
>>> @@ -230,7 +230,7 @@ static struct kmem_cache *create_cache(const char *name,
>>> if (args->use_freeptr_offset &&
>>> (args->freeptr_offset >= object_size ||
>>> !(flags & SLAB_TYPESAFE_BY_RCU) ||
>>> - !IS_ALIGNED(args->freeptr_offset, sizeof(freeptr_t))))
>>> + !IS_ALIGNED(args->freeptr_offset, __alignof(freeptr_t))))
>>
>> Seems only bunch of places uses __alignof but many use __alignoff__ and this
>> also is what seems to be documented?
>
> __alignoff__ -> __alignof__
Yeah I meant __alignof__
Will chage it locally then.
> Guenter
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] slab: Fix too strict alignment check in create_cache()
2024-11-20 15:44 ` Vlastimil Babka
@ 2024-11-20 15:50 ` Geert Uytterhoeven
0 siblings, 0 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2024-11-20 15:50 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Guenter Roeck, Christoph Lameter, Pekka Enberg, David Rientjes,
Joonsoo Kim, Andrew Morton, Roman Gushchin, Hyeonggon Yoo,
Jens Axboe, Pavel Begunkov, Mike Rapoport, Christian Brauner,
Kees Cook, Jann Horn, linux-mm, io-uring, linux-m68k,
linux-kernel
Hi Vlastimil,
On Wed, Nov 20, 2024 at 4:44 PM Vlastimil Babka <[email protected]> wrote:
> On 11/20/24 16:14, Guenter Roeck wrote:
> > On 11/20/24 07:03, Vlastimil Babka wrote:
> >> On 11/20/24 13:49, Geert Uytterhoeven wrote:
> >>> On m68k, where the minimum alignment of unsigned long is 2 bytes:
> >>>
> >>> Kernel panic - not syncing: __kmem_cache_create_args: Failed to create slab 'io_kiocb'. Error -22
> >>> CPU: 0 UID: 0 PID: 1 Comm: swapper Not tainted 6.12.0-atari-03776-g7eaa1f99261a #1783
> >>> Stack from 0102fe5c:
> >>> 0102fe5c 00514a2b 00514a2b ffffff00 00000001 0051f5ed 00425e78 00514a2b
> >>> 0041eb74 ffffffea 00000310 0051f5ed ffffffea ffffffea 00601f60 00000044
> >>> 0102ff20 000e7a68 0051ab8e 004383b8 0051f5ed ffffffea 000000b8 00000007
> >>> 01020c00 00000000 000e77f0 0041e5f0 005f67c0 0051f5ed 000000b6 0102fef4
> >>> 00000310 0102fef4 00000000 00000016 005f676c 0060a34c 00000010 00000004
> >>> 00000038 0000009a 01000000 000000b8 005f668e 0102e000 00001372 0102ff88
> >>> Call Trace: [<00425e78>] dump_stack+0xc/0x10
> >>> [<0041eb74>] panic+0xd8/0x26c
> >>> [<000e7a68>] __kmem_cache_create_args+0x278/0x2e8
> >>> [<000e77f0>] __kmem_cache_create_args+0x0/0x2e8
> >>> [<0041e5f0>] memset+0x0/0x8c
> >>> [<005f67c0>] io_uring_init+0x54/0xd2
> >>>
> >>> The minimal alignment of an integral type may differ from its size,
> >>> hence is not safe to assume that an arbitrary freeptr_t (which is
> >>> basically an unsigned long) is always aligned to 4 or 8 bytes.
> >>>
> >>> As nothing seems to require the additional alignment, it is safe to fix
> >>> this by relaxing the check to the actual minimum alignment of freeptr_t.
> >>>
> >>> Fixes: aaa736b186239b7d ("io_uring: specify freeptr usage for SLAB_TYPESAFE_BY_RCU io_kiocb cache")
> >>> Fixes: d345bd2e9834e2da ("mm: add kmem_cache_create_rcu()")
> >>> Reported-by: Guenter Roeck <[email protected]>
> >>> Closes: https://lore.kernel.org/[email protected]
> >>> Signed-off-by: Geert Uytterhoeven <[email protected]>
> >>
> >> Thanks, will add it to slab pull for 6.13.
> >>
> >>> ---
> >>> mm/slab_common.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/mm/slab_common.c b/mm/slab_common.c
> >>> index 893d320599151845..f2f201d865c108bd 100644
> >>> --- a/mm/slab_common.c
> >>> +++ b/mm/slab_common.c
> >>> @@ -230,7 +230,7 @@ static struct kmem_cache *create_cache(const char *name,
> >>> if (args->use_freeptr_offset &&
> >>> (args->freeptr_offset >= object_size ||
> >>> !(flags & SLAB_TYPESAFE_BY_RCU) ||
> >>> - !IS_ALIGNED(args->freeptr_offset, sizeof(freeptr_t))))
> >>> + !IS_ALIGNED(args->freeptr_offset, __alignof(freeptr_t))))
> >>
> >> Seems only bunch of places uses __alignof but many use __alignoff__ and this
> >> also is what seems to be documented?
> >
> > __alignoff__ -> __alignof__
>
> Yeah I meant __alignof__
> Will chage it locally then.
Thank you!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] slab: Fix too strict alignment check in create_cache()
2024-11-20 15:03 ` Vlastimil Babka
2024-11-20 15:14 ` Guenter Roeck
@ 2024-11-20 17:50 ` Christoph Lameter (Ampere)
2024-11-21 3:51 ` Matthew Wilcox
2024-11-21 8:15 ` Geert Uytterhoeven
1 sibling, 2 replies; 12+ messages in thread
From: Christoph Lameter (Ampere) @ 2024-11-20 17:50 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Geert Uytterhoeven, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton, Roman Gushchin, Hyeonggon Yoo, Jens Axboe,
Pavel Begunkov, Mike Rapoport, Christian Brauner, Guenter Roeck,
Kees Cook, Jann Horn, linux-mm, io-uring, linux-m68k,
linux-kernel
On Wed, 20 Nov 2024, Vlastimil Babka wrote:
> >
> > Fixes: aaa736b186239b7d ("io_uring: specify freeptr usage for SLAB_TYPESAFE_BY_RCU io_kiocb cache")
> > Fixes: d345bd2e9834e2da ("mm: add kmem_cache_create_rcu()")
> > Reported-by: Guenter Roeck <[email protected]>
> > Closes: https://lore.kernel.org/[email protected]
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
>
> Thanks, will add it to slab pull for 6.13.
Note that there are widespread assumptions in kernel code that the
alignment of scalars is the "natural alignment". Other portions of the
kernel may break. The compiler actually goes along with this??
How do you deal with torn reads/writes in such a scenario? Is this UP
only?
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] slab: Fix too strict alignment check in create_cache()
2024-11-20 17:50 ` Christoph Lameter (Ampere)
@ 2024-11-21 3:51 ` Matthew Wilcox
2024-11-21 8:15 ` Geert Uytterhoeven
1 sibling, 0 replies; 12+ messages in thread
From: Matthew Wilcox @ 2024-11-21 3:51 UTC (permalink / raw)
To: Christoph Lameter (Ampere)
Cc: Vlastimil Babka, Geert Uytterhoeven, Pekka Enberg, David Rientjes,
Joonsoo Kim, Andrew Morton, Roman Gushchin, Hyeonggon Yoo,
Jens Axboe, Pavel Begunkov, Mike Rapoport, Christian Brauner,
Guenter Roeck, Kees Cook, Jann Horn, linux-mm, io-uring,
linux-m68k, linux-kernel
On Wed, Nov 20, 2024 at 09:50:47AM -0800, Christoph Lameter (Ampere) wrote:
> On Wed, 20 Nov 2024, Vlastimil Babka wrote:
>
> > >
> > > Fixes: aaa736b186239b7d ("io_uring: specify freeptr usage for SLAB_TYPESAFE_BY_RCU io_kiocb cache")
> > > Fixes: d345bd2e9834e2da ("mm: add kmem_cache_create_rcu()")
> > > Reported-by: Guenter Roeck <[email protected]>
> > > Closes: https://lore.kernel.org/[email protected]
> > > Signed-off-by: Geert Uytterhoeven <[email protected]>
> >
> > Thanks, will add it to slab pull for 6.13.
>
> Note that there are widespread assumptions in kernel code that the
> alignment of scalars is the "natural alignment". Other portions of the
> kernel may break. The compiler actually goes along with this??
u64s aren't aligned on x86-32. it's caused some problems over the
years, but things work ok in general.
> How do you deal with torn reads/writes in such a scenario? Is this UP
> only?
there were never a lot of smp m68k. not sure i can think of one, tbh.
sun3 and hp300/400 seem like the obvious people who might have done an
smp m68k, but neither did.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] slab: Fix too strict alignment check in create_cache()
2024-11-20 17:50 ` Christoph Lameter (Ampere)
2024-11-21 3:51 ` Matthew Wilcox
@ 2024-11-21 8:15 ` Geert Uytterhoeven
1 sibling, 0 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2024-11-21 8:15 UTC (permalink / raw)
To: Christoph Lameter (Ampere)
Cc: Vlastimil Babka, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton, Roman Gushchin, Hyeonggon Yoo, Jens Axboe,
Pavel Begunkov, Mike Rapoport, Christian Brauner, Guenter Roeck,
Kees Cook, Jann Horn, linux-mm, io-uring, linux-m68k,
linux-kernel
Hi Christoph,
On Wed, Nov 20, 2024 at 6:50 PM Christoph Lameter (Ampere)
<[email protected]> wrote:
> On Wed, 20 Nov 2024, Vlastimil Babka wrote:
> > > Fixes: aaa736b186239b7d ("io_uring: specify freeptr usage for SLAB_TYPESAFE_BY_RCU io_kiocb cache")
> > > Fixes: d345bd2e9834e2da ("mm: add kmem_cache_create_rcu()")
> > > Reported-by: Guenter Roeck <[email protected]>
> > > Closes: https://lore.kernel.org/[email protected]
> > > Signed-off-by: Geert Uytterhoeven <[email protected]>
> >
> > Thanks, will add it to slab pull for 6.13.
>
> Note that there are widespread assumptions in kernel code that the
> alignment of scalars is the "natural alignment". Other portions of the
> kernel may break. The compiler actually goes along with this??
Linux has supported m68k since last century.
Any new such assumptions are fixed quickly (at least in the kernel).
If you need a specific alignment, make sure to use __aligned and/or
appropriate padding in structures.
And yes, the compiler knows, and provides __alignof__.
> How do you deal with torn reads/writes in such a scenario? Is this UP
> only?
Linux does not support (rate) SMP m68k machines.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] slab: Fix too strict alignment check in create_cache()
2024-11-20 12:46 [PATCH] slab: Fix too strict alignment check in create_cache() Geert Uytterhoeven
` (3 preceding siblings ...)
2024-11-20 15:03 ` Vlastimil Babka
@ 2024-11-21 10:19 ` Christian Brauner
4 siblings, 0 replies; 12+ messages in thread
From: Christian Brauner @ 2024-11-21 10:19 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton, Vlastimil Babka, Roman Gushchin, Hyeonggon Yoo,
Jens Axboe, Pavel Begunkov, Mike, Rapoport, Guenter Roeck,
Kees Cook, Jann Horn, linux-mm, io-uring, linux-m68k,
linux-kernel
On Wed, Nov 20, 2024 at 01:46:21PM +0100, Geert Uytterhoeven wrote:
> On m68k, where the minimum alignment of unsigned long is 2 bytes:
>
> Kernel panic - not syncing: __kmem_cache_create_args: Failed to create slab 'io_kiocb'. Error -22
> CPU: 0 UID: 0 PID: 1 Comm: swapper Not tainted 6.12.0-atari-03776-g7eaa1f99261a #1783
> Stack from 0102fe5c:
> 0102fe5c 00514a2b 00514a2b ffffff00 00000001 0051f5ed 00425e78 00514a2b
> 0041eb74 ffffffea 00000310 0051f5ed ffffffea ffffffea 00601f60 00000044
> 0102ff20 000e7a68 0051ab8e 004383b8 0051f5ed ffffffea 000000b8 00000007
> 01020c00 00000000 000e77f0 0041e5f0 005f67c0 0051f5ed 000000b6 0102fef4
> 00000310 0102fef4 00000000 00000016 005f676c 0060a34c 00000010 00000004
> 00000038 0000009a 01000000 000000b8 005f668e 0102e000 00001372 0102ff88
> Call Trace: [<00425e78>] dump_stack+0xc/0x10
> [<0041eb74>] panic+0xd8/0x26c
> [<000e7a68>] __kmem_cache_create_args+0x278/0x2e8
> [<000e77f0>] __kmem_cache_create_args+0x0/0x2e8
> [<0041e5f0>] memset+0x0/0x8c
> [<005f67c0>] io_uring_init+0x54/0xd2
>
> The minimal alignment of an integral type may differ from its size,
> hence is not safe to assume that an arbitrary freeptr_t (which is
> basically an unsigned long) is always aligned to 4 or 8 bytes.
>
> As nothing seems to require the additional alignment, it is safe to fix
> this by relaxing the check to the actual minimum alignment of freeptr_t.
>
> Fixes: aaa736b186239b7d ("io_uring: specify freeptr usage for SLAB_TYPESAFE_BY_RCU io_kiocb cache")
> Fixes: d345bd2e9834e2da ("mm: add kmem_cache_create_rcu()")
> Reported-by: Guenter Roeck <[email protected]>
> Closes: https://lore.kernel.org/[email protected]
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
Looks good to me,
Reviewed-by: Christian Brauner <[email protected]>
^ permalink raw reply [flat|nested] 12+ messages in thread