wake_up_info() ...
Subsystems (4)
A message that is not in the archive (sent off-list or not yet received): [email protected]
It would seem better if info were a void *, to permit sending more than a single unsigned long.
Quoted 2 lines · John Gardiner Myers
It would seem better if info were a void *, to permit sending more than a single unsigned long.
It's fine for me. Linus, Manfred?
- Davide
Quoted 6 lines · Davide Libenzi
Quoted 2 lines · John Gardiner Myers
It would seem better if info were a void *, to permit sending more than a single unsigned long.
It's fine for me. Linus, Manfred?
This is the "void *" version. I slightly prefer the "void *" one.
- Davide
fs/eventpoll.c renamed from fs/eventpoll.c._orig +4 -2
@@ -306,7 +306,8 @@static void ep_unregister_pollwait (struct eventpoll *ep,struct epitem *epi);static int ep_unlink (struct eventpoll *ep,struct epitem *epi);static int ep_remove (struct eventpoll *ep,struct epitem *epi); -static int ep_poll_callback (wait_queue_t *wait,unsigned mode,int sync); +static int ep_poll_callback (wait_queue_t *wait,unsigned mode,int sync, +void *info);static int ep_eventpoll_close (struct inode *inode,struct file *file);static unsigned int ep_eventpoll_poll (struct file *file,poll_table *wait); static int ep_collect_ready_items(struct eventpoll *ep,
@@ -1293,7 +1294,8 @@* machanism. It is called by the stored file descriptors when they * have events to report. */ -static int ep_poll_callback (wait_queue_t *wait,unsigned mode,int sync) +static int ep_poll_callback (wait_queue_t *wait,unsigned mode,int sync, +void *info) {int pwake =0 ;unsigned long flags;
include/linux/wait.h renamed from include/linux/wait.h._orig +10 -3
@@ -17,8 +17,10 @@#include <asm/system.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); +typedef int (*wait_queue_func_t )(wait_queue_t *wait,unsigned mode,int sync, +void *info); +extern int default_wake_function (wait_queue_t *wait,unsigned mode,int sync, +void *info);struct __wait_queue {unsigned int flags;
@@ -107,6 +109,8 @@extern void FASTCALL (__wake_up (wait_queue_head_t *q,unsigned int mode,int nr));extern void FASTCALL (__wake_up_locked (wait_queue_head_t *q,unsigned int mode));extern void FASTCALL (__wake_up_sync (wait_queue_head_t *q,unsigned int mode,int nr)); +extern void FASTCALL (__wake_up_info (wait_queue_head_t *q,unsigned int mode,int nr, +void *info));#define wake_up (x) __wake_up((x),TASK_UNINTERRUPTIBLE | TASK_INTERRUPTIBLE, 1)#define wake_up_nr (x, nr) __wake_up((x),TASK_UNINTERRUPTIBLE | TASK_INTERRUPTIBLE, nr)
@@ -117,6 +121,8 @@#define wake_up_interruptible_all (x) __wake_up((x),TASK_INTERRUPTIBLE, 0)#define wake_up_locked (x) __wake_up_locked((x), TASK_UNINTERRUPTIBLE | TASK_INTERRUPTIBLE)#define wake_up_interruptible_sync (x) __wake_up_sync((x),TASK_INTERRUPTIBLE, 1) +#define wake_up_info (x, i) __wake_up_info((x),TASK_UNINTERRUPTIBLE | TASK_INTERRUPTIBLE, 1, (i)) +#define wake_up_all_info (x, i) __wake_up_info((x),TASK_UNINTERRUPTIBLE | TASK_INTERRUPTIBLE, 0, (i))#define __wait_event (wq, condition) \ do { \
@@ -240,7 +246,8 @@void FASTCALL (prepare_to_wait_exclusive (wait_queue_head_t *q,wait_queue_t *wait,int state));void FASTCALL (finish_wait (wait_queue_head_t *q,wait_queue_t *wait)); -int autoremove_wake_function (wait_queue_t *wait,unsigned mode,int sync); +int autoremove_wake_function (wait_queue_t *wait,unsigned mode,int sync, +void *info);#define DEFINE_WAIT (name) \ wait_queue_t name = { \
kernel/sched.c renamed from kernel/sched.c._orig +32 -9
95 lines not on this page · raw message
kernel/fork.c renamed from kernel/fork.c._orig +3 -2
@@ -194,9 +194,10 @@EXPORT_SYMBOL (finish_wait); -int autoremove_wake_function (wait_queue_t *wait,unsigned mode,int sync) +int autoremove_wake_function (wait_queue_t *wait,unsigned mode,int sync, +void *info) { -int ret =default_wake_function (wait, mode, sync); +int ret =default_wake_function (wait, mode, sync, info);if (ret)list_del_init (&wait->task_list);
Quoted 3 lines · John Gardiner Myers
It would seem better if info were a void *, to permit sending more than a single unsigned long.
The argument against that is that since there is basically no synchronization here, you can't pass a pointer to some random object. So by default, you should think of the cookie as "pass-by-value", ie not a pointer. That way there are no liveness issues: there is no issue about what happens to the data when the recipient is actually scheduled (possibly _much_ much after the actual wakeup).
Also, the forseeable actual use of this piece of data is purely integer:
things like the POLLIN | POLLOUT flags. There may never be any other use.
Also, passing in an "unsigned long" does not preclude using a data area for more complex cases: if the users do their own synchronization around the waitqueue on a higher level, and a pointer is a valid thing to use, you can cast that "unsigned long" to a pointer. This is very common kernel
usage: it may not be "portable" in the theoretical sense, but it's deeply embedded in the kernel that you can pass pointers as just bitpatters that fit in an "unsigned long".
The basic rule should be: don't build complex infrastructure. Build simple infrastructure that you can build complexity on top of if you ever need it. This was my reaction, and apparently Manfred Spraul reacted the same way.
Linus
Quoted 7 lines · Linus Torvalds
Quoted 3 lines · John Gardiner Myers
It would seem better if info were a void *, to permit sending more than a single unsigned long.
Quoted 6 lines
The argument against that is that since there is basically no synchronization here, you can't pass a pointer to some random object. So by default, you should think of the cookie as "pass-by-value", ie not a pointer. That way there are no liveness issues: there is no issue about what happens to the data when the recipient is actually scheduled (possibly _much_ much after the actual wakeup).
An argoument in favour of the "void *" would be that there might be situations were a little structure needs to be passed, that will be copied by the target of the wakeup (callback) in its own data structure. But as you said, we already do "unsigned long" -> "pointer" conversions inside the kernel, so it does not really matter. Let's stick with the unsigned long version then.
- Davide