On Sat, 12 Feb 2005 12:38:26 +0100 Arnd Bergmann wrote:
> On Freedag 11 Februar 2005 20:55, Nishanth Aravamudan wrote:
>
> > + * If the macro name contains:
> > + * lock, then @lock should be held before calling wait_event*().
> > + * It is released before sleeping and grabbed after
> > + * waking, saving the current IRQ mask in @flags. This lock
> > + * should also be held when changing any variables
> > + * affecting the condition and when waking up the process.
>
> Hmm, I see two problems with that approach:
>
> 1. It might lead to people not thinking about their locking order
> thoroughly if you introduce a sleeping function that is called with
> a spinlock held. Anyone relying on that lock introduces races because
> it actually is given up by the macro. I'd prefer it to be called
> without the lock and then have it acquire the lock only to check the
> condition, e.g:
>
> #define __wait_event_lock(wq, condition, lock, flags) \
> do { \
> DEFINE_WAIT(__wait); \
> \
> for (;;) { \
> prepare_to_wait(&wq, &__wait, TASK_UNINTERRUPTIBLE); \
> spin_lock_irqsave(lock, flags); \
> if (condition) \
> break; \
> spin_unlock_irqrestore(lock, flags); \
> schedule(); \
> } \
> spin_unlock_irqrestore(lock, flags); \
> finish_wait(&wq, &__wait); \
> } while (0)But in this case the result of testing the condition becomes useless after spin_unlock_irqrestore - someone might grab the lock and change things. Therefore the calling code would need to add a loop around wait_event_lock - and the wait_event_* macros were added precisely to encapsulate such a loop and avoid the need to code it manually.
pgpWr8tJ7VL0F.pgp
Description: PGP signature
