* [patch] fix O_DIRECT read of last block in a sparse file
@ 2006-01-23 19:16 Jeff Moyer
2006-01-23 23:54 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Moyer @ 2006-01-23 19:16 UTC (permalink / raw)
To: linux-kernel; +Cc: akpm
Hi,
Currently, if you open a file O_DIRECT, truncate it to a size that is not a
multiple of the disk block size, and then try to read the last block in the
file, the read will return 0. The problem is in do_direct_IO, here:
/* Handle holes */
if (!buffer_mapped(map_bh)) {
char *kaddr;
...
if (dio->block_in_file >=
i_size_read(dio->inode)>>blkbits) {
/* We hit eof */
page_cache_release(page);
goto out;
}
We shift off any remaining bytes in the final block of the I/O, resulting
in a 0-sized read. I've attached a patch that fixes this. I'm not happy
about how ugly the math is getting, so suggestions are more than welcome.
I've tested this with a simple program that performs the steps outlined for
reproducing the problem above. Without the patch, we get a 0-sized result
from read. With the patch, we get the correct return value from the short
read.
Thanks,
Jeff
Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
--- linux-2.6.16-rc1-mm1/fs/direct-io.c.orig 2006-01-19 15:45:50.000000000 -0500
+++ linux-2.6.16-rc1-mm1/fs/direct-io.c 2006-01-19 16:09:33.000000000 -0500
@@ -857,6 +857,7 @@ do_holes:
/* Handle holes */
if (!buffer_mapped(map_bh)) {
char *kaddr;
+ loff_t i_size = i_size_read(dio->inode);
/* AKPM: eargh, -ENOTBLK is a hack */
if (dio->rw == WRITE) {
@@ -864,8 +865,10 @@ do_holes:
return -ENOTBLK;
}
- if (dio->block_in_file >=
- i_size_read(dio->inode)>>blkbits) {
+ /* Be sure to account for a partial block as
+ * the last block in the file */
+ if (dio->block_in_file >= i_size >> blkbits &&
+ !(i_size & ~((1<<blkbits)-1))) {
/* We hit eof */
page_cache_release(page);
goto out;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] fix O_DIRECT read of last block in a sparse file
2006-01-23 19:16 [patch] fix O_DIRECT read of last block in a sparse file Jeff Moyer
@ 2006-01-23 23:54 ` Andrew Morton
2006-01-24 12:09 ` Jeff Moyer
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2006-01-23 23:54 UTC (permalink / raw)
To: jmoyer; +Cc: linux-kernel, Badari Pulavarty
Jeff Moyer <jmoyer@redhat.com> wrote:
>
> Currently, if you open a file O_DIRECT, truncate it to a size that is not a
> multiple of the disk block size, and then try to read the last block in the
> file, the read will return 0. The problem is in do_direct_IO, here:
>
> /* Handle holes */
> if (!buffer_mapped(map_bh)) {
> char *kaddr;
>
> ...
>
> if (dio->block_in_file >=
> i_size_read(dio->inode)>>blkbits) {
> /* We hit eof */
> page_cache_release(page);
> goto out;
> }
>
> We shift off any remaining bytes in the final block of the I/O, resulting
> in a 0-sized read. I've attached a patch that fixes this. I'm not happy
> about how ugly the math is getting, so suggestions are more than welcome.
>
> I've tested this with a simple program that performs the steps outlined for
> reproducing the problem above. Without the patch, we get a 0-sized result
> from read. With the patch, we get the correct return value from the short
> read.
OK. We do have some helper functions to make the math a little clearer.
How does this look?
--- devel/fs/direct-io.c~fix-o_direct-read-of-last-block-in-a-sparse-file 2006-01-23 15:42:31.000000000 -0800
+++ devel-akpm/fs/direct-io.c 2006-01-23 15:51:14.000000000 -0800
@@ -857,6 +857,7 @@ do_holes:
/* Handle holes */
if (!buffer_mapped(map_bh)) {
char *kaddr;
+ loff_t i_size_aligned;
/* AKPM: eargh, -ENOTBLK is a hack */
if (dio->rw == WRITE) {
@@ -864,8 +865,14 @@ do_holes:
return -ENOTBLK;
}
+ /*
+ * Be sure to account for a partial block as the
+ * last block in the file
+ */
+ i_size_aligned = ALIGN(i_size_read(dio->inode),
+ 1 << blkbits);
if (dio->block_in_file >=
- i_size_read(dio->inode)>>blkbits) {
+ i_size_aligned >> blkbits) {
/* We hit eof */
page_cache_release(page);
goto out;
_
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] fix O_DIRECT read of last block in a sparse file
2006-01-23 23:54 ` Andrew Morton
@ 2006-01-24 12:09 ` Jeff Moyer
0 siblings, 0 replies; 3+ messages in thread
From: Jeff Moyer @ 2006-01-24 12:09 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, Badari Pulavarty
==> Regarding Re: [patch] fix O_DIRECT read of last block in a sparse file; Andrew Morton <akpm@osdl.org> adds:
akpm> Jeff Moyer <jmoyer@redhat.com> wrote:
>> Currently, if you open a file O_DIRECT, truncate it to a size that is
>> not a multiple of the disk block size, and then try to read the last
>> block in the file, the read will return 0. The problem is in
>> do_direct_IO, here:
>>
>> /* Handle holes */ if (!buffer_mapped(map_bh)) { char *kaddr;
>>
>> ...
>>
>> if (dio->block_in_file >= i_size_read(dio->inode)>>blkbits) { /* We hit
>> eof */ page_cache_release(page); goto out; }
>>
>> We shift off any remaining bytes in the final block of the I/O,
>> resulting in a 0-sized read. I've attached a patch that fixes this.
>> I'm not happy about how ugly the math is getting, so suggestions are
>> more than welcome.
>>
>> I've tested this with a simple program that performs the steps outlined
>> for reproducing the problem above. Without the patch, we get a 0-sized
>> result from read. With the patch, we get the correct return value from
>> the short read.
akpm> OK. We do have some helper functions to make the math a little
akpm> clearer. How does this look?
That's much cleaner, thanks!
-Jeff
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2006-01-24 12:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-01-23 19:16 [patch] fix O_DIRECT read of last block in a sparse file Jeff Moyer
2006-01-23 23:54 ` Andrew Morton
2006-01-24 12:09 ` Jeff Moyer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).