Currently, slab_sysfs_init() is an __initcall aka device_initcall. It
is rather time-consuming; on my board it takes around 11ms. That's
about 1% of the time budget I have from U-Boot letting go and until
linux must assume responsibility of keeping the external watchdog
happy.

There's no particular reason this would need to run at device_initcall
time, so instead make it a late_initcall to allow vital functionality
to get started a bit sooner.

This actually ends up winning more than just those 11ms, because the
slab caches that get created during other device_initcalls (and before
my watchdog device gets probed) now don't end up doing the somewhat
expensive sysfs_slab_add() themselves. Some example lines (with
initcall_debug set) before/after:

initcall ext4_init_fs+0x0/0x1ac returned 0 after 1386 usecs
initcall journal_init+0x0/0x138 returned 0 after 517 usecs
initcall init_fat_fs+0x0/0x68 returned 0 after 294 usecs

initcall ext4_init_fs+0x0/0x1ac returned 0 after 240 usecs
initcall journal_init+0x0/0x138 returned 0 after 32 usecs
initcall init_fat_fs+0x0/0x68 returned 0 after 18 usecs

Altogether, this means I now get to petting the watchdog around 17ms
sooner. [Of course, the time the other initcalls save is instead spent
in slab_sysfs_init(), which goes from 11ms to 16ms, so there's no
overall change in boot time.]

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---

The numbers certainly suggest that someone might want to look into
making sysfs/kobject/kset perform better. But that would be way more
complicated than this patch, and could not possibly achieve the same
win as getting the sysfs_slab_add() overhead completely out of the
way.


 mm/slub.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 4b98dff9be8e..dade5c84a7bb 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -6070,8 +6070,7 @@ static int __init slab_sysfs_init(void)
 	mutex_unlock(&slab_mutex);
 	return 0;
 }
-
-__initcall(slab_sysfs_init);
+late_initcall(slab_sysfs_init);
 #endif /* CONFIG_SYSFS */
 
 #if defined(CONFIG_SLUB_DEBUG) && defined(CONFIG_DEBUG_FS)
-- 
2.37.2

On 9/30/22 12:27, Rasmus Villemoes wrote:
> Currently, slab_sysfs_init() is an __initcall aka device_initcall. It
> is rather time-consuming; on my board it takes around 11ms. That's
> about 1% of the time budget I have from U-Boot letting go and until
> linux must assume responsibility of keeping the external watchdog
> happy.
> 
> There's no particular reason this would need to run at device_initcall
> time, so instead make it a late_initcall to allow vital functionality
> to get started a bit sooner.
> 
> This actually ends up winning more than just those 11ms, because the
> slab caches that get created during other device_initcalls (and before
> my watchdog device gets probed) now don't end up doing the somewhat
> expensive sysfs_slab_add() themselves. Some example lines (with
> initcall_debug set) before/after:
> 
> initcall ext4_init_fs+0x0/0x1ac returned 0 after 1386 usecs
> initcall journal_init+0x0/0x138 returned 0 after 517 usecs
> initcall init_fat_fs+0x0/0x68 returned 0 after 294 usecs
> 
> initcall ext4_init_fs+0x0/0x1ac returned 0 after 240 usecs
> initcall journal_init+0x0/0x138 returned 0 after 32 usecs
> initcall init_fat_fs+0x0/0x68 returned 0 after 18 usecs
> 
> Altogether, this means I now get to petting the watchdog around 17ms
> sooner. [Of course, the time the other initcalls save is instead spent
> in slab_sysfs_init(), which goes from 11ms to 16ms, so there's no
> overall change in boot time.]
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

Thanks, added to slab.git for-6.2/slub-sysfs

> ---
> 
> The numbers certainly suggest that someone might want to look into
> making sysfs/kobject/kset perform better. But that would be way more
> complicated than this patch, and could not possibly achieve the same
> win as getting the sysfs_slab_add() overhead completely out of the
> way.
> 
> 
>  mm/slub.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 4b98dff9be8e..dade5c84a7bb 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -6070,8 +6070,7 @@ static int __init slab_sysfs_init(void)
>  	mutex_unlock(&slab_mutex);
>  	return 0;
>  }
> -
> -__initcall(slab_sysfs_init);
> +late_initcall(slab_sysfs_init);
>  #endif /* CONFIG_SYSFS */
>  
>  #if defined(CONFIG_SLUB_DEBUG) && defined(CONFIG_DEBUG_FS)

On Fri, 30 Sep 2022, Rasmus Villemoes wrote:

