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.
> + 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.
> + /*
> + * 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.
