[PATCH 2/3] nvme: core: switch to non_owner variant of start_freeze/unfreeze queue

Ming Lei posted 3 patches 1 month ago
There is a newer version of this series
[PATCH 2/3] nvme: core: switch to non_owner variant of start_freeze/unfreeze queue
Posted by Ming Lei 1 month ago
nvme_start_freeze() and nvme_unfreeze() may be called from same context,
so switch them to call non_owner variant of start_freeze/unfreeze queue.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/nvme/host/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index ba6508455e18..06c1e4e8456f 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4871,7 +4871,7 @@ void nvme_unfreeze(struct nvme_ctrl *ctrl)
 
 	srcu_idx = srcu_read_lock(&ctrl->srcu);
 	list_for_each_entry_rcu(ns, &ctrl->namespaces, list)
-		blk_mq_unfreeze_queue(ns->queue);
+		blk_mq_unfreeze_queue_non_owner(ns->queue);
 	srcu_read_unlock(&ctrl->srcu, srcu_idx);
 	clear_bit(NVME_CTRL_FROZEN, &ctrl->flags);
 }
@@ -4913,7 +4913,7 @@ void nvme_start_freeze(struct nvme_ctrl *ctrl)
 	set_bit(NVME_CTRL_FROZEN, &ctrl->flags);
 	srcu_idx = srcu_read_lock(&ctrl->srcu);
 	list_for_each_entry_rcu(ns, &ctrl->namespaces, list)
-		blk_freeze_queue_start(ns->queue);
+		blk_freeze_queue_start_non_owner(ns->queue);
 	srcu_read_unlock(&ctrl->srcu, srcu_idx);
 }
 EXPORT_SYMBOL_GPL(nvme_start_freeze);
-- 
2.46.0
Re: [PATCH 2/3] nvme: core: switch to non_owner variant of start_freeze/unfreeze queue
Posted by Christoph Hellwig 1 month ago
On Wed, Oct 23, 2024 at 05:54:34PM +0800, Ming Lei wrote:
> @@ -4913,7 +4913,7 @@ void nvme_start_freeze(struct nvme_ctrl *ctrl)
>  	set_bit(NVME_CTRL_FROZEN, &ctrl->flags);
>  	srcu_idx = srcu_read_lock(&ctrl->srcu);
>  	list_for_each_entry_rcu(ns, &ctrl->namespaces, list)
> -		blk_freeze_queue_start(ns->queue);
> +		blk_freeze_queue_start_non_owner(ns->queue);

Maybe throw in a comment like:

/*
 * Will be unfrozen at I/O completion time when called by
 * nvme_passthru_start.
 */

so that it's clear why the non_owner version is used here.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Re: [PATCH 2/3] nvme: core: switch to non_owner variant of start_freeze/unfreeze queue
Posted by Ming Lei 1 month ago
On Wed, Oct 23, 2024 at 02:21:15PM +0200, Christoph Hellwig wrote:
> On Wed, Oct 23, 2024 at 05:54:34PM +0800, Ming Lei wrote:
> > @@ -4913,7 +4913,7 @@ void nvme_start_freeze(struct nvme_ctrl *ctrl)
> >  	set_bit(NVME_CTRL_FROZEN, &ctrl->flags);
> >  	srcu_idx = srcu_read_lock(&ctrl->srcu);
> >  	list_for_each_entry_rcu(ns, &ctrl->namespaces, list)
> > -		blk_freeze_queue_start(ns->queue);
> > +		blk_freeze_queue_start_non_owner(ns->queue);
> 
> Maybe throw in a comment like:
> 
> /*
>  * Will be unfrozen at I/O completion time when called by
>  * nvme_passthru_start.
>  */
> 
> so that it's clear why the non_owner version is used here.

There are one more such usage: 

- freeze in nvme_dev_disable()/apple_nvme_disable() from timeout work, but
unfreeze in nvme_reset_work()


Thanks,
Ming
Re: [PATCH 2/3] nvme: core: switch to non_owner variant of start_freeze/unfreeze queue
Posted by Christoph Hellwig 1 month ago
On Thu, Oct 24, 2024 at 09:43:24AM +0800, Ming Lei wrote:
> > so that it's clear why the non_owner version is used here.
> 
> There are one more such usage: 
> 
> - freeze in nvme_dev_disable()/apple_nvme_disable() from timeout work, but
> unfreeze in nvme_reset_work()

Then add it to the comment :)
Re: [PATCH 2/3] nvme: core: switch to non_owner variant of start_freeze/unfreeze queue
Posted by Ming Lei 1 month ago
On Thu, Oct 24, 2024 at 07:00:53AM +0200, Christoph Hellwig wrote:
> On Thu, Oct 24, 2024 at 09:43:24AM +0800, Ming Lei wrote:
> > > so that it's clear why the non_owner version is used here.
> > 
> > There are one more such usage: 
> > 
> > - freeze in nvme_dev_disable()/apple_nvme_disable() from timeout work, but
> > unfreeze in nvme_reset_work()
> 
> Then add it to the comment :)

OK, also nvme_passthru_start() and nvme_passthru_end() are always
paired, so they are actually not non_owner use.


Thanks, 
Ming