lkml.org 
[lkml]   [2005]   [Mar]   [8]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
DateMon, 7 Mar 2005 22:39:17 -0800
FromAndrew Morton <>
SubjectRe: [PATCH] 2.6.10 - direct-io async short read bug
Sébastien Dugué <sebastien.dugue@bull.net> wrote:
>
> When reading a file in async mode (using kernel AIO), and the file
>  size is lower than the requested size (short read),  the direct IO
>  layer reports an incorrect number of bytes read (transferred).
> 
>   That case is handled for the sync path in 'direct_io_worker' 
>  (fs/direct-io.c) where a check is made against the file size.
> 
>   This patch does the same thing for the async path.

It looks sane to me.  It needs a couple of fixes, below.  One of them is
horrid and isn't really a fix at all, but it improves things.

Would Suparna and Badari have time to check the logic of these two patches
please?




- i_size is 64 bit, ssize_t is 32-bit

- whitespace tweaks.

- i_size_read() in interrupt context is a no-no.

Signed-off-by: Andrew Morton <akpm@osdl.org>
---
 25-akpm/fs/direct-io.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)
diff -puN fs/direct-io.c~direct-io-async-short-read-fix-fix fs/direct-io.c
--- 25/fs/direct-io.c~direct-io-async-short-read-fix-fix	2005-03-07 22:28:52.000000000 -0800
+++ 25-akpm/fs/direct-io.c	2005-03-07 22:37:18.000000000 -0800
@@ -231,7 +231,7 @@ static void finished_one_bio(struct dio 
 	if (dio->bio_count == 1) {
 		if (dio->is_async) {
 			ssize_t transferred;
-			ssize_t i_size;
+			loff_t i_size;
 			loff_t offset;
 
 			/*
@@ -241,11 +241,19 @@ static void finished_one_bio(struct dio 
 			spin_unlock_irqrestore(&dio->bio_lock, flags);
 
 			/* Check for short read case */
+
+			/*
+			 * We should use i_size_read() here.  But we're called
+			 * in interrupt context.  If this CPU happened to be
+			 * in the middle of i_size_write() when the interrupt
+			 * occurred, i_size_read() would lock up.
+			 * So we just risk getting a wrong result instead :(
+			 */
+			i_size = dio->inode->i_size;
 			transferred = dio->result;
-			i_size = i_size_read (dio->inode);
 			offset = dio->iocb->ki_pos;
 
-			if ((dio->rw == READ) && ((offset + transferred) > i_size))
+			if ((dio->rw == READ) && (offset+transferred > i_size))
 				transferred = i_size - offset;
 
 			dio_complete(dio, offset, transferred);
_
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:10    [W:17.827 / U:0.000 seconds]
©2003-2008 Jasper Spaans