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)



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