Re: [PATCH v9 17/17] scsi: ufs: Inform the block layer about write ordering

Bao D. Nguyen
2023-08-17T21:48:49Z

On 8/17/2023 12:34 PM, Bart Van Assche wrote:
> On 8/17/23 12:00, Bao D. Nguyen wrote:
>> In legacy SDB mode with auto-hibernation enabled, the default setting
>> for the
>> driver_preserves_write_order = false.
> >
>> Using the default setting, it may be missing this check that is part
>> of the
>> ufshcd_auto_hibern8_update()->ufshcd_update_preserves_write_order().
>
> If auto-hibernation is enabled by the host driver,
> driver_preserves_write_order
> is set by the following code in ufshcd_slave_configure():
>
>     q->limits.driver_preserves_write_order =
>         !ufshcd_is_auto_hibern8_supported(hba) ||
>         FIELD_GET(UFSHCI_AHIBERN8_TIMER_MASK, hba->ahit) == 0;
>
> Does this answer your question?
Hi Bart,
My concern is that in the ufshcd_update_preserves_write_order() you have
this logic:

if (!preserves_write_order) {
shost_for_each_device(sdev, hba->host) {
struct request_queue *q = sdev->request_queue;
/*...*/
if (blk_queue_is_zoned(q) && !q->elevator)
return -EPERM;
}
}

The above logic is only invoked when ufshcd_auto_hibern8_update() is
called. During initialization, ufshcd_auto_hibern8_update() is not
called. Therefore, you may have SDB mode with auto hibernate enabled ->
preserves_write_order = false, and (blk_queue_is_zoned(q) &&
!q->elevator) is true. Is that a problem? If you called
ufshcd_auto_hibern8_update() during ufs probe, you would have returned
-EPERM and not enable auto-hibernation in that case.

Thanks,
Bao

>
> Thanks,
>
> Bart.