From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: disestablish dohooks
To: Ted Unangst <tedu@tedunangst.com>
Cc: tech@openbsd.org
Date: Tue, 10 Jun 2025 10:51:47 +0200

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.
 
> Index: kern/kern_subr.c
> ===================================================================
> RCS file: /home/cvs/src/sys/kern/kern_subr.c,v
> diff -u -p -r1.52 kern_subr.c
> --- kern/kern_subr.c	31 Jan 2023 15:18:56 -0000	1.52
> +++ kern/kern_subr.c	10 Jun 2025 08:44:28 -0000
> @@ -262,8 +262,7 @@ struct hook_desc_head startuphook_list =
>      TAILQ_HEAD_INITIALIZER(startuphook_list);
>  
>  void *
> -hook_establish(struct hook_desc_head *head, int tail, void (*fn)(void *),
> -    void *arg)
> +startuphook_establish(void (*fn)(void *), void *arg)
>  {
>  	struct hook_desc *hdp;
>  
> @@ -273,53 +272,23 @@ hook_establish(struct hook_desc_head *he
>  
>  	hdp->hd_fn = fn;
>  	hdp->hd_arg = arg;
> -	if (tail)
> -		TAILQ_INSERT_TAIL(head, hdp, hd_list);
> -	else
> -		TAILQ_INSERT_HEAD(head, hdp, hd_list);
> +	TAILQ_INSERT_TAIL(&startuphook_list, hdp, hd_list);
>  
>  	return (hdp);
>  }
>  
> -void
> -hook_disestablish(struct hook_desc_head *head, void *vhook)
> -{
> -	struct hook_desc *hdp;
> -
> -#ifdef DIAGNOSTIC
> -	for (hdp = TAILQ_FIRST(head); hdp != NULL;
> -	    hdp = TAILQ_NEXT(hdp, hd_list))
> -                if (hdp == vhook)
> -			break;
> -	if (hdp == NULL)
> -		return;
> -#endif
> -	hdp = vhook;
> -	TAILQ_REMOVE(head, hdp, hd_list);
> -	free(hdp, M_DEVBUF, sizeof(*hdp));
> -}
> -
>  /*
>   * Run hooks.  Startup hooks are invoked right after scheduler_start but
> - * before root is mounted.  Shutdown hooks are invoked immediately before the
> - * system is halted or rebooted, i.e. after file systems unmounted,
> - * after crash dump done, etc.
> + * before root is mounted.
>   */
>  void
> -dohooks(struct hook_desc_head *head, int flags)
> +dostartuphooks(void)
>  {
> -	struct hook_desc *hdp, *hdp_temp;
> +	struct hook_desc *hdp;
>  
> -	if ((flags & HOOK_REMOVE) == 0) {
> -		TAILQ_FOREACH_SAFE(hdp, head, hd_list, hdp_temp) {
> -			(*hdp->hd_fn)(hdp->hd_arg);
> -		}
> -	} else {
> -		while ((hdp = TAILQ_FIRST(head)) != NULL) {
> -			TAILQ_REMOVE(head, hdp, hd_list);
> -			(*hdp->hd_fn)(hdp->hd_arg);
> -			if ((flags & HOOK_FREE) != 0)
> -				free(hdp, M_DEVBUF, sizeof(*hdp));
> -		}
> +	while ((hdp = TAILQ_FIRST(&startuphook_list)) != NULL) {
> +		TAILQ_REMOVE(&startuphook_list, hdp, hd_list);
> +		(*hdp->hd_fn)(hdp->hd_arg);
> +		free(hdp, M_DEVBUF, sizeof(*hdp));
>  	}
>  }
> Index: sys/systm.h
> ===================================================================
> RCS file: /home/cvs/src/sys/sys/systm.h,v
> diff -u -p -r1.173 systm.h
> --- sys/systm.h	3 Jun 2025 00:20:31 -0000	1.173
> +++ sys/systm.h	10 Jun 2025 08:42:55 -0000
> @@ -302,18 +302,8 @@ TAILQ_HEAD(hook_desc_head, hook_desc);
>  
>  extern struct hook_desc_head startuphook_list;
>  
> -void	*hook_establish(struct hook_desc_head *, int, void (*)(void *), void *);
> -void	hook_disestablish(struct hook_desc_head *, void *);
> -void	dohooks(struct hook_desc_head *, int);
> -
> -#define HOOK_REMOVE	0x01
> -#define HOOK_FREE	0x02
> -
> -#define startuphook_establish(fn, arg) \
> -	hook_establish(&startuphook_list, 1, (fn), (arg))
> -#define startuphook_disestablish(vhook) \
> -	hook_disestablish(&startuphook_list, (vhook))
> -#define dostartuphooks() dohooks(&startuphook_list, HOOK_REMOVE|HOOK_FREE)
> +void	*startuphook_establish(void (*)(void *), void *);
> +void	dostartuphooks(void);
>  
>  struct uio;
>  int	uiomove(void *, size_t, struct uio *);
> 

-- 
:wq Claudio