v2.5.22 - add wait queue function callback support
PATCH 1/1 v2.5.22 - add wait queue function callback support
Hello Linus,
This patch against 2.5.22 adds support for wait queue function callbacks, which are used by aio to build async read / write operations on top of existing wait queues at points that would normally block a process. Comments?
-ben
Signature · 99 lines
"You will be reincarnated as a toad; and you will be much happier."
:r ~/patches/v2.5/v2.5.22-waitqueue-func.diff
diff -urN v2.5.22/include/linux/wait.h wq-func-v2.5.22.diff/include/linux/wait.h
--- v2.5.22/include/linux/wait.h Mon Jun 10 21:41:10 2002
+++ wq-func-v2.5.22.diff/include/linux/wait.h Mon Jun 17 15:53:25 2002
@@ -19,13 +19,16 @@
#include <asm/page.h>
#include <asm/processor.h>
+typedef struct __wait_queue wait_queue_t;
+typedef void (*wait_queue_func_t)(wait_queue_t *wait);
+
struct __wait_queue {
unsigned int flags;
#define WQ_FLAG_EXCLUSIVE 0x01
struct task_struct * task;
+ wait_queue_func_t func;
struct list_head task_list;
};
-typedef struct __wait_queue wait_queue_t;
struct __wait_queue_head {
spinlock_t lock;
@@ -40,6 +43,7 @@
#define __WAITQUEUE_INITIALIZER(name, tsk) { \
task: tsk, \
+ func: NULL, \
task_list: { NULL, NULL } }
#define DECLARE_WAITQUEUE(name, tsk) \
@@ -62,6 +66,15 @@
{
q->flags = 0;
q->task = p;
+ q->func = NULL;
+}
+
+static inline void init_waitqueue_func_entry(wait_queue_t *q,
+ wait_queue_func_t func)
+{
+ q->flags = 0;
+ q->task = NULL;
+ q->func = func;
}
static inline int waitqueue_active(wait_queue_head_t *q)
@@ -89,6 +102,22 @@
list_del(&old->task_list);
}
+#define add_wait_queue_cond(q, wait, cond) \
+ ({ \
+ unsigned long flags; \
+ int _raced = 0; \
+ wq_write_lock_irqsave(&(q)->lock, flags); \
+ (wait)->flags = 0; \
+ __add_wait_queue((q), (wait)); \
+ rmb(); \
+ if (!(cond)) { \
+ _raced = 1; \
+ __remove_wait_queue((q), (wait)); \
+ } \
+ wq_write_unlock_irqrestore(&(q)->lock, flags); \
+ _raced; \
+ })
+
#endif /* __KERNEL__ */
#endif
diff -urN v2.5.22/kernel/sched.c wq-func-v2.5.22.diff/kernel/sched.c
--- v2.5.22/kernel/sched.c Mon Jun 17 15:41:42 2002
+++ wq-func-v2.5.22.diff/kernel/sched.c Mon Jun 17 16:00:03 2002
@@ -916,13 +916,22 @@
*/
static inline void __wake_up_common(wait_queue_head_t *q, unsigned int mode, int nr_exclusive, int sync)
{
- struct list_head *tmp;
+ struct list_head *tmp, *next;
unsigned int state;
wait_queue_t *curr;
task_t *p;
- list_for_each(tmp, &q->task_list) {
+ list_for_each_safe(tmp, next, &q->task_list) {
+ wait_queue_func_t func;
curr = list_entry(tmp, wait_queue_t, task_list);
+ func = curr->func;
+ if (func) {
+ unsigned flags = curr->flags;
+ func(curr);
+ if ((flags & WQ_FLAG_EXCLUSIVE) && !--nr_exclusive)
+ break;
+ continue;
+ }
p = curr->task;
state = p->state;
if ((state & mode) && try_to_wake_up(p, sync) &&Review
Quoted 5 lines · Benjamin LaHaise
(quoted diff) +5
-0+#define add_wait_queue_cond(q, wait, cond) \ + ({ \ + unsigned long flags; \ + int _raced = 0; \ + wq_write_lock_irqsave(&(q)->lock, flags); \
I thought we killed off wq_write_lock_irqsave 1-2 kernels ago ?
Dave
Signature · 2 lines
| Dave Jones. http://www.codemonkey.org.uk | SuSE Labs
Quoted 8 lines · Dave Jones
Review
Quoted 5 lines · Benjamin LaHaise
(quoted diff) +5
-0+#define add_wait_queue_cond(q, wait, cond) \ + ({ \ + unsigned long flags; \ + int _raced = 0; \ + wq_write_lock_irqsave(&(q)->lock, flags); \I thought we killed off
wq_write_lock_irqsave1-2 kernels ago ?
Ah, I didn't notice that as I've not reached the point of merging everything into a working build -- I'd like to get some feedback and comments on the mergable units as they become available, since I'm rewriting a few portions to fix problems that experience with the code revealed. Anyways, the patch below changes add_wait_queue_cond to use spin_locks directly.
-ben
include/linux/wait.h +2 -2
@@ -106,7 +106,7 @@ ({ \ unsigned long flags; \ int _raced = 0; \ - wq_write_lock_irqsave(&(q)->lock, flags); \ + spin_lock_irqsave(&(q)->lock, flags); \ (wait)->flags = 0; \ __add_wait_queue((q), (wait)); \ rmb(); \
@@ -114,7 +114,7 @@ _raced =1 ; \__remove_wait_queue ((q), (wait)); \ } \ -wq_write_unlock_irqrestore (&(q)->lock, flags); \ +spin_lock_irqrestore (&(q)->lock, flags); \ _raced; \ })
Quoted 3 lines · Benjamin LaHaise
Anyways, the patch below changes
add_wait_queue_condto usespin_locksdirectly.
Can you do one more thing for me: change the "NULL" in the wake function to be a "default_wake_function()" that looks something like
int default_wake_function(task_t *p, unsigned int mode, int sync)
{
return (p->state & mode) && try_to_wake_up(p, sunc);
}and then you just make the code in __wake_up_common() do
list_for_each(tmp, &q->task_list) {
curr = list_entry(tmp, wait_queue_t, task_list);
p = curr->task;
if (curr->func(p, mode, sync) &&
((curr->flags & WQ_FLAG_EXCLUSIVE) && !--nr_exclusive))
break;
}instead of having separate code-paths for the "func" and the default wake-up action.
In short, the event "func" could never be NULL, and would always return whether the wakeup/event was "successful" from an exclusivity standpoint.
Linus
Quoted 2 lines · Linus Torvalds
In short, the event "func" could never be NULL, and would always return whether the wakeup/event was "successful" from an exclusivity standpoint.
How's the patch below? The main reason for passing in the pointer to the wait queue structure is that the aio functions need to remove themselves from the wait list if the event they were waiting for occurs. It seems to boot for me, how about others?
-ben
Signature · 123 lines
"You will be reincarnated as a toad; and you will be much happier."
diff -urN v2.5.22/include/linux/wait.h wq-func-v2.5.22-c.diff/include/linux/wait.h
--- v2.5.22/include/linux/wait.h Mon Jun 10 21:41:10 2002
+++ wq-func-v2.5.22-c.diff/include/linux/wait.h Mon Jun 17 17:34:09 2002
@@ -19,13 +19,17 @@
#include <asm/page.h>
#include <asm/processor.h>
+typedef struct __wait_queue wait_queue_t;
+typedef int (*wait_queue_func_t)(wait_queue_t *wait, unsigned mode, int sync);
+extern int default_wake_function(wait_queue_t *wait, unsigned mode, int sync);
+
struct __wait_queue {
unsigned int flags;
#define WQ_FLAG_EXCLUSIVE 0x01
struct task_struct * task;
+ wait_queue_func_t func;
struct list_head task_list;
};
-typedef struct __wait_queue wait_queue_t;
struct __wait_queue_head {
spinlock_t lock;
@@ -40,13 +44,14 @@
#define __WAITQUEUE_INITIALIZER(name, tsk) { \
task: tsk, \
+ func: default_wake_function, \
task_list: { NULL, NULL } }
#define DECLARE_WAITQUEUE(name, tsk) \
wait_queue_t name = __WAITQUEUE_INITIALIZER(name, tsk)
#define __WAIT_QUEUE_HEAD_INITIALIZER(name) { \
- lock: SPIN_LOCK_UNLOCKED, \
+ lock: SPIN_LOCK_UNLOCKED, \
task_list: { &(name).task_list, &(name).task_list } }
#define DECLARE_WAIT_QUEUE_HEAD(name) \
@@ -62,6 +67,15 @@
{
q->flags = 0;
q->task = p;
+ q->func = default_wake_function;
+}
+
+static inline void init_waitqueue_func_entry(wait_queue_t *q,
+ wait_queue_func_t func)
+{
+ q->flags = 0;
+ q->task = NULL;
+ q->func = func;
}
static inline int waitqueue_active(wait_queue_head_t *q)
@@ -89,6 +103,22 @@
list_del(&old->task_list);
}
+#define add_wait_queue_cond(q, wait, cond) \
+ ({ \
+ unsigned long flags; \
+ int _raced = 0; \
+ spin_lock_irqsave(&(q)->lock, flags); \
+ (wait)->flags = 0; \
+ __add_wait_queue((q), (wait)); \
+ rmb(); \
+ if (!(cond)) { \
+ _raced = 1; \
+ __remove_wait_queue((q), (wait)); \
+ } \
+ spin_lock_irqrestore(&(q)->lock, flags); \
+ _raced; \
+ })
+
#endif /* __KERNEL__ */
#endif
diff -urN v2.5.22/kernel/sched.c wq-func-v2.5.22-c.diff/kernel/sched.c
--- v2.5.22/kernel/sched.c Mon Jun 17 15:41:42 2002
+++ wq-func-v2.5.22-c.diff/kernel/sched.c Mon Jun 17 17:33:38 2002
@@ -905,6 +905,12 @@
}
#endif /* CONFIG_PREEMPT */
+int default_wake_function(wait_queue_t *curr, unsigned mode, int sync)
+{
+ task_t *p = curr->task;
+ return ((p->state & mode) && try_to_wake_up(p, sync));
+}
+
/*
* The core wakeup function. Non-exclusive wakeups (nr_exclusive == 0) just
* wake everything up. If it's an exclusive wakeup (nr_exclusive == small +ve
@@ -916,18 +922,17 @@
*/
static inline void __wake_up_common(wait_queue_head_t *q, unsigned int mode, int nr_exclusive, int sync)
{
- struct list_head *tmp;
- unsigned int state;
- wait_queue_t *curr;
- task_t *p;
+ struct list_head *tmp, *next;
- list_for_each(tmp, &q->task_list) {
+ list_for_each_safe(tmp, next, &q->task_list) {
+ wait_queue_t *curr;
+ unsigned flags;
curr = list_entry(tmp, wait_queue_t, task_list);
- p = curr->task;
- state = p->state;
- if ((state & mode) && try_to_wake_up(p, sync) &&
- ((curr->flags & WQ_FLAG_EXCLUSIVE) && !--nr_exclusive))
- break;
+ flags = curr->flags;
+ if (curr->func(curr, mode, sync) &&
+ (flags & WQ_FLAG_EXCLUSIVE) &&
+ !--nr_exclusive)
+ break;
}
}Quoted 5 lines · Benjamin LaHaise
How's the patch below? The main reason for passing in the pointer to the wait queue structure is that the aio functions need to remove themselves from the wait list if the event they were waiting for occurs. It seems to boot for me, how about others?
Looks ok at first glance, although I haven't booted yet.
One thing strikes me: we could move the "flags & WQ_FLAGS_EXCLUSIVE" test also into the wakeup function - making the "exclusivity" depend on which wakeup function you use. Does that make any sense? I'm not 100% convinced, but it would mean that the normal non-exclusive stuff would never even have to test the thing at run-time.
Linus
Quoted 9 lines · Dave Jones
Review
Quoted 5 lines · Benjamin LaHaise
(quoted diff) +5
-0+#define add_wait_queue_cond(q, wait, cond) \ + ({ \ + unsigned long flags; \ + int _raced = 0; \ + wq_write_lock_irqsave(&(q)->lock, flags); \I thought we killed off
wq_write_lock_irqsave1-2 kernels ago ?
Quoted 6 lines
Dave
Signature · 3 lines
| Dave Jones. http://www.codemonkey.org.uk | SuSE Labs -
Dave,
It depends on what you mean by killed off. I submitted a patch to Linus back at 2.5.3 to clean up the way the completion code called the wait queue interface. This interface got added then. You picked up those changes at that time (and still have them in your kernel tree) but the changes have never made it into Linus' tree.
So, Linus has never had the code to 'kill' and you've never dropped it after picking it up.
Signature · 2 lines
Bob Miller Email: [email protected] Open Source Development Lab Phone: 503.626.2455 Ext. 17
Quoted 9 lines · Bob Miller
Quoted 1 line
I thought we killed off
wq_write_lock_irqsave1-2 kernels ago ?It depends on what you mean by killed off. I submitted a patch to Linus back at 2.5.3 to clean up the way the completion code called the wait queue interface. This interface got added then. You picked up those changes at that time (and still have them in your kernel tree) but the changes have never made it into Linus' tree.
So, Linus has never had the code to 'kill' and you've never dropped it after picking it up.
Your patch was to use wq_write_lock and friends in sched.c iirc. That change is now removed from my tree (though I've not put up a version containing that change yet).
Since 2.5.20 or so, the wq_write_lock functions are dead as in gone. Not around, Extinct. They are ex-functions.
Dave
Signature · 2 lines
| Dave Jones. http://www.codemonkey.org.uk | SuSE Labs
Quoted 8 lines · Dave Jones
Your patch was to use
wq_write_lockand friends insched.ciirc. That change is now removed from my tree (though I've not put up a version containing that change yet).Since 2.5.20 or so, the
wq_write_lockfunctions are dead as in gone. Not around, Extinct. They are ex-functions.Dave
Always look before you leap. Just looked a 2.5.20 kernel and these are indeed gone. Sorry for the fire drill.
Signature · 2 lines
Bob Miller Email: [email protected] Open Source Development Lab Phone: 503.626.2455 Ext. 17
Quoted 8 lines · Bob Miller
It depends on what you mean by killed off. I submitted a patch to Linus back at 2.5.3 to clean up the way the completion code called the wait queue interface. This interface got added then. You picked up those changes at that time (and still have them in your kernel tree) but the changes have never made it into Linus' tree.
So, Linus has never had the code to 'kill' and you've never dropped it after picking it up.
Work has gone in since this.
During 2.5.20, Linus asked for and I submitted a patch to remove the whole wq_lock_t mess altogether. It was merged into 2.5.21. Subsequently, there is no wq_lock_t abstraction in current 2.5 kernels and code should use a standard spinlock.
Robert Love