Skip to content

Commit 3deaa71

Browse files
Shaohua Litorvalds
Shaohua Li
authored andcommitted
readahead: fix pipeline break caused by block plug
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 <[email protected]> Signed-off-by: Wu Fengguang <[email protected]> Reported-by: Herbert Poetzl <[email protected]> Tested-by: Eric Dumazet <[email protected]> Cc: Christoph Hellwig <[email protected]> Cc: Jens Axboe <[email protected]> Cc: Vivek Goyal <[email protected]> Cc: <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 55ca614 commit 3deaa71

File tree

1 file changed

+4
-4
lines changed

1 file changed

+4
-4
lines changed

mm/filemap.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1400,15 +1400,12 @@ generic_file_aio_read(struct kiocb *iocb, const struct iovec *iov,
14001400
unsigned long seg = 0;
14011401
size_t count;
14021402
loff_t *ppos = &iocb->ki_pos;
1403-
struct blk_plug plug;
14041403

14051404
count = 0;
14061405
retval = generic_segment_checks(iov, &nr_segs, &count, VERIFY_WRITE);
14071406
if (retval)
14081407
return retval;
14091408

1410-
blk_start_plug(&plug);
1411-
14121409
/* coalesce the iovecs and go direct-to-BIO for O_DIRECT */
14131410
if (filp->f_flags & O_DIRECT) {
14141411
loff_t size;
@@ -1424,8 +1421,12 @@ generic_file_aio_read(struct kiocb *iocb, const struct iovec *iov,
14241421
retval = filemap_write_and_wait_range(mapping, pos,
14251422
pos + iov_length(iov, nr_segs) - 1);
14261423
if (!retval) {
1424+
struct blk_plug plug;
1425+
1426+
blk_start_plug(&plug);
14271427
retval = mapping->a_ops->direct_IO(READ, iocb,
14281428
iov, pos, nr_segs);
1429+
blk_finish_plug(&plug);
14291430
}
14301431
if (retval > 0) {
14311432
*ppos = pos + retval;
@@ -1481,7 +1482,6 @@ generic_file_aio_read(struct kiocb *iocb, const struct iovec *iov,
14811482
break;
14821483
}
14831484
out:
1484-
blk_finish_plug(&plug);
14851485
return retval;
14861486
}
14871487
EXPORT_SYMBOL(generic_file_aio_read);

0 commit comments

Comments
 (0)