Messages in this thread | | | From | Rusty Russell <> | Subject | Re: [PATCH] kernel/futex.c: Uneeded memory barrier | Date | Tue, 16 Sep 2003 02:32:24 +1000 |
| |
In message <20030915102306.A22451@flint.arm.linux.org.uk> you write: > On Mon, Sep 15, 2003 at 01:41:30PM +1000, Rusty Russell wrote: > > ....hiding the subtlety in wrapper functions is the wrong approach. We > > have excellent wait_event, wait_event_interruptible and > > wait_event_interruptible_timeout macros in wait.h which these drivers > > should be using, which would make them simpler, less buggy and > > smaller. > > "smaller and simpler" hmm. And _more_ buggy. Let's take this case: > > add_wait_queue(&wq, &wait); > for (;;) { > set_current_state(TASK_INTERRUPTIBLE); > if (condition) > break; > if (file->f_flags & O_NONBLOCK) { > ret = -EAGAIN; > break; > } > if (signal_pending(current)) { > ret = -ERESTARTSYS; > break; > } > schedule(); > } > __set_current_state(TASK_RUNNING); > remove_wait_queue(&wq, &wait); > > There are cases like the above which make the wait_event*() macros > inappropriate:
There are, sure, but I'm not sure this is really one:
ret = 0; if (file->f_flags & O_NONBLOCK) { if (!condition) ret = -EAGAIN; } else wait_event_interruptible(&wq, condition, &ret);
AFAICT this is equivalent, and clearer?
> - needing to atomically dequeue some data
Definitely: the futex code is a good example of this. But the presence of spinlocks actually makes the barrier issue vanish anyway.
> I've yet to see anyone using wait_event*() in these circumstances - > they're great for your simple "did something happen" case which the > majority of drivers use, but there are use cases where wait_event*() > is not appropriate.
I don't mind someone doing something weird and needing to use lower primitives, but I still feel the wait_event* family are badly underused. Perhaps it's just lack of publicity...
Cheers! Rusty. -- Anyone who quotes me in their sig is an idiot. -- Rusty Russell. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |