On 11/21/19 11:08 PM, Bart Van Assche wrote:
> Instead of tracking which tags are in use in the ufs_hba.lrb_in_use
> bitmask, rely on the block layer tag allocation mechanism. This patch
> removes the following busy-waiting loop if ufshcd_issue_devman_upiu_cmd()
> and the block layer accidentally allocate the same tag for a SCSI request:
> * ufshcd_queuecommand() returns SCSI_MLQUEUE_HOST_BUSY.
> * The SCSI core requeues the SCSI command.
>
> Tested-by: Bean Huo <beanhuo@xxxxxxxxxx>
> Reviewed-by: Avri Altman <avri.altman@xxxxxxx>
> Cc: Can Guo <cang@xxxxxxxxxxxxxx>
> Cc: Stanley Chu <stanley.chu@xxxxxxxxxxxx>
> Cc: Avri Altman <avri.altman@xxxxxxx>
> Cc: Tomas Winkler <tomas.winkler@xxxxxxxxx>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
> drivers/scsi/ufs/ufshcd.c | 121 +++++++++++++++-----------------------
> drivers/scsi/ufs/ufshcd.h | 6 +-
> 2 files changed, 50 insertions(+), 77 deletions(-)
>
[ .. ]
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 2740f6941ec6..56b9da6db1cc 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -212,13 +212,11 @@ struct ufs_query {
> * @type: device management command type - Query, NOP OUT
> * @lock: lock to allow one command at a time
> * @complete: internal commands completion
> - * @tag_wq: wait queue until free command slot is available
> */
> struct ufs_dev_cmd {
> enum dev_cmd_type type;
> struct mutex lock;
> struct completion *complete;
> - wait_queue_head_t tag_wq;
> struct ufs_query query;
> };
>
> @@ -483,7 +481,7 @@ struct ufs_stats {
> * @host: Scsi_Host instance of the driver
> * @dev: device handle
> * @lrb: local reference block
> - * @lrb_in_use: lrb in use
> + * @cmd_queue: Used to allocate command tags from hba->host->tag_set.
> * @outstanding_tasks: Bits representing outstanding task requests
> * @outstanding_reqs: Bits representing outstanding transfer requests
> * @capabilities: UFS Controller Capabilities
> @@ -541,6 +539,7 @@ 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.
> @@ -561,7 +560,6 @@ struct ufs_hba {
> u32 ahit;
>
> struct ufshcd_lrb *lrb;
> - unsigned long lrb_in_use;
>
> unsigned long outstanding_tasks;
> unsigned long outstanding_reqs;
>
This is pretty similar to the 'reserved tags' patchset I send as a
proposal a while ago (cf 'scsi: enable reserved commands for LLDDs').
Shouldn't we resurrect this? Once the shared host_tags have gone in we
can leverage this from other dreivers, too ...
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
