Re: [PATCH] blk-mq: Use non-atomic operations to manipulate the sw-ctx busy bits

[Omar Sandoval <osandov@osandov.com>]

On Wed, Aug 31, 2016 at 03:03:24PM -0700, Bart Van Assche wrote:
> Protect sw-ctx busy bit manipulations via the sw queue lock.
> This allows to convert the atomic bit operations into slightly
> faster non-atomic operations. Document that blk_mq_run_hw_queues()
> tests these bits without holding the sw queue lock.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Sagi Grimberg <sagi@grimberg.me>
> ---
>  block/blk-mq.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 13f5a6c..0dcafa6 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -66,8 +66,9 @@ static void blk_mq_hctx_mark_pending(struct blk_mq_hw_ctx *hctx,
>  {
>  	struct blk_align_bitmap *bm = get_bm(hctx, ctx);
>  
> +	lockdep_assert_held(&ctx->lock);
>  	if (!test_bit(CTX_TO_BIT(hctx, ctx), &bm->word))
> -		set_bit(CTX_TO_BIT(hctx, ctx), &bm->word);
> +		__set_bit(CTX_TO_BIT(hctx, ctx), &bm->word);
>  }
>  
>  static void blk_mq_hctx_clear_pending(struct blk_mq_hw_ctx *hctx,
> @@ -75,7 +76,8 @@ static void blk_mq_hctx_clear_pending(struct blk_mq_hw_ctx *hctx,
>  {
>  	struct blk_align_bitmap *bm = get_bm(hctx, ctx);
>  
> -	clear_bit(CTX_TO_BIT(hctx, ctx), &bm->word);
> +	lockdep_assert_held(&ctx->lock);
> +	__clear_bit(CTX_TO_BIT(hctx, ctx), &bm->word);
>  }

NAK, we can't protect each bit with a separate lock, multiple software
queues share the bitmap. There's a race if we do non-atomic bit
operations on two software queues with bits in the same word.

-- 
Omar