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

[Bart Van Assche <bvanassche@acm.org>]

On 8/17/23 17:19, Bao D. Nguyen wrote:
> For example, in SDB mode, after the probe and you want to enable auto-hibern8, you would call ufshcd_auto_hibern8_update() which then calls
> ufshcd_update_preserves_write_order(). Before auto-hibern8 is enabled, you would check this condition:
>      if (blk_queue_is_zoned(q) && !q->elevator)
> 
> In other words, auto-hibern8 is enabled only if the above condition false.
> 
> However, the during a normal operation, the ufshcd_auto_hibern8_update() may not get called at all, and auto-hibern8 can be enabled in SDB mode as part of ufs init. Would that be a problem to have auto-hibern8 enabled without checking whether the above condition is false?

Hi Bao,

Probing the host controller happens as follows:
* The host controller probing function is called, e.g. ufs_qcom_probe().
* ufshcd_alloc_host() and ufshcd_init() are called directly or indirectly
   by the host controller probe function.
* ufshcd_init() calls scsi_add_host() and async_schedule(ufshcd_async_scan, hba).

Once ufshcd_async_scan() is called asynchronously, it performs the
following actions:
* ufs_probe_hba() is called. This function calls ufshcd_link_startup()
   indirectly. That function invokes the link_startup_notify vop if it has
   been defined. Some host drivers set hba->ahit from inside that vop.
* scsi_scan_host() is called and calls scsi_alloc_sdev() indirectly.
* scsi_alloc_sdev() calls blk_mq_init_queue() and shost->hostt->slave_alloc().
* scsi_add_lun() calls sdev->host->hostt->slave_configure().
* scsi_add_lun() calls scsi_sysfs_add_sdev(). This indirectly causes sd_probe()
   to be called asynchronously.
* sd_probe() calls sd_revalidate_disk(). This results in an indirect call of
   disk_set_zoned(). Additionally, the ELEVATOR_F_ZBD_SEQ_WRITE flag is set by
   the sd driver if q->limits.driver_preserves_write_order has not been set.
* Still from inside sd_probe(), device_add_disk() is called and a scheduler
   is selected based on the value of q->required_elevator_features.

There are two ways in which host drivers initialize auto-hibernation:
* Either by setting hba->ahit before ufshcd_init() is called.
* Or by calling ufshcd_auto_hibern8_update() from the link_startup_notify
   vop.

I think the above shows that the zoned model is queried *after* the above methods
for enabling auto-hibernation have completed and hence that blk_queue_is_zoned()
evaluates to false if a host driver calls ufshcd_auto_hibern8_update() from
inside the link_startup_notify vop.

If auto-hibernation is enabled from inside the host driver then that should
happen before sd_probe() is called. sd_probe() will use the value of
q->limits.driver_preserves_write_order that has been set by
ufshcd_slave_configure() so there is no risk for inconsistencies between the
auto-hibernation configuration and the request queue configuration.

Bart.