Hi Ming

Currently, blk_mq_update_dispatch_busy is hooked in blk_mq_dispatch_rq_list
and __blk_mq_issue_directly. blk_mq_update_dispatch_busy could be invoked on 
multiple
cpus concurrently. But there is not any protection on the hctx->dispatch_busy. 
We cannot
ensure the update on the dispatch_busy atomically.


Look at the test result after applied the debug patch below:

             fio-1761  [000] ....   227.246251: 
blk_mq_update_dispatch_busy.part.50: old 0 ewma 2 cur 2
             fio-1766  [004] ....   227.246252: 
blk_mq_update_dispatch_busy.part.50: old 2 ewma 1 cur 1
             fio-1755  [000] ....   227.246366: 
blk_mq_update_dispatch_busy.part.50: old 1 ewma 0 cur 0
             fio-1754  [003] ....   227.266050: 
blk_mq_update_dispatch_busy.part.50: old 2 ewma 3 cur 3
             fio-1763  [007] ....   227.266050: 
blk_mq_update_dispatch_busy.part.50: old 0 ewma 2 cur 2
             fio-1761  [000] ....   227.266051: 
blk_mq_update_dispatch_busy.part.50: old 3 ewma 2 cur 2
             fio-1766  [004] ....   227.266051: 
blk_mq_update_dispatch_busy.part.50: old 3 ewma 2 cur 2
             fio-1760  [005] ....   227.266165: 
blk_mq_update_dispatch_busy.part.50: old 2 ewma 1 cur 1

--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1088,11 +1088,12 @@ static bool blk_mq_mark_tag_wait(struct blk_mq_hw_ctx 
*hctx,
 static void blk_mq_update_dispatch_busy(struct blk_mq_hw_ctx *hctx, bool busy)
 {
        unsigned int ewma;
+       unsigned int old;
 
        if (hctx->queue->elevator)
                return;
 
-       ewma = hctx->dispatch_busy;
+       old = ewma = hctx->dispatch_busy;
 
        if (!ewma && !busy)
                return;
@@ -1103,6 +1104,8 @@ static void blk_mq_update_dispatch_busy(struct 
blk_mq_hw_ctx *hctx, bool busy)
        ewma /= BLK_MQ_DISPATCH_BUSY_EWMA_WEIGHT;
 
        hctx->dispatch_busy = ewma;
+
+       trace_printk("old %u ewma %u cur %u\n", old, ewma, 
READ_ONCE(hctx->dispatch_busy));
 }


Is it expected ?

Thanks
Jianchao

Reply via email to