Re: [RFC UPDATE PATCH] add wait_event_*_lock() functions and comments

[Arnd Bergmann <arnd@arndb.de>]

[-- Attachment #1: Type: text/plain, Size: 2119 bytes --]

On Sünnavend 12 Februar 2005 14:28, Sergey Vlasov wrote:
> On Sat, 12 Feb 2005 12:38:26 +0100 Arnd Bergmann wrote:
> > #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.

Ok, i understand now what the patch really wants to achieve. However,
I'm not convinced it's a good idea. In the usb/gadget/serial.c driver,
this appears to work only because an unconventional locking scheme is
used, i.e. there is an extra flag (port->port_in_use) that is set to
tell other functions about the state of the lock in case the lock holder
wants to sleep.

Is there any place in the kernel that would benefit of the 
wait_event_lock() macro family while using locks without such
special magic?

	Arnd <><

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]