Date:   Thu, 6 Oct 2022 15:06:30 +0900
From:   Hyeonggon Yoo <42.hyeyoo@...il.com>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     Christoph Lameter <cl@...ux.com>,
        Pekka Enberg <penberg@...nel.org>,
        David Rientjes <rientjes@...gle.com>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        Roman Gushchin <roman.gushchin@...ux.dev>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: slub: make slab_sysfs_init() a late_initcall

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@...il.com>

Thanks!

-- 
Thanks,
Hyeonggon

Powered by blists - more mailing lists