On 01/10/2018 08:39 PM, Bart Van Assche wrote: > Both add_wait_queue() and blk_mq_dispatch_wake() protect wait queue > manipulations with the wait queue lock. Hence also protect the > !list_empty(&wait->entry) test with the wait queue lock instead of > the hctx lock. > > Signed-off-by: Bart Van Assche <[email protected]> > Cc: Christoph Hellwig <[email protected]> > Cc: Omar Sandoval <[email protected]> > Cc: Hannes Reinecke <[email protected]> > Cc: Johannes Thumshirn <[email protected]> > --- > block/blk-mq.c | 19 +++++++++---------- > 1 file changed, 9 insertions(+), 10 deletions(-) > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index e770e8814f60..d5313ce60836 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -1184,7 +1184,7 @@ static bool blk_mq_mark_tag_wait(struct blk_mq_hw_ctx > **hctx, > bool shared_tags = (this_hctx->flags & BLK_MQ_F_TAG_SHARED) != 0; > struct sbq_wait_state *ws; > wait_queue_entry_t *wait; > - bool ret; > + bool on_wait_list, ret; > > if (!shared_tags) { > if (!test_bit(BLK_MQ_S_SCHED_RESTART, &this_hctx->state)) > @@ -1204,13 +1204,15 @@ static bool blk_mq_mark_tag_wait(struct blk_mq_hw_ctx > **hctx, > if (!list_empty_careful(&wait->entry)) > return false; > > - spin_lock(&this_hctx->lock); > - if (!list_empty(&wait->entry)) { > - spin_unlock(&this_hctx->lock); > + ws = bt_wait_ptr(&this_hctx->tags->bitmap_tags, this_hctx); > + > + spin_lock_irq(&ws->wait.lock); > + on_wait_list = !list_empty(&wait->entry); > + spin_unlock_irq(&ws->wait.lock); > + > + if (on_wait_list) > return false; > - } > > - ws = bt_wait_ptr(&this_hctx->tags->bitmap_tags, this_hctx); > add_wait_queue(&ws->wait, wait); > /* > * It's possible that a tag was freed in the window between the I'm actually not that convinced with this change; originally we had been checking if it's on the wait list, and only _then_ call bt_wait_ptr(). Now we call bt_wait_ptr() always, meaning we run a chance of increasing the bitmap_tags wait pointer without actually using it. Looking at the code I'm not sure this is the correct way of using it ...
Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking [email protected] +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
