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;
                }

Reply via email to