NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: kern/55663: Support for EVFILT_USER in kqueue(2)



The following reply was made to PR kern/55663; it has been noted by GNATS.

From: Ruslan Nikolaev <nruslan_devel%yahoo.com@localhost>
To: gnats-bugs%netbsd.org@localhost, kern-bug-people%netbsd.org@localhost,
 gnats-admin%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost, martin%duskware.de@localhost
Cc: 
Subject: Re: kern/55663: Support for EVFILT_USER in kqueue(2)
Date: Thu, 17 Sep 2020 12:29:15 -0400

 I attached a new patch below with changes per Martin's recommendations. 
 Please let me know if someone can review that.
 
 
 diff --git a/share/man/man9/kfilter_register.9 
 b/share/man/man9/kfilter_register.9
 index 43ba379b8c40..f405819140aa 100644
 --- a/share/man/man9/kfilter_register.9
 +++ b/share/man/man9/kfilter_register.9
 @@ -97,6 +97,7 @@ struct filterops {
   				/* called when knote is DELETEd */
   	int	(*f_event)(struct knote *kn, long hint);
   				/* called when event is triggered */
 +	void	(*f_touch)(struct knote *kn, struct kevent *kev, long hint);
   };
   .Ed
   .Pp
 diff --git a/sys/arch/arm/xscale/pxa2x0_apm.c 
 b/sys/arch/arm/xscale/pxa2x0_apm.c
 index f27b56901f82..869e392fb9f1 100644
 --- a/sys/arch/arm/xscale/pxa2x0_apm.c
 +++ b/sys/arch/arm/xscale/pxa2x0_apm.c
 @@ -104,8 +104,12 @@ void	filt_apmrdetach(struct knote *kn);
   int	filt_apmread(struct knote *kn, long hint);
   int	apmkqfilter(dev_t dev, struct knote *kn);
 
 -struct filterops apmread_filtops =
 -	{ 1, NULL, filt_apmrdetach, filt_apmread};
 +struct filterops apmread_filtops = {
 +	.f_isfd = 1,
 +	.f_attach = NULL,
 +	.f_detach = filt_apmrdetach,
 +	.f_event = filt_apmread,
 +};
   #endif
 
   /*
 diff --git a/sys/arch/macppc/dev/apm.c b/sys/arch/macppc/dev/apm.c
 index c96f2aac2b2d..ae8b15cffe5b 100644
 --- a/sys/arch/macppc/dev/apm.c
 +++ b/sys/arch/macppc/dev/apm.c
 @@ -431,8 +431,12 @@ filt_apmread(struct knote *kn, long hint)
   	return (kn->kn_data > 0);
   }
 
 -static struct filterops apmread_filtops =
 -	{ 1, NULL, filt_apmrdetach, filt_apmread};
 +static struct filterops apmread_filtops = {
 +	.f_isfd = 1,
 +	.f_attach = NULL,
 +	.f_detach = filt_apmrdetach,
 +	.f_event = filt_apmread,
 +};
 
   int
   apmkqfilter(dev_t dev, struct knote *kn)
 diff --git a/sys/arch/mips/ralink/ralink_gpio.c 
 b/sys/arch/mips/ralink/ralink_gpio.c
 index 127acd6b5991..a073029ad698 100644
 --- a/sys/arch/mips/ralink/ralink_gpio.c
 +++ b/sys/arch/mips/ralink/ralink_gpio.c
 @@ -495,10 +495,10 @@ static int  gpio_event_app_user_event(struct knote 
 *, long);
   static struct klist knotes;
   static int app_filter_id;
   static struct filterops app_fops = {
 -	0,
 -	gpio_event_app_user_attach,
 -	gpio_event_app_user_detach,
 -	gpio_event_app_user_event
 +	.f_isfd = 0,
 +	.f_attach = gpio_event_app_user_attach,
 +	.f_detach = gpio_event_app_user_detach,
 +	.f_event = gpio_event_app_user_event,
   };
   static struct callout led_tick_callout;
   static int gpio_driver_blink_leds = 1;
 diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c
 index 6ed5bec32e52..f8bb3968e84a 100644
 --- a/sys/kern/kern_event.c
 +++ b/sys/kern/kern_event.c
 @@ -31,6 +31,7 @@
 
   /*-
    * Copyright (c) 1999,2000,2001 Jonathan Lemon <jlemon%FreeBSD.org@localhost>
 + * Copyright (c) 2009 Apple, Inc
    * All rights reserved.
    *
    * Redistribution and use in source and binary forms, with or without
 @@ -109,6 +110,10 @@ static int	filt_timer(struct knote *, long hint);
   static int	filt_fsattach(struct knote *kn);
   static void	filt_fsdetach(struct knote *kn);
   static int	filt_fs(struct knote *kn, long hint);
 +static int	filt_userattach(struct knote *);
 +static void	filt_userdetach(struct knote *);
 +static int	filt_user(struct knote *, long hint);
 +static void	filt_usertouch(struct knote *, struct kevent *, long type);
 
   static const struct fileops kqueueops = {
   	.fo_name = "kqueue",
 @@ -128,6 +133,7 @@ static const struct filterops kqread_filtops = {
   	.f_attach = NULL,
   	.f_detach = filt_kqdetach,
   	.f_event = filt_kqueue,
 +	.f_touch = NULL,
   };
 
   static const struct filterops proc_filtops = {
 @@ -135,6 +141,7 @@ static const struct filterops proc_filtops = {
   	.f_attach = filt_procattach,
   	.f_detach = filt_procdetach,
   	.f_event = filt_proc,
 +	.f_touch = NULL,
   };
 
   static const struct filterops file_filtops = {
 @@ -142,6 +149,7 @@ static const struct filterops file_filtops = {
   	.f_attach = filt_fileattach,
   	.f_detach = NULL,
   	.f_event = NULL,
 +	.f_touch = NULL,
   };
 
   static const struct filterops timer_filtops = {
 @@ -149,6 +157,7 @@ static const struct filterops timer_filtops = {
   	.f_attach = filt_timerattach,
   	.f_detach = filt_timerdetach,
   	.f_event = filt_timer,
 +	.f_touch = NULL,
   };
 
   static const struct filterops fs_filtops = {
 @@ -156,6 +165,15 @@ static const struct filterops fs_filtops = {
   	.f_attach = filt_fsattach,
   	.f_detach = filt_fsdetach,
   	.f_event = filt_fs,
 +	.f_touch = NULL,
 +};
 +
 +static const struct filterops user_filtops = {
 +	.f_isfd = 0,
 +	.f_attach = filt_userattach,
 +	.f_detach = filt_userdetach,
 +	.f_event = filt_user,
 +	.f_touch = filt_usertouch,
   };
 
   static u_int	kq_ncallouts = 0;
 @@ -192,6 +210,7 @@ static struct kfilter sys_kfilters[] = {
   	{ "EVFILT_SIGNAL",	EVFILT_SIGNAL,	0, &sig_filtops, 0 },
   	{ "EVFILT_TIMER",	EVFILT_TIMER,	0, &timer_filtops, 0 },
   	{ "EVFILT_FS",		EVFILT_FS,	0, &fs_filtops, 0 },
 +	{ "EVFILT_USER",	EVFILT_USER,	0, &user_filtops, 0 },
   	{ NULL,			0,		0, NULL, 0 },
   };
 
 @@ -776,6 +795,106 @@ filt_fs(struct knote *kn, long hint)
   	return rv;
   }
 
 +static int
 +filt_userattach(struct knote *kn)
 +{
 +	struct kqueue *kq = kn->kn_kq;
 +
 +	/*
 +	 * EVFILT_USER knotes are not attached to anything in the kernel.
 +	 */
 +	mutex_spin_enter(&kq->kq_lock);
 +	kn->kn_hook = NULL;
 +	if (kn->kn_fflags & NOTE_TRIGGER)
 +		kn->kn_hookid = 1;
 +	else
 +		kn->kn_hookid = 0;
 +	mutex_spin_exit(&kq->kq_lock);
 +	return (0);
 +}
 +
 +static void
 +filt_userdetach(struct knote *kn)
 +{
 +
 +	/*
 +	 * EVFILT_USER knotes are not attached to anything in the kernel.
 +	 */
 +}
 +
 +static int
 +filt_user(struct knote *kn, long hint)
 +{
 +	struct kqueue *kq = kn->kn_kq;
 +	int hookid;
 +
 +	mutex_spin_enter(&kq->kq_lock);
 +	hookid = kn->kn_hookid;
 +	mutex_spin_exit(&kq->kq_lock);
 +
 +	return hookid;
 +}
 +
 +static void
 +filt_usertouch(struct knote *kn, struct kevent *kev, long type)
 +{
 +	struct kqueue *kq = kn->kn_kq;
 +	int ffctrl;
 +
 +	mutex_spin_enter(&kq->kq_lock);
 +	switch (type) {
 +	case EVENT_REGISTER:
 +		if (kev->fflags & NOTE_TRIGGER)
 +			kn->kn_hookid = 1;
 +
 +		ffctrl = kev->fflags & NOTE_FFCTRLMASK;
 +		kev->fflags &= NOTE_FFLAGSMASK;
 +		switch (ffctrl) {
 +		case NOTE_FFNOP:
 +			break;
 +
 +		case NOTE_FFAND:
 +			kn->kn_sfflags &= kev->fflags;
 +			break;
 +
 +		case NOTE_FFOR:
 +			kn->kn_sfflags |= kev->fflags;
 +			break;
 +
 +		case NOTE_FFCOPY:
 +			kn->kn_sfflags = kev->fflags;
 +			break;
 +
 +		default:
 +			/* XXX Return error? */
 +			break;
 +		}
 +		kn->kn_sdata = kev->data;
 +		if (kev->flags & EV_CLEAR) {
 +			kn->kn_hookid = 0;
 +			kn->kn_data = 0;
 +			kn->kn_fflags = 0;
 +		}
 +		break;
 +
 +	case EVENT_PROCESS:
 +		*kev = kn->kn_kevent;
 +		kev->fflags = kn->kn_sfflags;
 +		kev->data = kn->kn_sdata;
 +		if (kn->kn_flags & EV_CLEAR) {
 +			kn->kn_hookid = 0;
 +			kn->kn_data = 0;
 +			kn->kn_fflags = 0;
 +		}
 +		break;
 +
 +	default:
 +		panic("filt_usertouch() - invalid type (%ld)", type);
 +		break;
 +	}
 +	mutex_spin_exit(&kq->kq_lock);
 +}
 +
   /*
    * filt_seltrue:
    *
 @@ -809,6 +928,7 @@ const struct filterops seltrue_filtops = {
   	.f_attach = NULL,
   	.f_detach = filt_seltruedetach,
   	.f_event = filt_seltrue,
 +	.f_touch = NULL,
   };
 
   int
 @@ -1072,8 +1192,8 @@ kqueue_register(struct kqueue *kq, struct kevent *kev)
   	/*
   	 * kn now contains the matching knote, or NULL if no match
   	 */
 -	if (kev->flags & EV_ADD) {
 -		if (kn == NULL) {
 +	if (kn == NULL) {
 +		if (kev->flags & EV_ADD) {
   			/* create new knote */
   			kn = newkn;
   			newkn = NULL;
 @@ -1137,41 +1257,51 @@ kqueue_register(struct kqueue *kq, struct kevent 
 *kev)
   				goto done;
   			}
   			atomic_inc_uint(&kfilter->refcnt);
 +			goto done_ev_add;
   		} else {
 -			/*
 -			 * The user may change some filter values after the
 -			 * initial EV_ADD, but doing so will not reset any
 -			 * filter which have already been triggered.
 -			 */
 -			kn->kn_sfflags = kev->fflags;
 -			kn->kn_sdata = kev->data;
 -			kn->kn_kevent.udata = kev->udata;
 -		}
 -		/*
 -		 * We can get here if we are trying to attach
 -		 * an event to a file descriptor that does not
 -		 * support events, and the attach routine is
 -		 * broken and does not return an error.
 -		 */
 -		KASSERT(kn->kn_fop != NULL);
 -		KASSERT(kn->kn_fop->f_event != NULL);
 -		KERNEL_LOCK(1, NULL);			/* XXXSMP */
 -		rv = (*kn->kn_fop->f_event)(kn, 0);
 -		KERNEL_UNLOCK_ONE(NULL);		/* XXXSMP */
 -		if (rv)
 -			knote_activate(kn);
 -	} else {
 -		if (kn == NULL) {
 +			/* No matching knote and the EV_ADD flag is not set. */
   			error = ENOENT;
   			goto doneunlock;
   		}
 -		if (kev->flags & EV_DELETE) {
 -			/* knote_detach() drops fdp->fd_lock */
 -			knote_detach(kn, fdp, true);
 -			goto done;
 -		}
   	}
 
 +	if (kev->flags & EV_DELETE) {
 +		/* knote_detach() drops fdp->fd_lock */
 +		knote_detach(kn, fdp, true);
 +		goto done;
 +	}
 +
 +	/*
 +	 * The user may change some filter values after the
 +	 * initial EV_ADD, but doing so will not reset any
 +	 * filter which have already been triggered.
 +	 */
 +	kn->kn_kevent.udata = kev->udata;
 +	KASSERT(kn->kn_fop != NULL);
 +	if (!kn->kn_fop->f_isfd && kn->kn_fop->f_touch != NULL) {
 +		KERNEL_LOCK(1, NULL);		/* XXXSMP */
 +		(*kn->kn_fop->f_touch)(kn, kev, EVENT_REGISTER);
 +		KERNEL_UNLOCK_ONE(NULL);	/* XXXSMP */
 +	} else {
 +		kn->kn_sfflags = kev->fflags;
 +		kn->kn_sdata = kev->data;
 +	}
 +
 +	/*
 +	 * We can get here if we are trying to attach
 +	 * an event to a file descriptor that does not
 +	 * support events, and the attach routine is
 +	 * broken and does not return an error.
 +	 */
 +done_ev_add:
 +	KASSERT(kn->kn_fop != NULL);
 +	KASSERT(kn->kn_fop->f_event != NULL);
 +	KERNEL_LOCK(1, NULL);			/* XXXSMP */
 +	rv = (*kn->kn_fop->f_event)(kn, 0);
 +	KERNEL_UNLOCK_ONE(NULL);		/* XXXSMP */
 +	if (rv)
 +		knote_activate(kn);
 +
   	/* disable knote */
   	if ((kev->flags & EV_DISABLE)) {
   		mutex_spin_enter(&kq->kq_lock);
 @@ -1271,7 +1401,7 @@ kqueue_scan(file_t *fp, size_t maxevents, struct 
 kevent *ulistp,
   	struct timespec	ats, sleepts;
   	struct knote	*kn, *marker, morker;
   	size_t		count, nkev, nevents;
 -	int		timeout, error, rv;
 +	int		timeout, error, touch, rv;
   	filedesc_t	*fdp;
 
   	fdp = curlwp->l_fd;
 @@ -1382,8 +1512,20 @@ kqueue_scan(file_t *fp, size_t maxevents, struct 
 kevent *ulistp,
   					continue;
   				}
   			}
 +			KASSERT(kn->kn_fop != NULL);
 +			touch = (!kn->kn_fop->f_isfd &&
 +					kn->kn_fop->f_touch != NULL);
   			/* XXXAD should be got from f_event if !oneshot. */
 -			*kevp++ = kn->kn_kevent;
 +			if (touch) {
 +				mutex_spin_exit(&kq->kq_lock);
 +				KERNEL_LOCK(1, NULL);		/* XXXSMP */
 +				(*kn->kn_fop->f_touch)(kn, kevp, EVENT_PROCESS);
 +				KERNEL_UNLOCK_ONE(NULL);	/* XXXSMP */
 +				mutex_spin_enter(&kq->kq_lock);
 +			} else {
 +				*kevp = kn->kn_kevent;
 +			}
 +			kevp++;
   			nkev++;
   			if (kn->kn_flags & EV_ONESHOT) {
   				/* delete ONESHOT events after retrieval */
 @@ -1396,6 +1538,14 @@ kqueue_scan(file_t *fp, size_t maxevents, struct 
 kevent *ulistp,
   				/* clear state after retrieval */
   				kn->kn_data = 0;
   				kn->kn_fflags = 0;
 +				/*
 +				 * Manually clear knotes who weren't
 +				 * 'touch'ed.
 +				 */
 +				if (touch == 0) {
 +					kn->kn_data = 0;
 +					kn->kn_fflags = 0;
 +				}
   				kn->kn_status &= ~(KN_QUEUED|KN_ACTIVE|KN_BUSY);
   			} else if (kn->kn_flags & EV_DISPATCH) {
   				kn->kn_status |= KN_DISABLED;
 diff --git a/sys/net/if_tap.c b/sys/net/if_tap.c
 index d6505938e6c3..4176a57402c6 100644
 --- a/sys/net/if_tap.c
 +++ b/sys/net/if_tap.c
 @@ -1232,10 +1232,19 @@ tap_dev_poll(int unit, int events, struct lwp *l)
   	return revents;
   }
 
 -static struct filterops tap_read_filterops = { 1, NULL, tap_kqdetach,
 -	tap_kqread };
 -static struct filterops tap_seltrue_filterops = { 1, NULL, tap_kqdetach,
 -	filt_seltrue };
 +static struct filterops tap_read_filterops = {
 +	.f_isfd = 1,
 +	.f_attach = NULL,
 +	.f_detach = tap_kqdetach,
 +	.f_event = tap_kqread,
 +};
 +
 +static struct filterops tap_seltrue_filterops = {
 +	.f_isfd = 1,
 +	.f_attach = NULL,
 +	.f_detach = tap_kqdetach,
 +	.f_event = filt_seltrue,
 +};
 
   static int
   tap_cdev_kqfilter(dev_t dev, struct knote *kn)
 diff --git a/sys/sys/event.h b/sys/sys/event.h
 index dd18e51bb4ce..2473d7b220e3 100644
 --- a/sys/sys/event.h
 +++ b/sys/sys/event.h
 @@ -44,7 +44,8 @@
   #define	EVFILT_SIGNAL		5U	/* attached to struct proc */
   #define	EVFILT_TIMER		6U	/* arbitrary timer (in ms) */
   #define	EVFILT_FS		7U	/* filesystem events */
 -#define	EVFILT_SYSCOUNT		8U	/* number of filters */
 +#define	EVFILT_USER		8U	/* user events */
 +#define	EVFILT_SYSCOUNT		9U	/* number of filters */
 
   struct kevent {
   	uintptr_t	ident;		/* identifier for this event */
 @@ -90,6 +91,26 @@ _EV_SET(struct kevent *_kevp, uintptr_t _ident, 
 uint32_t _filter,
   #define	EV_EOF		0x8000U		/* EOF detected */
   #define	EV_ERROR	0x4000U		/* error, data contains errno */
 
 +/*
 + * data/hint flags/masks for EVFILT_USER, shared with userspace
 + *
 + * On input, the top two bits of fflags specifies how the lower twenty four
 + * bits should be applied to the stored value of fflags.
 + *
 + * On output, the top two bits will always be set to NOTE_FFNOP and the
 + * remaining twenty four bits will contain the stored fflags value.
 + */
 +#define	NOTE_FFNOP	0x00000000U		/* ignore input fflags */
 +#define	NOTE_FFAND	0x40000000U		/* AND fflags */
 +#define	NOTE_FFOR	0x80000000U		/* OR fflags */
 +#define	NOTE_FFCOPY	0xc0000000U		/* copy fflags */
 +
 +#define	NOTE_FFCTRLMASK	0xc0000000U		/* masks for operations */
 +#define	NOTE_FFLAGSMASK	0x00ffffffU
 +
 +#define	NOTE_TRIGGER	0x01000000U		/* Cause the event to be
 +						   triggered for output. */
 +
   /*
    * hint flag for in-kernel use - must not equal any existing note
    */
 @@ -161,6 +182,16 @@ struct kfilter_mapping {
    */
   #define	NOTE_SIGNAL	0x08000000U
 
 +/*
 + * Hint values for the optional f_touch event filter.  If f_touch is 
 not set
 + * to NULL and f_isfd is zero the f_touch filter will be called with 
 the type
 + * argument set to EVENT_REGISTER during a kevent() system call.  It is 
 also
 + * called under the same conditions with the type argument set to 
 EVENT_PROCESS
 + * when the event has been triggered.
 + */
 +#define	EVENT_REGISTER	1
 +#define	EVENT_PROCESS	2
 +
   /*
    * Callback methods for each filter type.
    */
 @@ -172,6 +203,7 @@ struct filterops {
   					/* called when knote is DELETEd */
   	int	(*f_event)	(struct knote *, long);
   					/* called when event is triggered */
 +	void	(*f_touch)	(struct knote *, struct kevent *, long);
   };
 
   /*
 @@ -196,6 +228,7 @@ struct knote {
   	const struct filterops	*kn_fop;
   	struct kfilter		*kn_kfilter;
   	void 			*kn_hook;
 +	int			kn_hookid;
 
   #define	KN_ACTIVE	0x01U			/* event has been triggered */
   #define	KN_QUEUED	0x02U			/* event is on queue */
 
 ------------------------------------------------------------------------------------------
 
 
 
 
 On 9/17/20 1:30 AM, Martin Husemann wrote:
 > The following reply was made to PR kern/55663; it has been noted by GNATS.
 > 
 > From: Martin Husemann <martin%duskware.de@localhost>
 > To: Ruslan Nikolaev <nruslan_devel%yahoo.com@localhost>
 > Cc: gnats-bugs%netbsd.org@localhost
 > Subject: Re: kern/55663: Support for EVFILT_USER in kqueue(2)
 > Date: Thu, 17 Sep 2020 07:25:10 +0200
 > 
 >   On Wed, Sep 16, 2020 at 04:09:36PM -0400, Ruslan Nikolaev wrote:
 >   > 1. Skip changes like you have shown, where f_touch is initialized to NULL,
 >   > assuming the default value for unspecified fields (= 0).
 >   >
 >   > 2. Change structs such as { 1, NULL, filt_apmrdetach, filt_apmread}
 >   > to {
 >   > 	.f_isfd = 1,
 >   > 	.f_detach = filt_apmrdetach,
 >   > 	.f_event = filt_apmread,
 >   > }
 >   
 >   Yes, exactly!
 >   
 >   Martin
 >   
 > 
 



Home | Main Index | Thread Index | Old Index