lkml.org 
[lkml]   [2003]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] kernel/futex.c: Uneeded memory barrier
Date
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/

\
 
 \ /
  Last update: 2005-03-22 13:48    [from the cache]
©2003-2011 Jasper Spaans