Messages in this thread | | | Subject | Re: [PATCH] ipc/sem: Fix race between to-be-woken task and waker | From | Manfred Spraul <> | Date | Sun, 29 Sep 2019 12:24:31 +0200 |
| |
Hi Waiman,
I have now written the mail 3 times: Twice I thought that I found a race, but during further analysis, it always turns out that the spin_lock() is sufficient.
First, to avoid any obvious things: Until the series with e.g. 27d7be1801a4824e, there was a race inside sem_lock().
Thus it was possible that multiple threads were operating on the same semaphore array, with obviously arbitrary impact.
On 9/20/19 5:54 PM, Waiman Long wrote:
> > + /* > + * A spurious wakeup at the right moment can cause race > + * between the to-be-woken task and the waker leading to > + * missed wakeup. Setting state back to TASK_INTERRUPTIBLE > + * before checking queue.status will ensure that the race > + * won't happen. > + * > + * CPU0 CPU1 > + * > + * <spurious wakeup> wake_up_sem_queue_prepare(): > + * state = TASK_INTERRUPTIBLE status = error > + * try_to_wake_up(): > + * smp_mb() smp_mb() > + * if (status == -EINTR) if (!(p->state & state)) > + * schedule() goto out > + */ > + set_current_state(TASK_INTERRUPTIBLE); > +
So the the hypothesis is that we have a race due to the optimization within try_to_wake_up(): If the status is already TASK_RUNNING, then the wakeup is a nop.
Correct?
The waker wants to use:
lock(); set_conditions(); unlock();
as the wake_q is a shared list, completely asynchroneously this will happen:
smp_mb(); //// ***1 if (current->state = TASK_INTERRUPTIBLE) current->state=TASK_RUNNING;
The only guarantee is that this will happen after lock(), it may happen before set_conditions().
The task that goes to sleep uses:
lock(); check_conditions(); __set_current_state(); unlock(); //// ***2 schedule();
You propose to change that to:
lock(); set_current_state(); check_conditions(); unlock(); schedule();
I don't see a race anymore, and I don't see how the proposed change will help. e.g.: __set_current_state() and smp_mb() have paired memory barriers ***1 and ***2 above.
--
Manfred
| |