Re: [PATCHv2 3/3] block: relax direct io memory alignment
From: Christoph Hellwig <hch@lst.de>
Date: 2022-05-19 07:38:20
Also in:
linux-fsdevel
quoted hunk ↗ jump to hunk
@@ -1207,6 +1207,7 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter) { unsigned short nr_pages = bio->bi_max_vecs - bio->bi_vcnt; unsigned short entries_left = bio->bi_max_vecs - bio->bi_vcnt; + struct request_queue *q = bdev_get_queue(bio->bi_bdev); struct bio_vec *bv = bio->bi_io_vec + bio->bi_vcnt; struct page **pages = (struct page **)bv; bool same_page = false;@@ -1223,6 +1224,8 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter) pages += entries_left * (PAGE_PTRS_PER_BVEC - 1); size = iov_iter_get_pages(iter, pages, LONG_MAX, nr_pages, &offset); + if (size > 0) + size = ALIGN_DOWN(size, queue_logical_block_size(q));
So if we do get a size that is not logical block size alignment here, we reduce it to the block size aligned one below. Why do we do that?
+ if ((pos | iov_iter_count(iter)) & (bdev_logical_block_size(bdev) - 1)) + return -EINVAL; + if (iov_iter_alignment(iter) & bdev_dma_alignment(bdev)) return -EINVAL;
Can we have a little inline helper for these checks instead of duplicating them three times?
quoted hunk ↗ jump to hunk
diff --git a/fs/direct-io.c b/fs/direct-io.c index 840752006f60..64cc176be60c 100644 --- a/fs/direct-io.c +++ b/fs/direct-io.c@@ -1131,7 +1131,7 @@ ssize_t __blockdev_direct_IO(struct kiocb *iocb, struct inode *inode, struct dio_submit sdio = { 0, }; struct buffer_head map_bh = { 0, }; struct blk_plug plug; - unsigned long align = offset | iov_iter_alignment(iter); + unsigned long align = iov_iter_alignment(iter);
I'd much prefer to not just relax this for random file systems, and especially not the legacy direct I/O code. I think we can eventually do iomap, but only after an audit and test of each file system, which might require a new IOMAP_DIO_* flag at least initially.
+static inline unsigned int bdev_dma_alignment(struct block_device *bdev)
+{
+ return queue_dma_alignment(bdev_get_queue(bdev));
+}Plase do this in a separate patch.