Re: [PATCH] scsi: ufs: mark HPB support as BROKEN

[Ming Lei <ming.lei@redhat.com>]

On Wed, Oct 27, 2021 at 10:01:42AM -0700, Bart Van Assche wrote:
> On 10/27/21 9:16 AM, Martin K. Petersen wrote:
> > Given that HPB developed over time, I am not sure how simple a revert
> > would be. And we only have a couple of days left before release. I
> > really want the smallest patch possible that either removes or disables
> > the 2.0 support.
> 
> How about one of the untested patches below?
> 
> The patch below disables support for HPB 2.0 by ignoring the HPB version reported
> by the UFS controller:
> 
> diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c
> index 66b19500844e..5f9f7139480a 100644
> --- a/drivers/scsi/ufs/ufshpb.c
> +++ b/drivers/scsi/ufs/ufshpb.c
> @@ -2872,8 +2872,8 @@ void ufshpb_get_dev_info(struct ufs_hba *hba, u8 *desc_buf)
>  		return;
>  	}
> 
> -	if (version == HPB_SUPPORT_LEGACY_VERSION)
> -		hpb_dev_info->is_legacy = true;
> +	/* Do not use HPB 2.0 because of the blk_insert_cloned_request() call. */
> +	hpb_dev_info->is_legacy = true;

I guess you may change ufshpb_is_required_wb() to return false simply
with comment.

> 
>  	ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
>  		QUERY_ATTR_IDN_MAX_HPB_SINGLE_CMD, 0, 0, &max_hpb_single_cmd);
> 
> 
> The second patch changes the blk_insert_cloned_request() call into a
> blk_execute_rq_nowait() call. That should work fine since this function
> bypasses the I/O scheduler for passthrough requests:

Either ->is_legacy is set as true or ufshpb_is_required_wb() returns
false, blk_insert_cloned_request() won't be called. But here
blk_execute_rq_nowait() should be used since it is one driver private IO.

That also shows the private command of pre_req is run concurrently with the
original FS IO, and two tags are consumed for doing one IO. It could be done
one by one, but I guess it is a bit slower, just saw Daejun replied this point.


thanks,
Ming