On 03/02/2017 01:53 PM, Omar Sandoval wrote: > 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? Done, I've synced the two comments now. Thanks for the review! -- Jens Axboe