Re: [PATCH] blk-mq: implement queue quiesce via percpu_ref for BLK_MQ_F_BLOCKING

[Jens Axboe <axboe@xxxxxxxxx>]

On 8/19/20 9:02 PM, Ming Lei wrote:
> @@ -699,24 +696,21 @@ void blk_mq_complete_request(struct request *rq)
>  }
>  EXPORT_SYMBOL(blk_mq_complete_request);
>  
> -static void hctx_unlock(struct blk_mq_hw_ctx *hctx, int srcu_idx)
> -	__releases(hctx->srcu)
> +static void hctx_unlock(struct blk_mq_hw_ctx *hctx)
>  {
>  	if (!(hctx->flags & BLK_MQ_F_BLOCKING))
>  		rcu_read_unlock();
>  	else
> -		srcu_read_unlock(hctx->srcu, srcu_idx);
> +		percpu_ref_put(&hctx->queue->dispatch_counter);
>  }
>  
> -static void hctx_lock(struct blk_mq_hw_ctx *hctx, int *srcu_idx)
> -	__acquires(hctx->srcu)
> +static inline bool hctx_lock(struct blk_mq_hw_ctx *hctx)
>  {
>  	if (!(hctx->flags & BLK_MQ_F_BLOCKING)) {
> -		/* shut up gcc false positive */
> -		*srcu_idx = 0;
>  		rcu_read_lock();
> +		return true;
>  	} else
> -		*srcu_idx = srcu_read_lock(hctx->srcu);
> +		return percpu_ref_tryget_live(&hctx->queue->dispatch_counter);
>  }

I don't mind the !flags checking, since this is (by far) the hot path.
I would make it look like:

static inline bool hctx_lock(struct blk_mq_hw_ctx *hctx)
{
	if (!(hctx->flags & BLK_MQ_F_BLOCKING)) {
		rcu_read_lock();
		return true;
	}

	return percpu_ref_tryget_live(&hctx->queue->dispatch_counter);
}

to make that perfectly clear. You can do the same for the unlock side so
they at least look identical in terms of locking.

Not too many comments on the rest, I think this is a nice cleanup too,
and getting rid of the srcu usage is a nice win on the BLOCKING side.

-- 
Jens Axboe