> Currently, slab_sysfs_init() is an __initcall aka device_initcall. It
> is rather time-consuming; on my board it takes around 11ms. That's
> about 1% of the time budget I have from U-Boot letting go and until
> linux must assume responsibility of keeping the external watchdog
> happy.
> 
> There's no particular reason this would need to run at device_initcall
> time, so instead make it a late_initcall to allow vital functionality
> to get started a bit sooner.
> 
> This actually ends up winning more than just those 11ms, because the
> slab caches that get created during other device_initcalls (and before
> my watchdog device gets probed) now don't end up doing the somewhat
> expensive sysfs_slab_add() themselves. Some example lines (with
> initcall_debug set) before/after:
> 
> initcall ext4_init_fs+0x0/0x1ac returned 0 after 1386 usecs
> initcall journal_init+0x0/0x138 returned 0 after 517 usecs
> initcall init_fat_fs+0x0/0x68 returned 0 after 294 usecs
> 
> initcall ext4_init_fs+0x0/0x1ac returned 0 after 240 usecs
> initcall journal_init+0x0/0x138 returned 0 after 32 usecs
> initcall init_fat_fs+0x0/0x68 returned 0 after 18 usecs
> 
> Altogether, this means I now get to petting the watchdog around 17ms
> sooner. [Of course, the time the other initcalls save is instead spent
> in slab_sysfs_init(), which goes from 11ms to 16ms, so there's no
> overall change in boot time.]
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

Acked-by: David Rientjes <rientjes@google.com>

On Fri, Sep 30, 2022 at 12:27:12PM +0200, Rasmus Villemoes wrote:
> Currently, slab_sysfs_init() is an __initcall aka device_initcall. It
> is rather time-consuming; on my board it takes around 11ms. That's
> about 1% of the time budget I have from U-Boot letting go and until
> linux must assume responsibility of keeping the external watchdog
> happy.
> 
> There's no particular reason this would need to run at device_initcall
> time, so instead make it a late_initcall to allow vital functionality
> to get started a bit sooner.
> 
> This actually ends up winning more than just those 11ms, because the
> slab caches that get created during other device_initcalls (and before
> my watchdog device gets probed) now don't end up doing the somewhat
> expensive sysfs_slab_add() themselves. Some example lines (with
> initcall_debug set) before/after:
> 
> initcall ext4_init_fs+0x0/0x1ac returned 0 after 1386 usecs
> initcall journal_init+0x0/0x138 returned 0 after 517 usecs
> initcall init_fat_fs+0x0/0x68 returned 0 after 294 usecs
> 
> initcall ext4_init_fs+0x0/0x1ac returned 0 after 240 usecs
> initcall journal_init+0x0/0x138 returned 0 after 32 usecs
> initcall init_fat_fs+0x0/0x68 returned 0 after 18 usecs
> 
> Altogether, this means I now get to petting the watchdog around 17ms
> sooner. [Of course, the time the other initcalls save is instead spent
> in slab_sysfs_init(), which goes from 11ms to 16ms, so there's no
> overall change in boot time.]

This looks okay and just curious,
can you explain what kind of benefit does enabling watchdog early provides?

> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> 
> The numbers certainly suggest that someone might want to look into
> making sysfs/kobject/kset perform better. But that would be way more
> complicated than this patch, and could not possibly achieve the same
> win as getting the sysfs_slab_add() overhead completely out of the
> way.
> 
> 
>  mm/slub.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 4b98dff9be8e..dade5c84a7bb 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -6070,8 +6070,7 @@ static int __init slab_sysfs_init(void)
>  	mutex_unlock(&slab_mutex);
>  	return 0;
>  }
> -
> -__initcall(slab_sysfs_init);
> +late_initcall(slab_sysfs_init);
>  #endif /* CONFIG_SYSFS */
>  
>  #if defined(CONFIG_SLUB_DEBUG) && defined(CONFIG_DEBUG_FS)
> -- 
> 2.37.2

This is only deferring slub's sysfs initialization step (still before init process) 
So IIUC it shouldn't be serious.

-- 
Thanks,
Hyeonggon

On 03/10/2022 10.17, Hyeonggon Yoo wrote:
> On Fri, Sep 30, 2022 at 12:27:12PM +0200, Rasmus Villemoes wrote:
>> Currently, slab_sysfs_init() is an __initcall aka device_initcall. It
>> is rather time-consuming; on my board it takes around 11ms. That's
>> about 1% of the time budget I have from U-Boot letting go and until
>> linux must assume responsibility of keeping the external watchdog
>> happy.
>>
>> There's no particular reason this would need to run at device_initcall
>> time, so instead make it a late_initcall to allow vital functionality
>> to get started a bit sooner.
>>
>> This actually ends up winning more than just those 11ms, because the
>> slab caches that get created during other device_initcalls (and before
>> my watchdog device gets probed) now don't end up doing the somewhat
>> expensive sysfs_slab_add() themselves. Some example lines (with
>> initcall_debug set) before/after:
>>
>> initcall ext4_init_fs+0x0/0x1ac returned 0 after 1386 usecs
>> initcall journal_init+0x0/0x138 returned 0 after 517 usecs
>> initcall init_fat_fs+0x0/0x68 returned 0 after 294 usecs
>>
>> initcall ext4_init_fs+0x0/0x1ac returned 0 after 240 usecs
>> initcall journal_init+0x0/0x138 returned 0 after 32 usecs
>> initcall init_fat_fs+0x0/0x68 returned 0 after 18 usecs
>>
>> Altogether, this means I now get to petting the watchdog around 17ms
>> sooner. [Of course, the time the other initcalls save is instead spent
>> in slab_sysfs_init(), which goes from 11ms to 16ms, so there's no
>> overall change in boot time.]
> 
> This looks okay and just curious,
> can you explain what kind of benefit does enabling watchdog early provides?

