Commit 76af79f393ad562077f79627a4c719219ef09ee8
Committed by
Greg Kroah-Hartman
1 parent
61339713b4
readahead: fix pipeline break caused by block plug
commit 3deaa7190a8da38453c4fabd9dec7f66d17fff67 upstream. Herbert Poetzl reported a performance regression since 2.6.39. The test is a simple dd read, but with big block size. The reason is: T1: ra (A, A+128k), (A+128k, A+256k) T2: lock_page for page A, submit the 256k T3: hit page A+128K, ra (A+256k, A+384). the range isn't submitted because of plug and there isn't any lock_page till we hit page A+256k because all pages from A to A+256k is in memory T4: hit page A+256k, ra (A+384, A+ 512). Because of plug, the range isn't submitted again. T5: lock_page A+256k, so (A+256k, A+512k) will be submitted. The task is waitting for (A+256k, A+512k) finish. There is no request to disk in T3 and T4, so readahead pipeline breaks. We really don't need block plug for generic_file_aio_read() for buffered I/O. The readahead already has plug and has fine grained control when I/O should be submitted. Deleting plug for buffered I/O fixes the regression. One side effect is plug makes the request size 256k, the size is 128k without it. This is because default ra size is 128k and not a reason we need plug here. Vivek said: : We submit some readahead IO to device request queue but because of nested : plug, queue never gets unplugged. When read logic reaches a page which is : not in page cache, it waits for page to be read from the disk : (lock_page_killable()) and that time we flush the plug list. : : So effectively read ahead logic is kind of broken in parts because of : nested plugging. Removing top level plug (generic_file_aio_read()) for : buffered reads, will allow unplugging queue earlier for readahead. Signed-off-by: Shaohua Li <shaohua.li@intel.com> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com> Reported-by: Herbert Poetzl <herbert@13thfloor.at> Tested-by: Eric Dumazet <eric.dumazet@gmail.com> Cc: Christoph Hellwig <hch@infradead.org> Cc: Jens Axboe <axboe@kernel.dk> Cc: Vivek Goyal <vgoyal@redhat.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Showing 1 changed file with 4 additions and 4 deletions Side-by-side Diff
mm/filemap.c
... | ... | @@ -1400,15 +1400,12 @@ |
1400 | 1400 | unsigned long seg = 0; |
1401 | 1401 | size_t count; |
1402 | 1402 | loff_t *ppos = &iocb->ki_pos; |
1403 | - struct blk_plug plug; | |
1404 | 1403 | |
1405 | 1404 | count = 0; |
1406 | 1405 | retval = generic_segment_checks(iov, &nr_segs, &count, VERIFY_WRITE); |
1407 | 1406 | if (retval) |
1408 | 1407 | return retval; |
1409 | 1408 | |
1410 | - blk_start_plug(&plug); | |
1411 | - | |
1412 | 1409 | /* coalesce the iovecs and go direct-to-BIO for O_DIRECT */ |
1413 | 1410 | if (filp->f_flags & O_DIRECT) { |
1414 | 1411 | loff_t size; |
1415 | 1412 | |
... | ... | @@ -1424,8 +1421,12 @@ |
1424 | 1421 | retval = filemap_write_and_wait_range(mapping, pos, |
1425 | 1422 | pos + iov_length(iov, nr_segs) - 1); |
1426 | 1423 | if (!retval) { |
1424 | + struct blk_plug plug; | |
1425 | + | |
1426 | + blk_start_plug(&plug); | |
1427 | 1427 | retval = mapping->a_ops->direct_IO(READ, iocb, |
1428 | 1428 | iov, pos, nr_segs); |
1429 | + blk_finish_plug(&plug); | |
1429 | 1430 | } |
1430 | 1431 | if (retval > 0) { |
1431 | 1432 | *ppos = pos + retval; |
... | ... | @@ -1481,7 +1482,6 @@ |
1481 | 1482 | break; |
1482 | 1483 | } |
1483 | 1484 | out: |
1484 | - blk_finish_plug(&plug); | |
1485 | 1485 | return retval; |
1486 | 1486 | } |
1487 | 1487 | EXPORT_SYMBOL(generic_file_aio_read); |