Re: [PATCH rfc v2 1/4] blk-mq: add async quiesce interface for blocking hw queues

[Chao Leng <lengchao@huawei.com>]

Looks great. One suggest: srcu provide the batch sync mechanism,
it may be more generic. Weakness: for the same srcu, concurrent batch
waiting is not supported. The code just for TINY_SRCU:

---
  block/blk-mq.c           | 24 ++++++++++++++++++++++++
  include/linux/srcu.h     |  2 ++
  include/linux/srcutiny.h |  1 +
  kernel/rcu/srcutiny.c    | 16 ++++++++++++++++
  4 files changed, 43 insertions(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4e0d173beaa3..97dabcf2cab8 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -235,6 +235,30 @@ void blk_mq_quiesce_queue(struct request_queue *q)
  }
  EXPORT_SYMBOL_GPL(blk_mq_quiesce_queue);

+void blk_mq_quiesce_blocking_queue_async(struct request_queue *q)
+{
+    struct blk_mq_hw_ctx *hctx;
+    unsigned int i;
+
+    blk_mq_quiesce_queue_nowait(q);
+
+    queue_for_each_hw_ctx(q, hctx, i)
+        if (hctx->flags & BLK_MQ_F_BLOCKING)
+            synchronize_srcu_async(hctx->srcu);
+}
+EXPORT_SYMBOL_GPL(blk_mq_quiesce_blocking_queue_async);
+
+void blk_mq_quiesce_blocking_queue_async_wait(struct request_queue *q)
+{
+    struct blk_mq_hw_ctx *hctx;
+    unsigned int i;
+
+    queue_for_each_hw_ctx(q, hctx, i)
+        if (hctx->flags & BLK_MQ_F_BLOCKING)
+            synchronize_srcu_async_wait(hctx->srcu);
+}
+EXPORT_SYMBOL_GPL(blk_mq_quiesce_blocking_queue_async_wait);
+
  /*
   * blk_mq_unquiesce_queue() - counterpart of blk_mq_quiesce_queue()
   * @q: request queue.
diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index e432cc92c73d..7e006e51ccf9 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -60,6 +60,8 @@ void cleanup_srcu_struct(struct srcu_struct *ssp);
  int __srcu_read_lock(struct srcu_struct *ssp) __acquires(ssp);
  void __srcu_read_unlock(struct srcu_struct *ssp, int idx) __releases(ssp);
  void synchronize_srcu(struct srcu_struct *ssp);
+void synchronize_srcu_async(struct srcu_struct *ssp);
+void synchronize_srcu_async_wait(struct srcu_struct *ssp);

  #ifdef CONFIG_DEBUG_LOCK_ALLOC

diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
index 5a5a1941ca15..3d7d871bef61 100644
--- a/include/linux/srcutiny.h
+++ b/include/linux/srcutiny.h
@@ -23,6 +23,7 @@ struct srcu_struct {
      struct rcu_head *srcu_cb_head;    /* Pending callbacks: Head. */
      struct rcu_head **srcu_cb_tail;    /* Pending callbacks: Tail. */
      struct work_struct srcu_work;    /* For driving grace periods. */
+    struct rcu_synchronize rcu_sync;
  #ifdef CONFIG_DEBUG_LOCK_ALLOC
      struct lockdep_map dep_map;
  #endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
index 6208c1dae5c9..6e1468175a45 100644
--- a/kernel/rcu/srcutiny.c
+++ b/kernel/rcu/srcutiny.c
@@ -190,6 +190,22 @@ void synchronize_srcu(struct srcu_struct *ssp)
  }
  EXPORT_SYMBOL_GPL(synchronize_srcu);

