Messages in this thread Patch in this message | | | From | "Hillf Danton" <> | Subject | Re: [PATCH 2/2] sched/wait: avoid abort_exclusive_wait() in __wait_on_bit_lock() | Date | Mon, 29 Aug 2016 16:41:50 +0800 |
| |
> > On 08/26, Oleg Nesterov wrote: > > > > We do not need anything tricky to avoid the race, we can just call > > finish_wait() if action() fails. 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(). > > To simplify the review see the code with the patch applied: > > > int __sched > __wait_on_bit_lock(wait_queue_head_t *wq, struct wait_bit_queue *q, > wait_bit_action_f *action, unsigned mode) > { > int ret = 0; > > for (;;) { > prepare_to_wait_exclusive(wq, &q->wait, mode); > if (test_bit(q->key.bit_nr, q->key.flags)) { > ret = action(&q->key, mode); > /* > * Ensure that clear_bit() + wake_up() right after > * test_and_set_bit() below can't see us; it should > * wake up another exclusive waiter if we fail. > */ > if (ret) > finish_wait(wq, &q->wait); > } > if (!test_and_set_bit(q->key.bit_nr, q->key.flags)) { > if (!ret) > finish_wait(wq, &q->wait); > return 0; > } else if (ret) { > return ret; > } > } > } > Can we fold two bit operations into one?
thanks Hillf --- linux-4.7/kernel/sched/wait.c Mon Jul 25 03:23:50 2016 +++ b/kernel/sched/wait.c Mon Aug 29 16:17:01 2016 @@ -425,20 +425,17 @@ int __sched __wait_on_bit_lock(wait_queue_head_t *wq, struct wait_bit_queue *q, wait_bit_action_f *action, unsigned mode) { - do { - int ret; + int ret = 0; - prepare_to_wait_exclusive(wq, &q->wait, mode); - if (!test_bit(q->key.bit_nr, q->key.flags)) - continue; + prepare_to_wait_exclusive(wq, &q->wait, mode); + do { + if (!test_and_set_bit(q->key.bit_nr, q->key.flags)) + break; ret = action(&q->key, mode); - if (!ret) - continue; - abort_exclusive_wait(wq, &q->wait, mode, &q->key); - return ret; - } while (test_and_set_bit(q->key.bit_nr, q->key.flags)); + } while (!ret); + finish_wait(wq, &q->wait); - return 0; + return ret; } EXPORT_SYMBOL(__wait_on_bit_lock); --
| |