Download raw body.
disestablish dohooks
> From: "Ted Unangst" <tedu@tedunangst.com>
> Date: Tue, 10 Jun 2025 16:15:50 -0400
>
> On 2025-06-10, Claudio Jeker wrote:
> > On Tue, Jun 10, 2025 at 04:48:04AM -0400, Ted Unangst wrote:
> > > Unless I've missed something, the only in use remnant of the hooks
> > > code is for startup hooks. And never disestablished.
> > >
> > > So we can burn through one layer of abstraction, and also eliminate
> > > one unused function entirely.
> > >
> > > There's also a collection of man.9 pages.
> >
> > We have various hooks all over the place reimplementing the same code.
> > Should we try to consolidate those instead. e.g. kthread_create_deferred
> > is a prime example for reimplementing the wheel for no good reason.
To repeat what I said to Claudio on icb: the startuphook stuff appears
to be an interface that was never widely adopted. Apart from some
crufty hppa code, only acpithinkpad(4) seems to use it.
That said, kthread_create_deferred() is a weird interface. Because
you still have to create your own kthread when your function gets
called...
> This turns kthread_create_deferred into a startuphook. Independent of
> previous diff. The functions appear to be used almost interchangably.
> The only difference is that kthread will run immediately post boot,
> while startuphook_establish called late will never run.
>
> I think this is a first step, then delete the kthread interface and
> add the "just run now" logic to startuphook_establish.
>
> The only ordering consideration is in acpi. I think we should move the
> thread creation there earlier, so it's running before any devices
> do stuff. (I think we should do this anyway, the acpi thread should be
> running before child threads, no?)
The startuphook_establish(9) man page clearly states that startup
hooks run immediately before the root and swap devices are configured.
Your diff makes it do something different. And since we have that
config_pending loop right before that matters. And that
config_pending loop is there to wait for work done in kthreads created
through kthread_create_deferred(). So that's a pretty hard NACK to
this diff.
Maybe Claudio meant to use the hooks infrastructure to create a new
hook for kthread_create_deferred() instead of it creating its own list?
> Index: kern/init_main.c
> ===================================================================
> RCS file: /home/cvs/src/sys/kern/init_main.c,v
> diff -u -p -r1.329 init_main.c
> --- kern/init_main.c 9 Jun 2025 10:57:46 -0000 1.329
> +++ kern/init_main.c 10 Jun 2025 19:56:10 -0000
> @@ -165,6 +165,7 @@ main(void *framep)
> struct pdevinit *pdev;
> extern struct pdevinit pdevinit[];
> extern void disk_init(void);
> + extern int kthread_create_now;
>
> /*
> * Initialize the current process pointer (curproc) before
> @@ -429,10 +430,12 @@ main(void *framep)
> }
>
> /*
> + * Run startup hooks.
> * Create any kernel threads whose creation was deferred because
> * initprocess had not yet been created.
> */
> - kthread_run_deferred_queue();
> + kthread_create_now = 1;
> + dostartuphooks();
>
> /*
> * Now that device driver threads have been created, wait for
> @@ -442,8 +445,6 @@ main(void *framep)
> */
> while (config_pending)
> tsleep_nsec(&config_pending, PWAIT, "cfpend", INFSLP);
> -
> - dostartuphooks();
>
> #if NVSCSI > 0
> config_rootfound("vscsi", NULL);
> Index: kern/kern_kthread.c
> ===================================================================
> RCS file: /home/cvs/src/sys/kern/kern_kthread.c,v
> diff -u -p -r1.46 kern_kthread.c
> --- kern/kern_kthread.c 26 Nov 2021 04:42:13 -0000 1.46
> +++ kern/kern_kthread.c 10 Jun 2025 19:53:30 -0000
> @@ -97,14 +97,6 @@ kthread_exit(int ecode)
> /* NOTREACHED */
> }
>
> -struct kthread_q {
> - SIMPLEQ_ENTRY(kthread_q) kq_q;
> - void (*kq_func)(void *);
> - void *kq_arg;
> -};
> -
> -SIMPLEQ_HEAD(, kthread_q) kthread_q = SIMPLEQ_HEAD_INITIALIZER(kthread_q);
> -
> /*
> * Defer the creation of a kernel thread. Once the standard kernel threads
> * and processes have been created, this queue will be run to callback to
> @@ -113,34 +105,9 @@ SIMPLEQ_HEAD(, kthread_q) kthread_q = SI
> void
> kthread_create_deferred(void (*func)(void *), void *arg)
> {
> - struct kthread_q *kq;
> -
> if (kthread_create_now) {
> (*func)(arg);
> return;
> }
> -
> - kq = malloc(sizeof *kq, M_TEMP, M_NOWAIT|M_ZERO);
> - if (kq == NULL)
> - panic("unable to allocate kthread_q");
> -
> - kq->kq_func = func;
> - kq->kq_arg = arg;
> -
> - SIMPLEQ_INSERT_TAIL(&kthread_q, kq, kq_q);
> -}
> -
> -void
> -kthread_run_deferred_queue(void)
> -{
> - struct kthread_q *kq;
> -
> - /* No longer need to defer kthread creation. */
> - kthread_create_now = 1;
> -
> - while ((kq = SIMPLEQ_FIRST(&kthread_q)) != NULL) {
> - SIMPLEQ_REMOVE_HEAD(&kthread_q, kq_q);
> - (*kq->kq_func)(kq->kq_arg);
> - free(kq, M_TEMP, sizeof(*kq));
> - }
> + startuphook_establish(func, arg);
> }
> Index: dev/acpi/acpi.c
> ===================================================================
> RCS file: /home/cvs/src/sys/dev/acpi/acpi.c,v
> diff -u -p -r1.448 acpi.c
> --- dev/acpi/acpi.c 2 Jun 2025 02:20:56 -0000 1.448
> +++ dev/acpi/acpi.c 10 Jun 2025 20:10:20 -0000
> @@ -1031,6 +1031,8 @@ acpi_attach_common(struct acpi_softc *sc
> "acpiwqpl", NULL);
> pool_setlowat(&acpiwqpool, 16);
>
> + kthread_create_deferred(acpi_create_thread, sc);
> +
> SIMPLEQ_INIT(&sc->sc_tables);
> SIMPLEQ_INIT(&sc->sc_wakedevs);
> #if NACPIPWRRES > 0
> @@ -1309,8 +1311,6 @@ acpi_attach_common(struct acpi_softc *sc
> pci_dopm = 1;
>
> acpi_attach_machdep(sc);
> -
> - kthread_create_deferred(acpi_create_thread, sc);
> }
>
> int
>
>
disestablish dohooks