The watchdog is _always_ enabled, from power-on onwards. There's nothing
one can do to disable it (short of using a soldering iron to modify the
board...), and usually nothing one can do to program its timeout [if it
is at all configurable, it's done during board design using appropriate
resistor/capacitor values].

All the custom boards I've met, across the very different industries
I've worked with, have always had such an external watchdog. Their
timing requirements may vary; currently I'm working on a board which has
a 1s margin, but I've also encountered something as low as (IIRC) 400ms.

While 10-20ms may not sound impressive, this is not the first nor the
last patch I'm trying to get upstream (see e7cb072eb988 for another
example, done in connection with another project) to gain as much margin
as possible - we want to be able to continue to upgrade our kernels for
the next 5, 10, 20 years, and undoubtedly the mainline kernel will grow
features and overhead in that timespan which won't be offset by better
compilers.

Rasmus

On Mon, Oct 03, 2022 at 12:25:26PM +0200, Rasmus Villemoes wrote:
> On 03/10/2022 10.17, Hyeonggon Yoo wrote:
> > On Fri, Sep 30, 2022 at 12:27:12PM +0200, Rasmus Villemoes wrote:
> >> Currently, slab_sysfs_init() is an __initcall aka device_initcall. It
> >> is rather time-consuming; on my board it takes around 11ms. That's
> >> about 1% of the time budget I have from U-Boot letting go and until
> >> linux must assume responsibility of keeping the external watchdog
> >> happy.
> >>
> >> There's no particular reason this would need to run at device_initcall
> >> time, so instead make it a late_initcall to allow vital functionality
> >> to get started a bit sooner.
> >>
> >> This actually ends up winning more than just those 11ms, because the
> >> slab caches that get created during other device_initcalls (and before
> >> my watchdog device gets probed) now don't end up doing the somewhat
> >> expensive sysfs_slab_add() themselves. Some example lines (with
> >> initcall_debug set) before/after:
> >>
> >> initcall ext4_init_fs+0x0/0x1ac returned 0 after 1386 usecs
> >> initcall journal_init+0x0/0x138 returned 0 after 517 usecs
> >> initcall init_fat_fs+0x0/0x68 returned 0 after 294 usecs
> >>
> >> initcall ext4_init_fs+0x0/0x1ac returned 0 after 240 usecs
> >> initcall journal_init+0x0/0x138 returned 0 after 32 usecs
> >> initcall init_fat_fs+0x0/0x68 returned 0 after 18 usecs
> >>
> >> Altogether, this means I now get to petting the watchdog around 17ms
> >> sooner. [Of course, the time the other initcalls save is instead spent
> >> in slab_sysfs_init(), which goes from 11ms to 16ms, so there's no
> >> overall change in boot time.]
> > 
> > This looks okay and just curious,
> > can you explain what kind of benefit does enabling watchdog early provides?
> 
> The watchdog is _always_ enabled, from power-on onwards. There's nothing
> one can do to disable it (short of using a soldering iron to modify the
> board...), and usually nothing one can do to program its timeout [if it
> is at all configurable, it's done during board design using appropriate
> resistor/capacitor values].
> 
> All the custom boards I've met, across the very different industries
> I've worked with, have always had such an external watchdog. Their
> timing requirements may vary; currently I'm working on a board which has
> a 1s margin, but I've also encountered something as low as (IIRC) 400ms.
> 
> While 10-20ms may not sound impressive, this is not the first nor the
> last patch I'm trying to get upstream (see e7cb072eb988 for another
> example, done in connection with another project) to gain as much margin
> as possible - we want to be able to continue to upgrade our kernels for
> the next 5, 10, 20 years, and undoubtedly the mainline kernel will grow
> features and overhead in that timespan which won't be offset by better
> compilers.
> 
> Rasmus

Thank you for such a detailed explanation.
Now I get your motivation for this.

To best of my knowledge 1) it is not increasing boot time significantly as
it is just postponing slab_sysfs_init(), and 2) it is still before init
process so some systemd scripts interacting with SLUB sysfs interface
ran just after boot won't be broken (and at least on my machine it didn't).

Thus..

Acked-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>

Thanks!

-- 
Thanks,
Hyeonggon