Messages in this thread | | | Date | Thu, 23 Feb 2006 12:59:55 +0530 | From | Suparna Bhattacharya <> | Subject | [RFC][WIP] DIO simplification and AIO-DIO stability |
| |
DIO code complexity and stability concerns were discussed way back during OLS and Kernel summit last year. Still, the lack of a solid alternative and motivation to subject oneself to the test of courage and delicate balance that fiddling with this code entails, has meant that gingerly applying fixes and bandaids as and when bugs are found, and moving on thereafter, continues to be the most palatable option.
A recent AIO-DIO bug reported by Kenneth Chen, came very close to being the proverbial last straw for me. Hence, here is a rough attempt to put together a (currently WIP) draft towards DIO code simplication, based on suggestions that some of you have brought up at various times. Several details, e.g. range locking implementation still need to be fleshed out completely, ideas/comments/suggestions would be welcome.
http://www.kernel.org/pub/linux/kernel/people/suparna/DIO-simplify.txt (also inlined below)
It would be quite pointless (and painful!), if the rewrite ends up becoming just as tricky and error prone as before. Such a patch will need a very close critical review by many sharp eyes, to avoid disrupting the current state of stability. So before going any further with this, I'm looking for feedback along the lines of:
- Is this a worthwhile thing to attempt ? Or is status quo good enough ? - Does the approach make sense ? Is there a simpler way ? - Is there any hidden complexity or performance overhead that you forsee ? - Adding an extra tag to the radix-tree for locking a range of pages would impact the size of the radix tree; would that be a concern ?
Regards Suparna
------------------------------------------------------
Need for simplification ----------------------- - The code has become tricky and hence difficult to understand and maintain - Stability has been a concern, bugs never seem to stop cropping up (especially with regard to error handling), and there still are some potential loopholes ini the current implementation, esp AIO-DIO e.g. - AIO-DIO has some potential races in handling of IO errors, e.g. EIO. - For AIO-DIO writes, invalidate_inode_pages may be called before write completes
Considerations for simplification --------------------------------- - Reduce the number of special cases and conditions to check for - Bring locking model closer to that followed for regular read/writes - Unify logic as far as possible for conceptual simplicity - Optimize for the most common situation, i.e. non-extending DIO to already allocated blocks, with no concurrent buffered IO on the same file, and no concurrent DIO to the same part of the file.
I. DIO read -----------
Protection against buffered IO hole overwrite, where uninstantiated blocks may be exposed until writeback completes. Buffered reads go through page cache and hence never see stale data. Direct reads however go directly to disk.
(a) Current Solution: - i_alloc_sem held for read => protects against truncate - i_mutex held => no new blocks get created until DIO getblocks completes - filemap_write_and_wait => ensures any existing uninstantiated blocks complete writeback to disk
- AIO error handling paths differ for errors during submission vs during IO - in one case aio_complete issued by higher layer, in another case within the io completion path in finished_one_bio
(b) Alternative Solution (Follow similar locking rules as buffered read) - i_alloc_sem held for read => protects against truncate - Lookup and lock pages in the range by tagging the radix tree slots as "TAG_LOCKED", and locking pages that were present. Modify find_get_page et al to return a dummy locked cache page if TAG_LOCKED. (Alternatively put the check in add_to_page_cache, instead) => no new writes or blocks instantiated until DIO getblocks completes - Issue equiv of filemap_write_and_wait_range that acts on already locked pages (also likely to be useful for mpage_writepages) => ensures existing uninstantiated blocks written to disk - On IO completion release lock on pages and the range, including unlocking and releasing dummy cache page. => This will wake up readers and writers blocked on locked pages. Since on wakeup most code paths check mapping once again and also hold an extra ref count, this should be safe. (hopefully !)
Optional: - With retry based AIO-DIO => aio_complete can happen in the same way during submission and during a retry following completion, thus error handling takes the same path in both cases.
II. DIO write
Protection against buffered IO reads or DIO reads being exposed to uninstantiated blocks during a DIO hole overwrite/extend. Holding i_sem in DIO writes doesn't help protect buffered IO reads, since i_sem is not acquired in read paths.
(a) Current Solution - i_alloc_sem held for read => protects against truncate - Hold i_mutex at least till completion of DIO getblocks and submission - Force file extending DIO writes to be i_mutex holding until i_size increase (i.e till DIO completes) to avoid exposing new blocks - Fallback to buffered IO for hole overwrites, so that the default page based locking protects buffered IO reads from seeing uninstantiated blocks - filemap_write_and_wait => ensures any pending writes complete writeback to disk - Perform DIO write - invalidate pages => subsequent reads should see newly written data
- Force extending AIO-DIO writes to be synchronous - Force AIO-DIO overwrites that cross holes to be synchronous for issued DIO before fallback to buffered IO. - AIO Error handling takes different paths for errors during submission and during IO completion. Fallback to buffered adds another level of complexity, as does forced synchronous behaviour for extends.
(b) Alternative Solution (Follow similar locking rules as buffered IO) - i_alloc_sem held for read => protects against truncate - check if this is an extending write, and if not release i_mutex - Lookup and lock pages in the range by tagging the radix tree slots as "TAG_LOCKED", and locking pages that were present. => reads are blocked until DIO write completes, i.e uninstantiated blocks not exposed - Issue equiv of filemap_write_and_wait_range that acts on already locked pages and then invalidate the pages. => subsequent reads should see newly written data - Perform DIO write (including block allocations etc) - On IO completion release lock on pages and unlock the range, including unlocking and releasing dummy cache page, release i_alloc_sem and i_mutex.
[i_mutex and i_alloc_sem handling can be moved entirely into higher levels, which avoids DIO_LOCKING checks inside the DIO code; Since i_mutex is held only for extending writes it should not be a concurrency issue; Also in the AIO case, it is ok to update i_size and release i_mutex (perhaps even i_alloc_sem) w/o having to wait for IO completion since pages are still locked ] Optional: - With retry based AIO-DIO => aio_complete happens in the same way during submission and during a retry following completion. Unlocking the range and release of pages can likewise happen in retry context (not in interrupt context).
-- Suparna Bhattacharya (suparna@in.ibm.com) Linux Technology Center IBM Software Lab, India
- 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/
| |