Re: [PATCH] 2.6.10 - direct-io async short read bug

[Andrew Morton]

Sébastien Dugué <sebastien.dugue@xxxxxxxx> 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@xxxxxxxx>
---

 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/