On Sat, Mar 25, 2017 at 1:24 AM, Bart Van Assche <Bart.VanAssche@xxxxxxxxxxx> wrote: > On Fri, 2017-03-24 at 20:36 +0800, Ming Lei wrote: >> Without the barrier, reading DEAD flag of .q_usage_counter >> and reading .mq_freeze_depth may be reordered, then the >> following wait_event_interruptible() may never return. >> >> Signed-off-by: Ming Lei <tom.leiming@xxxxxxxxx> >> --- >> block/blk-core.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/block/blk-core.c b/block/blk-core.c >> index ad388d5e309a..44eed17319c0 100644 >> --- a/block/blk-core.c >> +++ b/block/blk-core.c >> @@ -669,6 +669,14 @@ int blk_queue_enter(struct request_queue *q, bool nowait) >> if (nowait) >> return -EBUSY; >> >> + /* >> + * read pair of barrier in blk_mq_freeze_queue_start(), >> + * we need to order reading DEAD flag of .q_usage_counter >> + * and reading .mq_freeze_depth, otherwise the following >> + * wait may never return if the two read are reordered. >> + */ >> + smp_rmb(); >> + >> ret = wait_event_interruptible(q->mq_freeze_wq, >> !atomic_read(&q->mq_freeze_depth) || >> blk_queue_dying(q)); > > Hello Ming, > > The code looks fine to me but the comment not. You probably wanted to refer > to the "dying" flag instead of the "dead" flag? The read order has to be No, looks you misunderstand the issue. I mean the order between reading __PERCPU_REF_DEAD of .q_usage_counter and reading .mq_freeze_depth should be enhanced, especially it is in blk_queue_enter() vs. blk_mq_freeze_queue_start(). In the last patch, you will find the dying flag is mentioned in above comment after we call blk_freeze_queue_start() just after the dying flag is set. > enforced for the "dying" flag and q_usage_counter because of the order in > which these are set by blk_set_queue_dying(). As I explained, the dying flag should only be mentioned after we change the code in blk_set_queue_dying(). Thanks, Ming Lei
