On Thu, Dec 19, 2024 at 05:39:11PM +0000, Christoph Hellwig wrote:
> Split out the struct iomap-dio level final completion from
> iomap_dio_bio_end_io into a helper to clean up the code and make it
> reusable.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/iomap/direct-io.c | 76 ++++++++++++++++++++++----------------------
> 1 file changed, 38 insertions(+), 38 deletions(-)
>
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 641649a04614..ed658eb09a1a 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -165,43 +165,31 @@ static inline void iomap_dio_set_error(struct iomap_dio *dio, int ret)
> cmpxchg(&dio->error, 0, ret);
> }
>
> -void iomap_dio_bio_end_io(struct bio *bio)
> +/*
> + * Called when dio->ref reaches zero from and I/O completion.
an I/O completion
This hoist looks fine to me, so with that fixed:
Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
--D
> + */
> +static void iomap_dio_done(struct iomap_dio *dio)
> {
> - struct iomap_dio *dio = bio->bi_private;
> - bool should_dirty = (dio->flags & IOMAP_DIO_DIRTY);
> struct kiocb *iocb = dio->iocb;
>
> - if (bio->bi_status)
> - iomap_dio_set_error(dio, blk_status_to_errno(bio->bi_status));
> - if (!atomic_dec_and_test(&dio->ref))
> - goto release_bio;
> -
> - /*
> - * Synchronous dio, task itself will handle any completion work
> - * that needs after IO. All we need to do is wake the task.
> - */
> if (dio->wait_for_completion) {
> + /*
> + * Synchronous I/O, task itself will handle any completion work
> + * that needs after IO. All we need to do is wake the task.
> + */
> struct task_struct *waiter = dio->submit.waiter;
>
> WRITE_ONCE(dio->submit.waiter, NULL);
> blk_wake_io_task(waiter);
> - goto release_bio;
> - }
> -
> - /*
> - * Flagged with IOMAP_DIO_INLINE_COMP, we can complete it inline
> - */
> - if (dio->flags & IOMAP_DIO_INLINE_COMP) {
> + } else if (dio->flags & IOMAP_DIO_INLINE_COMP) {
> WRITE_ONCE(iocb->private, NULL);
> iomap_dio_complete_work(&dio->aio.work);
> - goto release_bio;
> - }
> -
> - /*
> - * If this dio is flagged with IOMAP_DIO_CALLER_COMP, then schedule
> - * our completion that way to avoid an async punt to a workqueue.
> - */
> - if (dio->flags & IOMAP_DIO_CALLER_COMP) {
> + } else if (dio->flags & IOMAP_DIO_CALLER_COMP) {
> + /*
> + * If this dio is flagged with IOMAP_DIO_CALLER_COMP, then
> + * schedule our completion that way to avoid an async punt to a
> + * workqueue.
> + */
> /* only polled IO cares about private cleared */
> iocb->private = dio;
> iocb->dio_complete = iomap_dio_deferred_complete;
> @@ -219,19 +207,31 @@ void iomap_dio_bio_end_io(struct bio *bio)
> * issuer.
> */
> iocb->ki_complete(iocb, 0);
> - goto release_bio;
> + } else {
> + struct inode *inode = file_inode(iocb->ki_filp);
> +
> + /*
> + * Async DIO completion that requires filesystem level
> + * completion work gets punted to a work queue to complete as
> + * the operation may require more IO to be issued to finalise
> + * filesystem metadata changes or guarantee data integrity.
> + */
> + INIT_WORK(&dio->aio.work, iomap_dio_complete_work);
> + queue_work(inode->i_sb->s_dio_done_wq, &dio->aio.work);
> }
> +}
> +
> +void iomap_dio_bio_end_io(struct bio *bio)
> +{
> + struct iomap_dio *dio = bio->bi_private;
> + bool should_dirty = (dio->flags & IOMAP_DIO_DIRTY);
> +
> + if (bio->bi_status)
> + iomap_dio_set_error(dio, blk_status_to_errno(bio->bi_status));
> +
> + if (atomic_dec_and_test(&dio->ref))
> + iomap_dio_done(dio);
>
> - /*
> - * Async DIO completion that requires filesystem level completion work
> - * gets punted to a work queue to complete as the operation may require
> - * more IO to be issued to finalise filesystem metadata changes or
> - * guarantee data integrity.
> - */
> - INIT_WORK(&dio->aio.work, iomap_dio_complete_work);
> - queue_work(file_inode(iocb->ki_filp)->i_sb->s_dio_done_wq,
> - &dio->aio.work);
> -release_bio:
> if (should_dirty) {
> bio_check_pages_dirty(bio);
> } else {
> --
> 2.45.2
>
>
