Re: [PATCH v3 10/17] scsi: ufs: Fix a deadlock in the error handler

From: Adrian Hunter <adrian.hunter@intel.com>
To: Bart Van Assche <bvanassche@acm.org>,
	"Martin K . Petersen" <martin.petersen@oracle.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>,
	linux-scsi@vger.kernel.org,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	Bean Huo <beanhuo@micron.com>, Avri Altman <avri.altman@wdc.com>,
	Can Guo <cang@codeaurora.org>,
	Stanley Chu <stanley.chu@mediatek.com>,
	Asutosh Das <asutoshd@codeaurora.org>,
	Keoseong Park <keosung.park@samsung.com>
Subject: Re: [PATCH v3 10/17] scsi: ufs: Fix a deadlock in the error handler
Date: Thu, 2 Dec 2021 10:25:51 +0200	[thread overview]
Message-ID: <e65a5cd1-7397-c04e-953d-194781a79f3e@intel.com> (raw)
In-Reply-To: <ab84bffe-fd84-82c6-d4f2-3ee73e7a850e@acm.org>

On 01/12/2021 23:26, Bart Van Assche wrote:
> On 12/1/21 5:48 AM, Adrian Hunter wrote:
>> I think cmd_queue is not used anymore after this.
> 
> Let's remove cmd_queue via a separate patch. I have started testing this patch:
> 
> Subject: [PATCH] scsi: ufs: Remove hba->cmd_queue
> 
> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/scsi/ufs/ufshcd.c | 11 +----------
>  drivers/scsi/ufs/ufshcd.h |  2 --
>  2 files changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 5b3efc880246..d379c2b0c058 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -9409,7 +9409,6 @@ void ufshcd_remove(struct ufs_hba *hba)
>      ufs_sysfs_remove_nodes(hba->dev);
>      blk_cleanup_queue(hba->tmf_queue);
>      blk_mq_free_tag_set(&hba->tmf_tag_set);
> -    blk_cleanup_queue(hba->cmd_queue);
>      scsi_remove_host(hba->host);
>      /* disable interrupts */
>      ufshcd_disable_intr(hba, hba->intr_mask);
> @@ -9630,12 +9629,6 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
>          goto out_disable;
>      }
> 
> -    hba->cmd_queue = blk_mq_init_queue(&hba->host->tag_set);
> -    if (IS_ERR(hba->cmd_queue)) {
> -        err = PTR_ERR(hba->cmd_queue);
> -        goto out_remove_scsi_host;
> -    }
> -
>      hba->tmf_tag_set = (struct blk_mq_tag_set) {
>          .nr_hw_queues    = 1,
>          .queue_depth    = hba->nutmrs,
> @@ -9644,7 +9637,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
>      };
>      err = blk_mq_alloc_tag_set(&hba->tmf_tag_set);
>      if (err < 0)
> -        goto free_cmd_queue;
> +        goto out_remove_scsi_host;
>      hba->tmf_queue = blk_mq_init_queue(&hba->tmf_tag_set);
>      if (IS_ERR(hba->tmf_queue)) {
>          err = PTR_ERR(hba->tmf_queue);
> @@ -9713,8 +9706,6 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
>      blk_cleanup_queue(hba->tmf_queue);
>  free_tmf_tag_set:
>      blk_mq_free_tag_set(&hba->tmf_tag_set);
> -free_cmd_queue:
> -    blk_cleanup_queue(hba->cmd_queue);
>  out_remove_scsi_host:
>      scsi_remove_host(hba->host);
>  out_disable:
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 411c6015bbfe..88c20f3608c2 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -738,7 +738,6 @@ struct ufs_hba_monitor {
>   * @host: Scsi_Host instance of the driver
>   * @dev: device handle
>   * @lrb: local reference block
> - * @cmd_queue: Used to allocate command tags from hba->host->tag_set.
>   * @outstanding_tasks: Bits representing outstanding task requests
>   * @outstanding_lock: Protects @outstanding_reqs.
>   * @outstanding_reqs: Bits representing outstanding transfer requests
> @@ -805,7 +804,6 @@ struct ufs_hba {
> 
>      struct Scsi_Host *host;
>      struct device *dev;
> -    struct request_queue *cmd_queue;
>      /*
>       * This field is to keep a reference to "scsi_device" corresponding to
>       * "UFS device" W-LU.