Re: [PATCH] kernel/futex.c: Uneeded memory barrier

[Russell King <rmk@arm.linux.org.uk>]

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:

- needing to test for extra conditions to set "ret" accordingly (eg,
  non-blocking IO)
- needing to atomically dequeue some data

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.

-- 
Russell King (rmk@arm.linux.org.uk)	http://www.arm.linux.org.uk/personal/
Linux kernel maintainer of:
  2.6 ARM Linux   - http://www.arm.linux.org.uk/
  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
  2.6 Serial core