Add a flag that allows to disable merging per slab. This can be used for
more fine grained control over the caches or for debugging builds where
separate slabs can verify that no objects leak.
The slab_nomerge boot option is too coarse and would need to be enabled
on all testing hosts. There are some other ways how to disable merging,
e.g. a slab constructor but this disables poisoning besides that it adds
additional overhead. Other flags are internal and may have other
semantics.
A concrete example what motivates the flag. During 'btrfs balance' slab
top reported huge increase in caches like
1330095 1330095 100% 0.10K 34105 39 136420K Acpi-ParseExt
1734684 1734684 100% 0.14K 61953 28 247812K pid_namespace
8244036 6873075 83% 0.11K 229001 36 916004K khugepaged_mm_slot
which was confusing and that it's because of slab merging was not the
first idea. After rebooting with slab_nomerge all the caches were from
btrfs_ namespace as expected.
Signed-off-by: David Sterba <[email protected]>
---
include/linux/slab.h | 3 +++
mm/slab_common.c | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 6b3e155b70bf..06b94dfbce65 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -106,6 +106,9 @@
/* Avoid kmemleak tracing */
#define SLAB_NOLEAKTRACE ((slab_flags_t __force)0x00800000U)
+/* Don't merge slab */
+#define SLAB_NO_MERGE ((slab_flags_t __force)0x01000000U)
+
/* Fault injection mark */
#ifdef CONFIG_FAILSLAB
# define SLAB_FAILSLAB ((slab_flags_t __force)0x02000000U)
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 607249785c07..0e0a617eae7d 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -47,7 +47,7 @@ static DECLARE_WORK(slab_caches_to_rcu_destroy_work,
*/
#define SLAB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \
SLAB_TRACE | SLAB_TYPESAFE_BY_RCU | SLAB_NOLEAKTRACE | \
- SLAB_FAILSLAB | kasan_never_merge())
+ SLAB_FAILSLAB | SLAB_NO_MERGE | kasan_never_merge())
#define SLAB_MERGE_SAME (SLAB_RECLAIM_ACCOUNT | SLAB_CACHE_DMA | \
SLAB_CACHE_DMA32 | SLAB_ACCOUNT)
--
2.40.0
On Wed, May 24, 2023 at 12:17:48PM +0200, David Sterba wrote:
> Add a flag that allows to disable merging per slab. This can be used for
> more fine grained control over the caches or for debugging builds where
> separate slabs can verify that no objects leak.
> The slab_nomerge boot option is too coarse and would need to be enabled
> on all testing hosts.
Hello David,
There is no users nor interface to set this flag, I guess you're going
to use it by modifying source code, when debugging?
Does introducing new slub_debug option (i.e. slub_debug=N,pid_namespace)
work for your use case? (there are some boot-time slub_debug options described in
Documentation/mm/slub.rst)
> There are some other ways how to disable merging,
> e.g. a slab constructor but this disables poisoning besides that it adds
> additional overhead. Other flags are internal and may have other
> semantics.
>
> A concrete example what motivates the flag. During 'btrfs balance' slab
> top reported huge increase in caches like
>
> 1330095 1330095 100% 0.10K 34105 39 136420K Acpi-ParseExt
> 1734684 1734684 100% 0.14K 61953 28 247812K pid_namespace
> 8244036 6873075 83% 0.11K 229001 36 916004K khugepaged_mm_slot
>
> which was confusing and that it's because of slab merging was not the
> first idea. After rebooting with slab_nomerge all the caches were from
> btrfs_ namespace as expected.
>
> Signed-off-by: David Sterba <[email protected]>
> ---
> include/linux/slab.h | 3 +++
> mm/slab_common.c | 2 +-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/slab.h b/include/linux/slab.h
Thanks,
--
Hyeonggon Yoo
Doing kernel stuff as a hobby
Undergraduate | Chungnam National University
Dept. Computer Science & Engineering
On 5/24/23 14:56, Hyeonggon Yoo wrote:
> On Wed, May 24, 2023 at 12:17:48PM +0200, David Sterba wrote:
>> Add a flag that allows to disable merging per slab. This can be used for
>> more fine grained control over the caches or for debugging builds where
>> separate slabs can verify that no objects leak.
>> The slab_nomerge boot option is too coarse and would need to be enabled
>> on all testing hosts.
>
> Hello David,
>
> There is no users nor interface to set this flag, I guess you're going
> to use it by modifying source code, when debugging?
>
> Does introducing new slub_debug option (i.e. slub_debug=N,pid_namespace)
> work for your use case? (there are some boot-time slub_debug options described in
> Documentation/mm/slub.rst)
Yeah, it supports globbing so it would be e.g. slub_debug=N,btrfs*
That would deal with the "too coarse" aspect slab_nomerge. As for "need to
be enabled on all testing hosts", is it more convenient to deploy a debug
kernel build on them? Might be because you do that for other reasons
already? Just want to clarify.
BTW this was proposed as RFC few months ago but not pursued:
https://lore.kernel.org/all/167396280045.539803.7540459812377220500.stgit@firesoul/
I have no big objections, just wouldn't like to see its usage proliferate
unconditionally into non-debug builds.
>> There are some other ways how to disable merging,
>> e.g. a slab constructor but this disables poisoning besides that it adds
>> additional overhead. Other flags are internal and may have other
>> semantics.
>>
>> A concrete example what motivates the flag. During 'btrfs balance' slab
>> top reported huge increase in caches like
>>
>> 1330095 1330095 100% 0.10K 34105 39 136420K Acpi-ParseExt
>> 1734684 1734684 100% 0.14K 61953 28 247812K pid_namespace
>> 8244036 6873075 83% 0.11K 229001 36 916004K khugepaged_mm_slot
>>
>> which was confusing and that it's because of slab merging was not the
>> first idea. After rebooting with slab_nomerge all the caches were from
>> btrfs_ namespace as expected.
>>
>> Signed-off-by: David Sterba <[email protected]>
>> ---
>> include/linux/slab.h | 3 +++
>> mm/slab_common.c | 2 +-
>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/slab.h b/include/linux/slab.h
>
> Thanks,
>
On Wed, May 24, 2023 at 09:56:02PM +0900, Hyeonggon Yoo wrote:
> On Wed, May 24, 2023 at 12:17:48PM +0200, David Sterba wrote:
> > Add a flag that allows to disable merging per slab. This can be used for
> > more fine grained control over the caches or for debugging builds where
> > separate slabs can verify that no objects leak.
> > The slab_nomerge boot option is too coarse and would need to be enabled
> > on all testing hosts.
>
> There is no users nor interface to set this flag, I guess you're going
> to use it by modifying source code, when debugging?
An example usage
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -12,6 +12,12 @@
#include "async-thread.h"
#include "block-rsv.h"
+#ifdef CONFIG_BTRFS_DEBUG
+#define SLAB_DEBUG_NO_MERGE 0
+#else
+#define SLAB_DEBUG_NO_MERGE SLAB_NO_MERGE
+#endif
+
#define BTRFS_MAX_EXTENT_SIZE SZ_128M
#define BTRFS_OLDEST_GENERATION 0ULL
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -5049,7 +5049,7 @@ int __init btrfs_ctree_init(void)
{
btrfs_path_cachep = kmem_cache_create("btrfs_path",
sizeof(struct btrfs_path), 0,
- SLAB_MEM_SPREAD, NULL);
+ SLAB_MEM_SPREAD | SLAB_DEBUG_NO_MERGE, NULL);
if (!btrfs_path_cachep)
return -ENOMEM;
return 0;
---
and this will be a permanent change, not added as needed.
> Does introducing new slub_debug option (i.e. slub_debug=N,pid_namespace)
> work for your use case? (there are some boot-time slub_debug options described in
> Documentation/mm/slub.rst)
I'd like to keep boot parameters unchanged, the testing setups are
different, physical, local VM, hosted. For the same reason the config
option CONFIG_SLUB_DEBUG_ON=y is very convenient.
On Wed, May 24, 2023 at 03:53:11PM +0200, Vlastimil Babka wrote:
> On 5/24/23 14:56, Hyeonggon Yoo wrote:
> > On Wed, May 24, 2023 at 12:17:48PM +0200, David Sterba wrote:
> > work for your use case? (there are some boot-time slub_debug options described in
> > Documentation/mm/slub.rst)
>
> Yeah, it supports globbing so it would be e.g. slub_debug=N,btrfs*
> That would deal with the "too coarse" aspect slab_nomerge. As for "need to
> be enabled on all testing hosts", is it more convenient to deploy a debug
> kernel build on them? Might be because you do that for other reasons
> already? Just want to clarify.
Yeah, agreed.
> BTW this was proposed as RFC few months ago but not pursued:
> https://lore.kernel.org/all/167396280045.539803.7540459812377220500.stgit@firesoul/
>
> I have no big objections, just wouldn't like to see its usage proliferate
> unconditionally into non-debug builds.
It would be fine for me to make it conditionally available, e.g.
depending on SLUB_DEBUG.