block: fix DIO handling regressions in blkdev_read_iter()
PATCH 1/1 block: fix DIO handling regressions in blkdev_read_iter()
Commit ceaa762527f4 ("block: move direct_IO into our own read_iter handler") introduced several regressions for bdev DIO:
1. read spanning EOF always returns 0 instead of the number of bytes read. This is because "count" is assigned early and isn't updated when the iterator is truncated:
$ lsblk -o name,size /dev/vdb
NAME SIZE
vdb 1G
$ xfs_io -d -c 'pread -b 4M 1021M 4M' /dev/vdb
read 0/4194304 bytes at offset 1070596096
0.000000 bytes, 0 ops; 0.0007 sec (0.000000 bytes/sec and 0.0000 ops/sec) instead of
$ xfs_io -d -c 'pread -b 4M 1021M 4M' /dev/vdb
read 3145728/4194304 bytes at offset 1070596096
3 MiB, 1 ops; 0.0007 sec (3.865 GiB/sec and 1319.2612 ops/sec)2. truncated iterator isn't reexpanded
3. iterator isn't reverted on blkdev_direct_IO() error
4. zero size read no longer skips atime update
--- block/fops.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-)
block/fops.c +19 -14
@@ -566,34 +566,37 @@ static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to) {struct block_device *bdev = iocb->ki_filp->private_data;loff_t size =bdev_nr_bytes (bdev); -size_t count =iov_iter_count (to);loff_t pos = iocb->ki_pos;size_t shorted =0 ;ssize_t ret =0 ; +size_t count; -if (unlikely (pos + count > size)) { +if (unlikely (pos +iov_iter_count (to) > size)) {if (pos >= size)return 0 ; size -= pos; -if (count > size) { - shorted = count - size; -iov_iter_truncate (to, size); - } + shorted =iov_iter_count (to) - size; +iov_iter_truncate (to, size); } + count =iov_iter_count (to); +if (!count) + goto reexpand;/* skip atime */ +if (iocb->ki_flags &IOCB_DIRECT ) {struct address_space *mapping = iocb->ki_filp->f_mapping;if (iocb->ki_flags &IOCB_NOWAIT ) { -if (filemap_range_needs_writeback (mapping, iocb->ki_pos, - iocb->ki_pos + count -1 )) -return -EAGAIN ; +if (filemap_range_needs_writeback (mapping, pos, + pos + count -1 )) { + ret = -EAGAIN ; + goto reexpand; + } }else { - ret =filemap_write_and_wait_range (mapping, - iocb->ki_pos, - iocb->ki_pos + count -1 ); + ret =filemap_write_and_wait_range (mapping, pos, + pos + count -1 );if (ret <0 ) -return ret; + goto reexpand; }file_accessed (iocb->ki_filp);
@@ -603,12 +606,14 @@ static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to) iocb->ki_pos += ret; count -= ret; } +iov_iter_revert (to, count -iov_iter_count (to));if (ret <0 || !count) -return ret; + goto reexpand; } ret =filemap_read (iocb, to, ret); +reexpand:if (unlikely (shorted))iov_iter_reexpand (to,iov_iter_count (to) + shorted);return ret;
Signature · 1 line
2.19.2
Looks good:
Quoted 8 lines · Ilya Dryomov
Commit
ceaa762527f4("block: movedirect_IOinto our ownread_iterhandler") introduced several regressions for bdev DIO:1. read spanning EOF always returns 0 instead of the number of bytes read. This is because "count" is assigned early and isn't updated when the iterator is truncated:
[...]
Applied, thanks!
[1/1] block: fix DIO handling regressions in blkdev_read_iter()
commit: 3e1f941dd9f33776b3df4e30f741fe445ff773f3
Best regards,
Signature · 1 line
Jens Axboe