[PATCH 1/1] block_dev: Fixed direct I/O bio sector calculation

[PATCH 1/1] block_dev: Fixed direct I/O bio sector calculation

[Damien Le Moal]

A direct I/O alignment must be always checked against the device blocks size,
but the I/O offset (bio->bi_iter.bi_sector must always use 512B sector unit, and
not the actual logical block size.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 fs/block_dev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 35cc494..e49fb79 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -226,7 +226,7 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter,
 	bio.bi_max_vecs = nr_pages;
 	bio.bi_io_vec = vecs;
 	bio.bi_bdev = bdev;
-	bio.bi_iter.bi_sector = pos >> blkbits;
+	bio.bi_iter.bi_sector = pos >> 9;
 	bio.bi_private = current;
 	bio.bi_end_io = blkdev_bio_end_io_simple;
 
@@ -358,7 +358,7 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
 
 	for (;;) {
 		bio->bi_bdev = bdev;
-		bio->bi_iter.bi_sector = pos >> blkbits;
+		bio->bi_iter.bi_sector = pos >> 9;
 		bio->bi_private = dio;
 		bio->bi_end_io = blkdev_bio_end_io;
 
-- 
2.7.4

Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:

This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.

Re: [PATCH 1/1] block_dev: Fixed direct I/O bio sector calculation

[Christoph Hellwig]

On Tue, Nov 22, 2016 at 03:38:49PM +0900, Damien Le Moal wrote:
> A direct I/O alignment must be always checked against the device blocks size,
> but the I/O offset (bio->bi_iter.bi_sector must always use 512B sector unit, and
> not the actual logical block size.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>

Ooops, yeah..

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/1] block_dev: Fixed direct I/O bio sector calculation

[Jens Axboe]

On 11/21/2016 11:38 PM, Damien Le Moal wrote:
> A direct I/O alignment must be always checked against the device blocks size,
> but the I/O offset (bio->bi_iter.bi_sector must always use 512B sector unit, and
> not the actual logical block size.

Oops indeed, that's not great... Added for 4.10.

While there, we can then also get rid of the blkbits variable, and (more
importantly), the call to blksize_bits():

diff --git a/fs/block_dev.c b/fs/block_dev.c
index e49fb797e447..b0c790a19db9 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -202,7 +202,6 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct 
iov_iter *iter,
  {
  	struct file *file = iocb->ki_filp;
  	struct block_device *bdev = I_BDEV(bdev_file_inode(file));
-	unsigned blkbits = blksize_bits(bdev_logical_block_size(bdev));
  	struct bio_vec inline_vecs[DIO_INLINE_BIO_VECS], *vecs, *bvec;
  	loff_t pos = iocb->ki_pos;
  	bool should_dirty = false;
@@ -211,7 +210,8 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct 
iov_iter *iter,
  	blk_qc_t qc;
  	int i;

-	if ((pos | iov_iter_alignment(iter)) & ((1 << blkbits) - 1))
+	if ((pos | iov_iter_alignment(iter)) &
+	    (bdev_logical_block_size(bdev) - 1))
  		return -EINVAL;

  	if (nr_pages <= DIO_INLINE_BIO_VECS)
@@ -331,7 +331,6 @@ __blkdev_direct_IO(struct kiocb *iocb, struct 
iov_iter *iter, int nr_pages)
  	struct file *file = iocb->ki_filp;
  	struct inode *inode = bdev_file_inode(file);
  	struct block_device *bdev = I_BDEV(inode);
-	unsigned blkbits = blksize_bits(bdev_logical_block_size(bdev));
  	struct blkdev_dio *dio;
  	struct bio *bio;
  	bool is_read = (iov_iter_rw(iter) == READ);
@@ -339,7 +338,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct 
iov_iter *iter, int nr_pages)
  	blk_qc_t qc = BLK_QC_T_NONE;
  	int ret;

-	if ((pos | iov_iter_alignment(iter)) & ((1 << blkbits) - 1))
+	if ((pos | iov_iter_alignment(iter)) &
+	    (bdev_logical_block_size(bdev) - 1))
  		return -EINVAL;

  	bio = bio_alloc_bioset(GFP_KERNEL, nr_pages, blkdev_dio_pool);


-- 
Jens Axboe