On Fri, Mar 09, 2018 at 12:04:18PM +0100, Sebastian Andrzej Siewior wrote:
> +void swake_add_all_wq(struct swait_queue_head *q, struct wake_q_head *wq)
>  {
>       struct swait_queue *curr;
>  
>       while (!list_empty(&q->task_list)) {
>  
>               curr = list_first_entry(&q->task_list, typeof(*curr),
>                                       task_list);
>               list_del_init(&curr->task_list);
> +             wake_q_add(wq, curr->task);
>       }
>  }
> +EXPORT_SYMBOL(swake_add_all_wq);
>  
>  void swake_up(struct swait_queue_head *q)
>  {
> @@ -66,25 +62,14 @@ EXPORT_SYMBOL(swake_up);
>   */
>  void swake_up_all(struct swait_queue_head *q)
>  {
> +     unsigned long flags;
> +     DEFINE_WAKE_Q(wq);
>  
> +     raw_spin_lock_irqsave(&q->lock, flags);
> +     swake_add_all_wq(q, &wq);
> +     raw_spin_unlock_irqrestore(&q->lock, flags);
>  
> +     wake_up_q(&wq);
>  }
>  EXPORT_SYMBOL(swake_up_all);

This is fundamentally wrong. The whole point of wake_up_all() is that
_all_ is unbounded and should not ever land in a single critical
section, be it IRQ or PREEMPT disabled. The above does both.

Yes, wake_up_all() is crap, it is also fundamentally incompatible with
in-*irq usage. Nothing to be done about that.

So NAK on this.

Reply via email to