+void synchronize_srcu_async(struct srcu_struct *ssp)
+{
+    init_rcu_head(&ssp->rcu_sync.head);
+    init_completion(&ssp->rcu_sync.completion);
+    call_srcu(ssp, &ssp->rcu_sync.head, wakeme_after_rcu_batch);
+
+}
+EXPORT_SYMBOL_GPL(synchronize_srcu_async);
+
+void synchronize_srcu_async_wait(struct srcu_struct *ssp)
+{
+    wait_for_completion(&ssp->rcu_sync.completion);
+    destroy_rcu_head(&ssp->rcu_sync.head);
+}
+EXPORT_SYMBOL_GPL(synchronize_srcu_async_wait);
+
  /* Lockdep diagnostics.  */
  void __init rcu_scheduler_starting(void)
  {
-- 


On 2020/7/25 7:06, Sagi Grimberg wrote:
> Drivers that use blocking hw queues may have to quiesce a large amount
> of request queues at once (e.g. controller or adapter reset). These
> drivers would benefit from an async quiesce interface such that
> the can trigger quiesce asynchronously and wait for all in parallel.
>
> This leaves the synchronization responsibility to the driver, but adds
> a convenient interface to quiesce async and wait in a single pass.
>
> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
> ---
>   block/blk-mq.c         | 31 +++++++++++++++++++++++++++++++
>   include/linux/blk-mq.h |  4 ++++
>   2 files changed, 35 insertions(+)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index abcf590f6238..7326709ed2d1 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -209,6 +209,37 @@ void blk_mq_quiesce_queue_nowait(struct request_queue *q)
>   }
>   EXPORT_SYMBOL_GPL(blk_mq_quiesce_queue_nowait);
>   
> +void blk_mq_quiesce_blocking_queue_async(struct request_queue *q)
> +{
> +	struct blk_mq_hw_ctx *hctx;
> +	unsigned int i;
> +
> +	blk_mq_quiesce_queue_nowait(q);
> +
> +	queue_for_each_hw_ctx(q, hctx, i) {
> +		if (!(hctx->flags & BLK_MQ_F_BLOCKING))
> +			continue;
> +		init_completion(&hctx->rcu_sync.completion);
> +		init_rcu_head(&hctx->rcu_sync.head);
> +		call_srcu(hctx->srcu, &hctx->rcu_sync.head, wakeme_after_rcu);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(blk_mq_quiesce_blocking_queue_async);
> +
> +void blk_mq_quiesce_blocking_queue_async_wait(struct request_queue *q)
> +{
> +	struct blk_mq_hw_ctx *hctx;
> +	unsigned int i;
> +
> +	queue_for_each_hw_ctx(q, hctx, i) {
> +		if (!(hctx->flags & BLK_MQ_F_BLOCKING))
> +			continue;
> +		wait_for_completion(&hctx->rcu_sync.completion);
> +		destroy_rcu_head(&hctx->rcu_sync.head);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(blk_mq_quiesce_blocking_queue_async_wait);
> +
>   /**
>    * blk_mq_quiesce_queue() - wait until all ongoing dispatches have finished
>    * @q: request queue.
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 23230c1d031e..863b372d32aa 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -5,6 +5,7 @@
>   #include <linux/blkdev.h>
>   #include <linux/sbitmap.h>
>   #include <linux/srcu.h>
> +#include <linux/rcupdate_wait.h>
>   
>   struct blk_mq_tags;
>   struct blk_flush_queue;
> @@ -170,6 +171,7 @@ struct blk_mq_hw_ctx {
>   	 */
>   	struct list_head	hctx_list;
>   
> +	struct rcu_synchronize	rcu_sync;
>   	/**
>   	 * @srcu: Sleepable RCU. Use as lock when type of the hardware queue is
>   	 * blocking (BLK_MQ_F_BLOCKING). Must be the last member - see also
> @@ -532,6 +534,8 @@ int blk_mq_map_queues(struct blk_mq_queue_map *qmap);
>   void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);
>   
>   void blk_mq_quiesce_queue_nowait(struct request_queue *q);
> +void blk_mq_quiesce_blocking_queue_async(struct request_queue *q);
> +void blk_mq_quiesce_blocking_queue_async_wait(struct request_queue *q);
>   
>   unsigned int blk_mq_rq_cpu(struct request *rq);
>   

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme