Re: [PATCH] dm crypt: fix lost ioprio when queuing crypto bios from task with ioprio

[Jens Axboe <axboe@kernel.dk>]

 > diff --git a/block/bio.c b/block/bio.c
> index 0c2208a5446d..ed68fdd78547 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -647,6 +647,30 @@ struct bio *bio_clone_fast(struct bio *bio, gfp_t gfp_mask, struct bio_set *bs)
>  }
>  EXPORT_SYMBOL(bio_clone_fast);
>  
> +/**
> + *	bio_set_task_prio - set bio's ioprio to task's ioprio, if any.
> + *	@bio: bio to set the ioprio of, can be NULL
> + *	@task: task of interest
> + *	@gfp_flags: allocation flags, used if allocation is necessary
> + *	@node: allocation node, used if allocation is necessary
> + */
> +void bio_set_task_prio(struct bio *bio, struct task_struct *task,
> +		       gfp_t gfp_flags, int node)
> +{
> +	struct io_context *ioc;
> +
> +	if (!bio)
> +		return;
> +
> +	ioc = get_task_io_context(current, gfp_flags, node);
> +	if (ioc) {
> +		if (ioprio_valid(ioc->ioprio))
> +			bio_set_prio(bio, ioc->ioprio);
> +		put_io_context(ioc);
> +	}
> +}
> +EXPORT_SYMBOL(bio_set_task_prio);
>

Shouldn't this just be a lookup, not a create io context? If the io
priority has been set, the ioc would exist anyway.

void bio_set_prio_from_current(struct bio *bio)
{
	struct io_context *ioc = current->io_context;

	if (ioc && ioprio_valid(ioc->ioprio))
		bio_set_prio(bio, ioc->ioprio);
}

and rename it like so. I don't like passing in the task, and all the
users are 'current' anyway. Passing in a task_struct implies that we
could have looked it up and would need a reference to it.

And don't make it pass in bio == NULL, that's just odd.

Apart from that, looks fine ;-)

-- 
Jens Axboe