Messages in this thread | | | Date | Thu, 1 Sep 2016 21:08:58 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 2/2] sched/wait: avoid abort_exclusive_wait() in __wait_on_bit_lock() |
| |
On Thu, Sep 01, 2016 at 09:01:41PM +0200, Peter Zijlstra wrote: > > test_and_set_bit() implies mb() so > > the lockless list_empty_careful() case is fine, we can not miss the > > condition if we race with unlock_page(). > > You're talking about this ordering?: > > finish_wait() clear_bit_unlock(); > list_empty_careful() > > /* MB implied */ smp_mb__after_atomic(); > test_and_set_bit() wake_up_page() > ... > autoremove_wake_function() > list_del_init(); > > > That could do with spelling out I feel.. :-)
This ^^^
> > I am not sure we even want to conditionalize both finish_wait()'s, > > we could simply call it unconditionally and once before test_and_set(), > > the spurious wakeup is unlikely case. > > > ret = 0; > > for (;;) { > prepare_to_wait_exclusive(wq, &q->wait, mode); > > if (test_bit(&q->key.bit_nr, &q->key.flag)) > ret = action(&q->key, mode); > > if (!test_and_set_bit(&q->key.bit_nr, &q->key.flag)) { > /* we got the lock anyway, ignore the signal */ > ret = 0; > break; > } > > if (ret) > break; > } > finish_wait(wq, &q->wait); > > return ret; > > > Would not that work too?
Nope, because we need to do that finish_wait() before test_and_set_bit()..
Also the problem with doing finish_wait() unconditionally would be destroying the FIFO order. With a bit of bad luck you'd get starvation cases :/
| |