On 10/20/21 07:36, Christoph Hellwig wrote:
On Tue, Oct 19, 2021 at 10:24:21PM +0100, Pavel Begunkov wrote:
+ bio = bio_alloc_kiocb(iocb, nr_pages, &blkdev_dio_pool);
+ dio = container_of(bio, struct blkdev_dio, bio);
+ __bio_set_dev(bio, bdev);
+ bio->bi_iter.bi_sector = pos >> 9;
SECTOR_SHIFT.
+ bio->bi_write_hint = iocb->ki_hint;
+ bio->bi_end_io = blkdev_bio_end_io_async;
+ bio->bi_ioprio = iocb->ki_ioprio;
+ dio->flags = 0;
+ dio->iocb = iocb;
+
+ ret = bio_iov_iter_get_pages(bio, iter);
+ if (unlikely(ret)) {
+ bio->bi_status = BLK_STS_IOERR;
+ bio_endio(bio);
+ return BLK_STS_IOERR;
This function does not return a blk_status_t, so this is wrong (and
sparse should have complained). I also don't think the error path
here should go hrough the bio for error handling but just do a put and
return the error.
My bad, following __blkdev_direct_IO() it was intended to be
blk_status_to_errno(BLK_STS_IOERR), but just return is much
better.
+ if (iov_iter_rw(iter) == READ) {
+ bio->bi_opf = REQ_OP_READ;
+ if (iter_is_iovec(iter)) {
+ dio->flags |= DIO_SHOULD_DIRTY;
+ bio_set_pages_dirty(bio);
+ }
+ } else {
+ bio->bi_opf = dio_bio_write_op(iocb);
+ task_io_account_write(bio->bi_iter.bi_size);
+ }
+
+ if (iocb->ki_flags & IOCB_NOWAIT)
+ bio->bi_opf |= REQ_NOWAIT;
This code is entirely duplicated, pleae move it into an (inline) helper.
I'll try it out, thanks
+ /*
+ * Don't plug for HIPRI/polled IO, as those should go straight
+ * to issue
+ */
This comment seems misplaced as the function does not use plugging at
all.
will kill it
--
Pavel Begunkov