Possible lock-less list race in scheduler_ipi()

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

Hi,

I recently reviewed the scheduler_ipi() code by
coincidence, and noticed that the use of llist.h
in there seemed a bit odd. So I'd like to have
more eyes to help me look into this.

I've been told that there has been issues with
IPIs lately, so here is one possible scenario
I think might be possible:

- scheduler_ipi()
  - sched_ttwu_pending()
    - llist_del_all()
      (grabs the wake_list with xchg())
    - raw_spin_lock_irqsave(&rq->lock, flags);
    - iteration on the llist (owned locally by interrupt handler)
      (for each item)
        p = llist_entry(llist, struct task_struct, wake_entry);
        llist = llist_next(llist);
        ttwu_do_activate(rq, p, 0);
    - raw_spin_unlock_irqrestore(&rq->lock, flags);

ttwu_do_activate() ends up calling
ttwu_activate() and ttwu_do_wakeup(), 

I'm worried that ttwu_do_wakeup() might end up
indirectly handing off the task to the following
functions (within another execution context):

- try_to_wake_up()
  - ttwu_queue()
    - ttwu_queue_remote adds the process to another
      wake_list with a cmpxchg()

llist_next() is pretty simple:

static inline struct llist_node *llist_next(struct llist_node *node)
{
        return node->next;
}

It is so simple that I wonder if the compiler would be
within its rights to reorder the load of node->next
after some operations within ttwu_do_activate(), thus
causing corruption of this linked-list due to a
concurrent try_to_wake_up() performed by another core.

Am I too paranoid about the possible compiler mishaps
there, or are my concerns justified ?

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com