Re: dd hangs when reading large partitions

From: Bart Van Assche <bvanassche@acm.org>
To: Jens Axboe <axboe@kernel.dk>,
	"jianchao.wang" <jianchao.w.wang@oracle.com>,
	Marc Gonzalez <marc.w.gonzalez@free.fr>,
	fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-block <linux-block@vger.kernel.org>
Cc: SCSI <linux-scsi@vger.kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Jan Kara <jack@suse.com>, Christoph Hellwig <hch@infradead.org>,
	Joao Pinto <jpinto@synopsys.com>,
	Fujita Tomonori <fujita.tomonori@lab.ntt.co.jp>,
	Paolo Valente <paolo.valente@linaro.org>
Subject: Re: dd hangs when reading large partitions
Date: Fri, 18 Jan 2019 09:51:16 -0800	[thread overview]
Message-ID: <1547833876.102272.0.camel@acm.org> (raw)
In-Reply-To: <ef734b94-e72b-771f-350b-08d8054a58f3@kernel.dk>

On Fri, 2019-01-18 at 10:48 -0700, Jens Axboe wrote:
> It's UFS that totally buggy, if you look at its queuecommand, it does:
> 
>         if (!down_read_trylock(&hba->clk_scaling_lock))                         
>                 return SCSI_MLQUEUE_HOST_BUSY;
> 
> UFS either needs to get fixed up, or we'll want a way to do something like
> the below.
> 
> Marc, can you test this?
> 
> 
> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> index eaf329db3973..e28c3420a9d9 100644
> --- a/drivers/scsi/hosts.c
> +++ b/drivers/scsi/hosts.c
> @@ -412,6 +412,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
>  	shost->hostt = sht;
>  	shost->this_id = sht->this_id;
>  	shost->can_queue = sht->can_queue;
> +	shost->queue_may_block = sht->queue_may_block;
>  	shost->sg_tablesize = sht->sg_tablesize;
>  	shost->sg_prot_tablesize = sht->sg_prot_tablesize;
>  	shost->cmd_per_lun = sht->cmd_per_lun;
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index b13cc9288ba0..4e266af2871f 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1902,6 +1902,8 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
>  	shost->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE;
>  	shost->tag_set.flags |=
>  		BLK_ALLOC_POLICY_TO_MQ_FLAG(shost->hostt->tag_alloc_policy);
> +	if (shost->queue_may_blocK)
> +		shost->tag_set.flags |= BLK_MQ_F_BLOCKING;
>  	shost->tag_set.driver_data = shost;
>  
>  	return blk_mq_alloc_tag_set(&shost->tag_set);
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 9ba7671b84f8..9ab354e43630 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -6981,6 +6981,7 @@ static struct scsi_host_template ufshcd_driver_template = {
>  	.sg_tablesize		= SG_ALL,
>  	.cmd_per_lun		= UFSHCD_CMD_PER_LUN,
>  	.can_queue		= UFSHCD_CAN_QUEUE,
> +	.queue_may_block	= 1,
>  	.max_host_blocked	= 1,
>  	.track_queue_depth	= 1,
>  	.sdev_groups		= ufshcd_driver_groups,
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 6ca954e9f752..30aa7b6c4342 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -339,6 +339,11 @@ struct scsi_host_template {
>  	 */
>  	int can_queue;
>  
> +	/*
> +	 * If the ->queuecommand() ever blocks, this should be set
> +	 */
> +	int queue_may_block;
> +
>  	/*
>  	 * In many instances, especially where disconnect / reconnect are
>  	 * supported, our host also has an ID on the SCSI bus.  If this is
> @@ -584,6 +589,7 @@ struct Scsi_Host {
>  
>  	int this_id;
>  	int can_queue;
> +	int queue_may_block;
>  	short cmd_per_lun;
>  	short unsigned int sg_tablesize;
>  	short unsigned int sg_prot_tablesize;

Hi Jens,

Did you perhaps want to include a change from down_read_trylock() into
down_read() in the UFS queuecommand function in this patch?

Thanks,

Bart.