| Message ID | 20220930102712.789755-1-linux@rasmusvillemoes.dk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: slub: make slab_sysfs_init() a late_initcall | expand |
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.
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!
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 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)
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)
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(-)