Prevent direct I/O from mapping extents beyond eof
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 16 lines · Lachlan McIlroy
With the help from some tracing I found that we try to map extents beyond eof when doing a direct I/O read. It appears that the way to inform the generic direct I/O path (ie
do_direct_IO()) that we have breached eof is to return an unmapped buffer fromxfs_get_blocks_direct(). This will causedo_direct_IO()to jump to the hole handling code where is will check for eof and then abort.This problem was found because a direct I/O read was trying to map beyond eof and was encountering delayed allocations. The delayed allocations beyond eof are speculative allocations and they didn't get converted when the direct I/O flushed the file because there was only enough space in the current AG to convert and write out the dirty pages within eof. Note that
xfs_iomap_write_allocate()wont necessarily convert all the delayed allocation passed to it - it will return after allocating the first extent - so if the delayed allocation extends beyond eof then it will stay that way.
Quoted 1 line
This change will detect a direct I/O read beyond eof:
The change looks good to me, but I really think the direct I/O could should never send down requests like this down to the filesystems. akpm and -fsdevel Cc'ed.
Quoted 5 lines
fs/xfs/linux-2.6/xfs_aops.c __xfs_get_blocks( · line 1347
Quoted 10 lines
fs/xfs/linux-2.6/xfs_aops.c +4
-0@@ -1338,6 +1338,10 @@ __xfs_get_blocks( offset = (xfs_off_t )iblock << inode->i_blkbits;ASSERT (bh_result->b_size >= (1 << inode->i_blkbits)); size = bh_result->b_size; + +if (!create && direct && offset >=i_size_read (inode)) +return 0 ; + error =xfs_iomap (XFS_I (inode), offset, size, create ? flags :BMAPI_READ , &iomap, &niomap);if (error)
---end quoted text---
Quoted 17 lines · On Fri, 15 Aug 2008 18:09:58 -0400 Christoph Hellwig <[email protected]> wrote:
Quoted 16 lines · Lachlan McIlroy
With the help from some tracing I found that we try to map extents beyond eof when doing a direct I/O read. It appears that the way to inform the generic direct I/O path (ie
do_direct_IO()) that we have breached eof is to return an unmapped buffer fromxfs_get_blocks_direct(). This will causedo_direct_IO()to jump to the hole handling code where is will check for eof and then abort.This problem was found because a direct I/O read was trying to map beyond eof and was encountering delayed allocations. The delayed allocations beyond eof are speculative allocations and they didn't get converted when the direct I/O flushed the file because there was only enough space in the current AG to convert and write out the dirty pages within eof. Note that
xfs_iomap_write_allocate()wont necessarily convert all the delayed allocation passed to it - it will return after allocating the first extent - so if the delayed allocation extends beyond eof then it will stay that way.
Quoted 5 lines
Quoted 1 line
This change will detect a direct I/O read beyond eof:
The change looks good to me, but I really think the direct I/O could should never send down requests like this down to the filesystems. akpm and -fsdevel Cc'ed.
Oh gee, I forget, and so many people have done drivebys on that code...
We _could_ add additional i_size checking into direct-io.c but bear in mind that it would be best-effort unreliable stuff. The code will still be tripped up by concurrent extends and concurrent truncates.
So we'll still end up calling the fs for blocks outside i_size, only less commonly. I think.
Quoted 8 lines · Andrew Morton
Oh gee, I forget, and so many people have done drivebys on that code...
We _could_ add additional
i_sizechecking intodirect-io.cbut bear in mind that it would be best-effort unreliable stuff. The code will still be tripped up by concurrent extends and concurrent truncates.So we'll still end up calling the fs for blocks outside
i_size, only less commonly. I think.
Yeah, guess we should put in this patch then.