Subject: Re: timedwork
To: None <ad@netbsd.org>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-kern
Date: 01/15/2007 18:04:03
--NextPart-20070115175741-0112100
Content-Type: Text/Plain; charset=us-ascii

> I would prefer not to have a seperate callout_setwork(), but instead
> a flag to callout_init that can be ignored if we later do interrupts
> as threads:
> 
> 	callout_init(&foo, CALLOUT_THREAD);
> 
> We will need to add a CALLOUT_MPSAFE flag at some point.
> 
> Andrew

here's a patch.

i'm not sure if i want to change the prototype of callout_init() for this,
given the number of users of it, and both of CALLOUT_THREAD and
CALLOUT_MPSAFE seem somewhat transient.
(if we take a route of interrupt threads.)

YAMAMOTO Takashi

--NextPart-20070115175741-0112100
Content-Type: Text/Plain; charset=us-ascii
Content-Disposition: attachment; filename="a.diff"

Index: sys/callout.h
===================================================================
--- sys/callout.h	(revision 1464)
+++ sys/callout.h	(working copy)
@@ -68,6 +68,8 @@
 #ifndef _SYS_CALLOUT_H_
 #define _SYS_CALLOUT_H_
 
+#include <sys/workqueue.h>
+
 /*
  * The following funkyness is to appease gcc3's strict aliasing.
  */
@@ -90,25 +92,37 @@ struct callout_circq {
 #define	cq_prev_l	cq_prev.list
 
 struct callout {
-	struct callout_circq c_list;		/* linkage on queue */
+	union {
+		struct callout_circq u_list;	/* linkage on queue */
+		struct work u_work;
+	} c_u;
 	void	(*c_func)(void *);		/* function to call */
 	void	*c_arg;				/* function argument */
 	int	c_time;				/* when callout fires */
 	int	c_flags;			/* state of this entry */
 };
+#define	c_list	c_u.u_list
+#define	c_work	c_u.u_work
 
+#define	CALLOUT_THREAD		0x0001	/* invoked in a thread context. */
 #define	CALLOUT_PENDING		0x0002	/* callout is on the queue */
 #define	CALLOUT_FIRED		0x0004	/* callout has fired */
 #define	CALLOUT_INVOKING	0x0008	/* callout function is being invoked */
 
+#define	CALLOUT_INITIALIZER_SETFUNC_FLAGS(func, arg, flags)		\
+	{ .c_func = func, .c_arg = arg, .c_flags = flags }
+	
+
 #define	CALLOUT_INITIALIZER_SETFUNC(func, arg)				\
-				{ {{NULL}, {NULL}}, func, arg, 0, 0 }
+	CALLOUT_INITIALIZER_SETFUNC_FLAGS(func, arg, 0)
 
 #define	CALLOUT_INITIALIZER	CALLOUT_INITIALIZER_SETFUNC(NULL, NULL)
 
 #ifdef _KERNEL
 void	callout_startup(void);
+void	callout_thread_startup(void);
 void	callout_init(struct callout *);
+void	callout_init_flags(struct callout *, int);
 void	callout_setfunc(struct callout *, void (*)(void *), void *);
 void	callout_reset(struct callout *, int, void (*)(void *), void *);
 void	callout_schedule(struct callout *, int);
Index: kern/kern_timeout.c
===================================================================
--- kern/kern_timeout.c	(revision 1885)
+++ kern/kern_timeout.c	(working copy)
@@ -78,6 +78,8 @@ __KERNEL_RCSID(0, "$NetBSD: kern_timeout
 #include <sys/kernel.h>
 #include <sys/lock.h>
 #include <sys/callout.h>
+#include <sys/once.h>
+#include <sys/workqueue.h>
 
 #ifdef DDB
 #include <machine/db_machdep.h>
@@ -193,6 +195,29 @@ do {									\
 static struct evcnt callout_ev_late;
 #endif
 
+static struct workqueue *callout_workqueue;
+
+static void
+callout_worker(struct work *wk, void *arg)
+{
+	struct callout *c = (void *)wk;
+
+	KASSERT(&c->c_work == wk);
+	(*c->c_func)(c->c_arg);
+}
+
+void
+callout_thread_startup(void)
+{
+	int error;
+
+	error =  workqueue_create(&callout_workqueue, "callout",
+	    callout_worker, NULL, PUSER - 1, IPL_SOFTCLOCK, 0);
+	if (error) {
+		panic("%s: workqueue_create failed %d\n", __func__, error);
+	}
+}
+
 /*
  * callout_startup:
  *
@@ -227,6 +252,19 @@ callout_init(struct callout *c)
 }
 
 /*
+ * callout_init_flags:
+ *
+ *	Initialize a callout structure with flags.
+ */
+void
+callout_init_flags(struct callout *c, int flags)
+{
+
+	callout_init(c);
+	c->c_flags = flags;
+}
+
+/*
  * callout_reset:
  *
  *	Reset a callout structure with a new function and argument, and
@@ -380,12 +418,19 @@ softclock(void *v)
 			c->c_flags = (c->c_flags  & ~CALLOUT_PENDING) |
 			    (CALLOUT_FIRED|CALLOUT_INVOKING);
 
-			func = c->c_func;
-			arg = c->c_arg;
+			if ((c->c_flags & CALLOUT_THREAD) != 0) {
+				CALLOUT_UNLOCK(s);
+				workqueue_enqueue(callout_workqueue,
+				    &c->c_work);
+				CALLOUT_LOCK(s);
+			} else {
+				func = c->c_func;
+				arg = c->c_arg;
 
-			CALLOUT_UNLOCK(s);
-			(*func)(arg);
-			CALLOUT_LOCK(s);
+				CALLOUT_UNLOCK(s);
+				(*func)(arg);
+				CALLOUT_LOCK(s);
+			}
 		}
 	}
 
Index: kern/init_main.c
===================================================================
--- kern/init_main.c	(revision 1963)
+++ kern/init_main.c	(working copy)
@@ -431,6 +431,11 @@ main(void)
 		panic("fork init");
 
 	/*
+	 * Create a thread for CALLOUT_THREAD.
+	 */
+	callout_thread_startup();
+
+	/*
 	 * Create any kernel threads who's creation was deferred because
 	 * initproc had not yet been created.
 	 */

--NextPart-20070115175741-0112100--