Messages in this thread |  | | Date | Thu, 12 Dec 2013 11:17:48 -0500 | From | Paul Gortmaker <> | Subject | Re: [PATCH 1/3] wait-simple: Introduce the simple waitqueue implementation |
| |
On 13-12-12 06:18 AM, Peter Zijlstra wrote: > On Wed, Dec 11, 2013 at 08:06:37PM -0500, Paul Gortmaker wrote: >> +/* >> + * Event API >> + */ >> +#define __swait_event(wq, condition) \ >> +do { \ >> + DEFINE_SWAITER(__wait); \ >> + \ >> + for (;;) { \ >> + swait_prepare(&wq, &__wait, TASK_UNINTERRUPTIBLE); \ >> + if (condition) \ >> + break; \ >> + schedule(); \ >> + } \ >> + swait_finish(&wq, &__wait); \ >> +} while (0) >> + >> +#define __swait_event_interruptible(wq, condition, ret) \ >> +do { \ >> + DEFINE_SWAITER(__wait); \ >> + \ >> + for (;;) { \ >> + swait_prepare(&wq, &__wait, TASK_INTERRUPTIBLE); \ >> + if (condition) \ >> + break; \ >> + if (signal_pending(current)) { \ >> + ret = -ERESTARTSYS; \ >> + break; \ >> + } \ >> + schedule(); \ >> + } \ >> + swait_finish(&wq, &__wait); \ >> +} while (0) >> + >> +#define __swait_event_interruptible_timeout(wq, condition, ret) \ >> +do { \ >> + DEFINE_SWAITER(__wait); \ >> + \ >> + for (;;) { \ >> + swait_prepare(&wq, &__wait, TASK_INTERRUPTIBLE); \ >> + if (condition) \ >> + break; \ >> + if (signal_pending(current)) { \ >> + ret = -ERESTARTSYS; \ >> + break; \ >> + } \ >> + ret = schedule_timeout(ret); \ >> + if (!ret) \ >> + break; \ >> + } \ >> + swait_finish(&wq, &__wait); \ >> +} while (0) > > Urgh, please have a look at ___wait_event() we just killed all the > pointless replication for the normal waitqueues, please don't add more > of it.
Right, I recall seeing that series go by in October ; thanks for the reminder, I'll clean this up to match what was done in commit 41a1431b178c3b73 and its follow-on commits.
Paul. --
> > >> +unsigned int >> +__swake_up_locked(struct swait_queue_head *head, unsigned int state, >> + unsigned int num) >> +{ >> + struct swaiter *curr, *next; >> + int woken = 0; >> + >> + list_for_each_entry_safe(curr, next, &head->task_list, node) { >> + if (wake_up_state(curr->task, state)) { >> + __swait_dequeue(curr); >> + /* >> + * The waiting task can free the waiter as >> + * soon as curr->task = NULL is written, >> + * without taking any locks. A memory barrier >> + * is required here to prevent the following >> + * store to curr->task from getting ahead of >> + * the dequeue operation. >> + */ >> + smp_wmb(); >> + curr->task = NULL; >> + if (++woken == num) >> + break; >> + } >> + } >> + return woken; >> +} >> + >> +unsigned int >> +__swake_up(struct swait_queue_head *head, unsigned int state, unsigned int num) >> +{ >> + unsigned long flags; >> + int woken; >> + >> + if (!swaitqueue_active(head)) >> + return 0; >> + >> + raw_spin_lock_irqsave(&head->lock, flags); >> + woken = __swake_up_locked(head, state, num); >> + raw_spin_unlock_irqrestore(&head->lock, flags); >> + return woken; >> +} >> +EXPORT_SYMBOL(__swake_up); > > Urgh, fail. Do not put unbounded loops in raw_spin_lock. > > I think I posted a patch a while back to cure this. > >
|  |