Skip to content

Commit

Permalink
do_generic_file_read: s/EINTR/EIO/ if lock_page_killable() fails
Browse files Browse the repository at this point in the history
If lock_page_killable() fails because the task was killed by SIGKILL or
any other fatal signal, do_generic_file_read() returns -EIO.

This seems to be OK, because in fact the userspace won't see this error,
the task will dequeue SIGKILL and exit.

However, /sbin/init is different, it will dequeue SIGKILL, ignore it, and
return to the user-space with the bogus -EIO.

Change the code to return the error code from lock_page_killable(), -EINTR.
This doesn't fix the bug, but perhaps makes sense anyway. Imho, with this
change the code looks a bit more logical, and the "good" init should handle
the spurious EINTR or short read.

Afaics we can also change lock_page_killable() to return -ERESTARTNOINTR,
but this can't prevent the short reads.

Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
  • Loading branch information
Oleg Nesterov authored and Ingo Molnar committed Oct 14, 2008
1 parent 7591103 commit 8546232
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions mm/filemap.c
Original file line number Diff line number Diff line change
Expand Up @@ -1100,8 +1100,9 @@ static void do_generic_file_read(struct file *filp, loff_t *ppos,

page_not_up_to_date:
/* Get exclusive access to the page ... */
if (lock_page_killable(page))
goto readpage_eio;
error = lock_page_killable(page);
if (unlikely(error))
goto readpage_error;

page_not_up_to_date_locked:
/* Did it get truncated before we got the lock? */
Expand Down Expand Up @@ -1130,8 +1131,9 @@ static void do_generic_file_read(struct file *filp, loff_t *ppos,
}

if (!PageUptodate(page)) {
if (lock_page_killable(page))
goto readpage_eio;
error = lock_page_killable(page);
if (unlikely(error))
goto readpage_error;
if (!PageUptodate(page)) {
if (page->mapping == NULL) {
/*
Expand All @@ -1143,15 +1145,14 @@ static void do_generic_file_read(struct file *filp, loff_t *ppos,
}
unlock_page(page);
shrink_readahead_size_eio(filp, ra);
goto readpage_eio;
error = -EIO;
goto readpage_error;
}
unlock_page(page);
}

goto page_ok;

readpage_eio:
error = -EIO;
readpage_error:
/* UHHUH! A synchronous read error occurred. Report it */
desc->error = error;
Expand Down

0 comments on commit 8546232

Please sign in to comment.