Messages in this thread | | | Date | Sat, 12 Dec 2015 11:41:26 -0800 | Subject | Re: [PATCH] lock_page() doesn't lock if __wait_on_bit_lock returns -EINTR | From | Linus Torvalds <> |
| |
On Sat, Dec 12, 2015 at 10:33 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > Peter, did that patch also handle just plain "lock_page()" case?
Looking more at it, I think this all goes back to commit 743162013d40 ("sched: Remove proliferation of wait_on_bit() action functions").
Before that, we had wait_on_page_bit() doing:
__wait_on_bit(page_waitqueue(page), &wait, sleep_on_page, TASK_UNINTERRUPTIBLE);
and after that, the "sleep_on_page" got changed to "bit_wait_io".
But that is bogus, because sleep_on_page() used to look like this:
static int sleep_on_page(void *word) { io_schedule(); return 0; }
while bit_wait_io() looks like this:
__sched int bit_wait_io(void *word) { if (signal_pending_state(current->state, current)) return 1; io_schedule(); return 0; }
which is ok, because as long as the task state is TASK_UNINTERRUPTIBLE, the whole signal_pending_state() thing turns into a no-op.
So far, so fine.
However, then commit 68985633bccb ("sched/wait: Fix signal handling in bit wait helpers") _really_ screwed up, and changed the function to
__sched int bit_wait(struct wait_bit_key *word) { schedule(); if (signal_pending(current)) return -EINTR; return 0; }
so now it returns an error when no error should happen. Which in turn makes __wait_on_bit() exit the bit-wait loop early.
It looks like PeterZ's pending patch should fix this, by passing in the proper TASK_UNINTERRUPTIBLE to the bit_wait_io function, and going back to signal_pending_state(). PeterZ, did I follow the history of this correctly?
Linus
| |