On Thu, Mar 02, 2017 at 09:13:30AM -0700, Jens Axboe wrote:
> I was worried about that. How about the below? We need to grab the lock
> at some point for legacy scheduling, but the ordering should be correct.
Makes sense, now the locking is consistent with the other place we call
ioc_exit_icq(). One nit below, and you can add
Reviewed-by: Omar Sandoval <osandov@xxxxxx>
> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index b12f9c87b4c3..6fd633b5d567 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -54,7 +54,7 @@ static void ioc_exit_icq(struct io_cq *icq)
> icq->flags |= ICQ_EXITED;
> }
>
> -/* Release an icq. Called with both ioc and q locked. */
> +/* Release an icq. Called with ioc locked. */
For ioc_exit_icq(), we have the more explicit comment
/*
* Exit an icq. Called with both ioc and q locked for sq, only ioc locked for
* mq.
*/
Could you document that here, too?
> static void ioc_destroy_icq(struct io_cq *icq)
> {
> struct io_context *ioc = icq->ioc;
> @@ -62,7 +62,6 @@ static void ioc_destroy_icq(struct io_cq *icq)
> struct elevator_type *et = q->elevator->type;
>
> lockdep_assert_held(&ioc->lock);
> - lockdep_assert_held(q->queue_lock);
>
> radix_tree_delete(&ioc->icq_tree, icq->q->id);
> hlist_del_init(&icq->ioc_node);
> @@ -222,24 +221,40 @@ void exit_io_context(struct task_struct *task)
> put_io_context_active(ioc);
> }
