From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: move signal kqueue code to kern_event.c
To: tech@openbsd.org
Date: Mon, 29 Jul 2024 15:36:04 +0300

On Fri, Jul 26, 2024 at 12:17:24PM +0200, Claudio Jeker wrote:
> Move the signal related kqueue filters to kern_event.c.
> 
> Since proc and signal filters share the same klist it makes sense to keep
> them together. This is a preparation step towards making those filters
> MPSAFE.
> 

ok mvs

> -- 
> :wq Claudio
> 
> Index: kern/kern_event.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_event.c,v
> diff -u -p -r1.198 kern_event.c
> --- kern/kern_event.c	20 Aug 2023 15:13:43 -0000	1.198
> +++ kern/kern_event.c	26 Jul 2024 10:14:46 -0000
> @@ -124,6 +124,9 @@ int	filt_kqueue_common(struct knote *kn,
>  int	filt_procattach(struct knote *kn);
>  void	filt_procdetach(struct knote *kn);
>  int	filt_proc(struct knote *kn, long hint);
> +int	filt_sigattach(struct knote *kn);
> +void	filt_sigdetach(struct knote *kn);
> +int	filt_signal(struct knote *kn, long hint);
>  int	filt_fileattach(struct knote *kn);
>  void	filt_timerexpire(void *knx);
>  int	filt_timerattach(struct knote *kn);
> @@ -148,6 +151,13 @@ const struct filterops proc_filtops = {
>  	.f_event	= filt_proc,
>  };
>  
> +const struct filterops sig_filtops = {
> +	.f_flags	= 0,
> +	.f_attach	= filt_sigattach,
> +	.f_detach	= filt_sigdetach,
> +	.f_event	= filt_signal,
> +};
> +
>  const struct filterops file_filtops = {
>  	.f_flags	= FILTEROP_ISFD | FILTEROP_MPSAFE,
>  	.f_attach	= filt_fileattach,
> @@ -448,6 +458,55 @@ filt_proc(struct knote *kn, long hint)
>  	}
>  
>  	return (kn->kn_fflags != 0);
> +}
> +
> +/*
> + * signal knotes are shared with proc knotes, so we apply a mask to
> + * the hint in order to differentiate them from process hints.  This
> + * could be avoided by using a signal-specific knote list, but probably
> + * isn't worth the trouble.
> + */
> +int
> +filt_sigattach(struct knote *kn)
> +{
> +	struct process *pr = curproc->p_p;
> +	int s;
> +
> +	if (kn->kn_id >= NSIG)
> +		return EINVAL;
> +
> +	kn->kn_ptr.p_process = pr;
> +	kn->kn_flags |= EV_CLEAR;		/* automatically set */
> +
> +	s = splhigh();
> +	klist_insert_locked(&pr->ps_klist, kn);
> +	splx(s);
> +
> +	return (0);
> +}
> +
> +void
> +filt_sigdetach(struct knote *kn)
> +{
> +	struct process *pr = kn->kn_ptr.p_process;
> +	int s;
> +
> +	s = splhigh();
> +	klist_remove_locked(&pr->ps_klist, kn);
> +	splx(s);
> +}
> +
> +int
> +filt_signal(struct knote *kn, long hint)
> +{
> +
> +	if (hint & NOTE_SIGNAL) {
> +		hint &= ~NOTE_SIGNAL;
> +
> +		if (kn->kn_id == hint)
> +			kn->kn_data++;
> +	}
> +	return (kn->kn_data != 0);
>  }
>  
>  #define NOTE_TIMER_UNITMASK \
> Index: kern/kern_sig.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_sig.c,v
> diff -u -p -r1.334 kern_sig.c
> --- kern/kern_sig.c	24 Jul 2024 15:31:08 -0000	1.334
> +++ kern/kern_sig.c	26 Jul 2024 10:14:46 -0000
> @@ -70,17 +70,6 @@
>  
>  int nosuidcoredump = 1;
>  
> -int	filt_sigattach(struct knote *kn);
> -void	filt_sigdetach(struct knote *kn);
> -int	filt_signal(struct knote *kn, long hint);
> -
> -const struct filterops sig_filtops = {
> -	.f_flags	= 0,
> -	.f_attach	= filt_sigattach,
> -	.f_detach	= filt_sigdetach,
> -	.f_event	= filt_signal,
> -};
> -
>  /*
>   * The array below categorizes the signals and their default actions.
>   */
> @@ -1972,55 +1961,6 @@ initsiginfo(siginfo_t *si, int sig, u_lo
>  			break;
>  		}
>  	}
> -}
> -
> -int
> -filt_sigattach(struct knote *kn)
> -{
> -	struct process *pr = curproc->p_p;
> -	int s;
> -
> -	if (kn->kn_id >= NSIG)
> -		return EINVAL;
> -
> -	kn->kn_ptr.p_process = pr;
> -	kn->kn_flags |= EV_CLEAR;		/* automatically set */
> -
> -	s = splhigh();
> -	klist_insert_locked(&pr->ps_klist, kn);
> -	splx(s);
> -
> -	return (0);
> -}
> -
> -void
> -filt_sigdetach(struct knote *kn)
> -{
> -	struct process *pr = kn->kn_ptr.p_process;
> -	int s;
> -
> -	s = splhigh();
> -	klist_remove_locked(&pr->ps_klist, kn);
> -	splx(s);
> -}
> -
> -/*
> - * signal knotes are shared with proc knotes, so we apply a mask to
> - * the hint in order to differentiate them from process hints.  This
> - * could be avoided by using a signal-specific knote list, but probably
> - * isn't worth the trouble.
> - */
> -int
> -filt_signal(struct knote *kn, long hint)
> -{
> -
> -	if (hint & NOTE_SIGNAL) {
> -		hint &= ~NOTE_SIGNAL;
> -
> -		if (kn->kn_id == hint)
> -			kn->kn_data++;
> -	}
> -	return (kn->kn_data != 0);
>  }
>  
>  void
> Index: sys/event.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/event.h,v
> diff -u -p -r1.71 event.h
> --- sys/event.h	20 Aug 2023 15:13:43 -0000	1.71
> +++ sys/event.h	26 Jul 2024 10:14:46 -0000
> @@ -285,7 +285,6 @@ struct proc;
>  struct rwlock;
>  struct timespec;
>  
> -extern const struct filterops sig_filtops;
>  extern const struct filterops dead_filtops;
>  
>  extern void	kqpoll_init(unsigned int);
>