do_generic_file_read: s/EINTR/EIO/ if lock_page_killable() fails
authorOleg Nesterov <oleg@tv-sign.ru>
Sun, 8 Jun 2008 17:20:43 +0000 (21:20 +0400)
committerIngo Molnar <mingo@elte.hu>
Tue, 14 Oct 2008 15:15:33 +0000 (17:15 +0200)
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>
mm/filemap.c

index 876bc595d0f8b3b3e4f626bf1ad7acbb657bdc66..494ff20b6cfa7c66291e4fbfc425de0e477322da 100644 (file)
@@ -1100,8 +1100,9 @@ page_ok:
 
 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? */
@@ -1130,8 +1131,9 @@ readpage:
                }
 
                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) {
                                        /*
@@ -1143,15 +1145,14 @@ readpage:
                                }
                                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;