RE: [EXT] [PATCH RFC v2 2/5] ufs: Use reserved tags for TMFs

From: "Bean Huo (beanhuo)" <beanhuo@micron.com>
To: Bart Van Assche <bvanassche@acm.org>,
	Avri Altman <avri.altman@wdc.com>,
	"James E . J . Bottomley" <jejb@linux.vnet.ibm.com>
Cc: "Martin K . Petersen" <martin.petersen@oracle.com>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	Christoph Hellwig <hch@lst.de>,
	Yaniv Gardi <ygardi@codeaurora.org>,
	Subhash Jadavani <subhashj@codeaurora.org>,
	Stanley Chu <stanley.chu@mediatek.com>,
	Tomas Winkler <tomas.winkler@intel.com>
Subject: RE: [EXT] [PATCH RFC v2 2/5] ufs: Use reserved tags for TMFs
Date: Tue, 5 Nov 2019 11:58:42 +0000	[thread overview]
Message-ID: <BN7PR08MB56849F96E16115321F44F257DB7E0@BN7PR08MB5684.namprd08.prod.outlook.com> (raw)
In-Reply-To: <20191105004226.232635-3-bvanassche@acm.org>

Hi, Bart

> 
> Reserved tags are numerically lower than non-reserved tags. Compensate the
> change caused by reserving tags by subtracting the number of reserved tags
> from the tag number assigned by the block layer.
> 
> Cc: Yaniv Gardi <ygardi@codeaurora.org>
> Cc: Subhash Jadavani <subhashj@codeaurora.org>
> Cc: Stanley Chu <stanley.chu@mediatek.com>
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/ufs/ufshcd.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index
> 9fc05a535624..3e3c6257a343 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -2402,7 +2402,7 @@ static int ufshcd_queuecommand(struct Scsi_Host
> *host, struct scsi_cmnd *cmd)
> 
>  	hba = shost_priv(host);
> 
> -	tag = cmd->request->tag;
> +	tag = cmd->request->tag - hba->nutmrs;
>  	if (!ufshcd_valid_tag(hba, tag)) {
>  		dev_err(hba->dev,
>  			"%s: invalid command tag %d: cmd=0x%p, cmd-
> >request=0x%p", @@ -5965,7 +5965,8 @@ static int
> ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
> 
>  	host = cmd->device->host;
>  	hba = shost_priv(host);
> -	tag = cmd->request->tag;
> +	tag = cmd->request->tag - hba->nutmrs;
> +	WARN_ON_ONCE(!ufshcd_valid_tag(hba, tag));

Changing request tag number here is not proper way, we have trace tool using this tag to track request from block,
SCSI to UFS layer. If tags being changed in UFS driver, there will be a confusion. 

> 
>  	lrbp = &hba->lrb[tag];
>  	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, 0, UFS_LOGICAL_RESET,
> &resp); @@ -6036,7 +6037,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
> 
>  	host = cmd->device->host;
>  	hba = shost_priv(host);
> -	tag = cmd->request->tag;
> +	tag = cmd->request->tag - hba->nutmrs;
>  	lrbp = &hba->lrb[tag];
>  	if (!ufshcd_valid_tag(hba, tag)) {
>  		dev_err(hba->dev,
> @@ -8320,7 +8321,8 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem
> *mmio_base, unsigned int irq)
>  	/* Configure LRB */
>  	ufshcd_host_memory_configure(hba);
> 
> -	host->can_queue = hba->nutrs;
> +	host->can_queue = hba->nutrs + hba->nutmrs;
> +	host->reserved_tags = hba->nutmrs;

hba->nutmrs is for task management, relevant door-bell register is bit0-bit7 validate. 
If my understanding is correct, UFS task management requests normally issued by UFS layer, rather than upper layers. 
Before issuing task management request, UFS driver layer will apply a free slot, see ufshcd_get_tm_free_slot().
I don't think it is necessary to change host->can_queue size by adding task management depth hba->nutmrs.

>  	host->cmd_per_lun = hba->nutrs;
>  	host->max_id = UFSHCD_MAX_ID;
>  	host->max_lun = UFS_MAX_LUNS;
> --
> 2.24.0.rc1.363.gb1bccd3e3d-goog

//Bean