RE: [PATCH v3] scsi: ufs: Add batched WB buffer flush

[Avri Altman]

 Hi,

> Currently, WriteBooster (WB) buffer is always flushed during hibern8.
> However,
> this is inefficient because data in the WB buffer can be invalid due to
> spatial locality of IO workload.
> If the WB buffer flush is flushed in a batched manner, the amount of data
> migration and power consumption can be reduced because the overwritten
> data
> of the WB buffer may be invalid due to spatial locality.
> 
> This patch supports batched flush of WB buffer. When batched flush is
> enabled,
> fWriteBoosterBufferFlushDuringHibernate is set only when
> b_rpm_dev_flush_capable is true during runtime suspend. When the device
> is
> resumed, fWriteBoosterBufferFlushDuringHibernate is cleared to stop flush
> during hibern8.
I guess the idea that stands in the basis of your proposal is - "set the flag only when the device signals it's needed".
However,
a) Only a small fraction of WB flush during hibern8 time is generated via runtime suspend.  and
b) Today, the wb buffer fullness, is only tested in runtime suspend.

So you might end up with significantly reducing the % of time the flag is set, Not even knowing that it is needed.
This might cause a performance degradation.

Moreover, for platforms that runtime suspend is not configured, the flag is not set at all.
This is also the case when the platform is connected to USB.

I think that we need to find a way, to more promptly allow the device to signal it requires wb flush time.
How about calling ufshcd_wb_need_flush more often, e.g. on urgent bkops,
or better yet respond to a WRITEBOOSTER_EVENT_EN?


> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -643,6 +643,7 @@ struct ufs_hba_variant_params {
>         struct devfreq_simple_ondemand_data ondemand_data;
>         u16 hba_enable_delay_us;
>         u32 wb_flush_threshold;
> +       bool wb_batched_flush;
>  };
While at it, you need to set this vop for a platform, otherwise it is just a dead code.

Thanks,
Avri