Messages in this thread Patch in this message | | | Date | Wed, 24 Jun 2020 18:11:42 +0200 | From | Oleg Nesterov <> | Subject | wait_on_page_bit_common(TASK_KILLABLE, EXCLUSIVE) can miss wakeup? |
| |
Suppose that 2 threads T1 and T2 call __lock_page_killable() and sleep in wait_on_page_bit_common() -> io_schedule().
T1 is killed, it does test_and_set_bit_lock() but the page is still locked.
unlock_page() calls __wake_up_common(nr_exclusive = 1), this wakes T1 up. T2 is not woken.
T1 checks signal_pending_state() and returns EINTR.
T2 will sleep until another thread does lock/unlock ?
---------------------------------------------------------------------------- I noticed this by accident, I am hunting for another / unrelated bug. I did git-blame and iiuc the commit a8b169afbf06a ("Avoid page waitqueue race leaving possible page locker waiting") tried to fix the problem but see above, I don't understand how can it help.
Don't we need something like below or I am totally confused?
Oleg.
--- x/mm/filemap.c +++ x/mm/filemap.c @@ -1131,14 +1131,23 @@ static inline int wait_on_page_bit_commo wait_page.bit_nr = bit_nr; for (;;) { + int intr = 0; + spin_lock_irq(&q->lock); - if (likely(list_empty(&wait->entry))) { - __add_wait_queue_entry_tail(q, wait); - SetPageWaiters(page); - } + // see the comment prepare_to_wait_event() + if (signal_pending_state(state, current)) { + list_del_init(&wait->entry); + intr = 1; + } else { + if (likely(list_empty(&wait->entry))) { + // HMM. head/tail depending on EXCLUSIVE ??? + __add_wait_queue_entry_tail(q, wait); + SetPageWaiters(page); + } - set_current_state(state); + set_current_state(state); + } spin_unlock_irq(&q->lock); @@ -1146,7 +1155,7 @@ static inline int wait_on_page_bit_commo if (behavior == DROP) put_page(page); - if (likely(bit_is_set)) + if (!intr && likely(bit_is_set)) io_schedule(); if (behavior == EXCLUSIVE) { @@ -1157,7 +1166,7 @@ static inline int wait_on_page_bit_commo break; } - if (signal_pending_state(state, current)) { + if (intr) { ret = -EINTR; break; }
| |