[PATCH] ufs: core: Improve IOPS

[PATCH] ufs: core: Improve IOPS

[Bart Van Assche]

Measurements have shown that IOPS improve by 2% - 3% on my UFS 4 test
setup every time a ktime_get() call is removed from the UFS driver
command processing path. Hence this patch that modifies
ufshcd_clk_scaling_start_busy() such that ktime_get() is only called
if the returned value will be used.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index b20f262fc8e4..9579e2481062 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2231,11 +2231,13 @@ static void ufshcd_exit_clk_gating(struct ufs_hba *hba)
 static void ufshcd_clk_scaling_start_busy(struct ufs_hba *hba)
 {
 	bool queue_resume_work = false;
-	ktime_t curr_t = ktime_get();
+	ktime_t curr_t;
 
 	if (!ufshcd_is_clkscaling_supported(hba))
 		return;
 
+	curr_t = ktime_get();
+
 	guard(spinlock_irqsave)(&hba->clk_scaling.lock);
 
 	if (!hba->clk_scaling.active_reqs++)

Re: [PATCH] ufs: core: Improve IOPS

[Peter Wang (王信友)]

On Wed, 2025-08-13 at 10:10 -0700, Bart Van Assche wrote:
> Measurements have shown that IOPS improve by 2% - 3% on my UFS 4 test
> setup every time a ktime_get() call is removed from the UFS driver
> command processing path. Hence this patch that modifies
> ufshcd_clk_scaling_start_busy() such that ktime_get() is only called
> if the returned value will be used.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/ufs/core/ufshcd.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index b20f262fc8e4..9579e2481062 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -2231,11 +2231,13 @@ static void ufshcd_exit_clk_gating(struct
> ufs_hba *hba)
>  static void ufshcd_clk_scaling_start_busy(struct ufs_hba *hba)
>  {
>         bool queue_resume_work = false;
> -       ktime_t curr_t = ktime_get();
> +       ktime_t curr_t;
> 
>         if (!ufshcd_is_clkscaling_supported(hba))
>                 return;
> 
> +       curr_t = ktime_get();
> +
>         guard(spinlock_irqsave)(&hba->clk_scaling.lock);
> 
>         if (!hba->clk_scaling.active_reqs++)

Hi Bart,

Although this may not cause any errors, it is clearly a 
coding defect that impacts performance.
Have you considered adding a "Fixes" and "Cc" tag to 
address this issue?

Thanks.
Peter

Re: [PATCH] ufs: core: Improve IOPS

[Bean Huo]

On Wed, 2025-08-13 at 10:10 -0700, Bart Van Assche wrote:
> Measurements have shown that IOPS improve by 2% - 3% on my UFS 4 test
> setup every time a ktime_get() call is removed from the UFS driver
> command processing path. Hence this patch that modifies
> ufshcd_clk_scaling_start_busy() such that ktime_get() is only called
> if the returned value will be used.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/ufs/core/ufshcd.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index b20f262fc8e4..9579e2481062 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -2231,11 +2231,13 @@ static void ufshcd_exit_clk_gating(struct ufs_hba *hba)
>  static void ufshcd_clk_scaling_start_busy(struct ufs_hba *hba)
>  {
>         bool queue_resume_work = false;
> -       ktime_t curr_t = ktime_get();
> +       ktime_t curr_t;
>  
>         if (!ufshcd_is_clkscaling_supported(hba))
>                 return;
>  
> +       curr_t = ktime_get();
> +
>         guard(spinlock_irqsave)(&hba->clk_scaling.lock);
>  
>         if (!hba->clk_scaling.active_reqs++)
> 

Nice!

Reviewed-by: Bean Huo <beanhuo@micron.com>

Re: [PATCH] ufs: core: Improve IOPS

[Bart Van Assche]

On 8/14/25 1:28 AM, Peter Wang (王信友) wrote:
> Although this may not cause any errors, it is clearly a
> coding defect that impacts performance.
> Have you considered adding a "Fixes" and "Cc" tag to
> address this issue?

Hi Peter,

My understanding is that the "Fixes" tag is reserved for bug fixes and
also that it should not be used for performance improvements unless if
the patch fixes a performance regression. I think this patch is a
performance improvement and not a performance regression fix. Hence, the
"Fixes" tag should not be used. However, if someone wants to submit this
patch for inclusion in an Android kernel, I will be happy to support
that effort.

Bart.

Re: [PATCH] ufs: core: Improve IOPS

[Peter Wang (王信友)]

On Thu, 2025-08-14 at 08:48 -0700, Bart Van Assche wrote:
> On 8/14/25 1:28 AM, Peter Wang (王信友) wrote:
> > Although this may not cause any errors, it is clearly a
> > coding defect that impacts performance.
> > Have you considered adding a "Fixes" and "Cc" tag to
> > address this issue?
> 
> Hi Peter,
> 
> My understanding is that the "Fixes" tag is reserved for bug fixes
> and
> also that it should not be used for performance improvements unless
> if
> the patch fixes a performance regression. I think this patch is a
> performance improvement and not a performance regression fix. Hence,
> the
> "Fixes" tag should not be used. However, if someone wants to submit
> this
> patch for inclusion in an Android kernel, I will be happy to support
> that effort.
> 
> Bart.

Got it, thanks.

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH] ufs: core: Improve IOPS

[Martin K. Petersen]

Bart,

> Measurements have shown that IOPS improve by 2% - 3% on my UFS 4 test
> setup every time a ktime_get() call is removed from the UFS driver
> command processing path. Hence this patch that modifies
> ufshcd_clk_scaling_start_busy() such that ktime_get() is only called
> if the returned value will be used.

Applied to 6.18/scsi-staging, thanks!

-- 
Martin K. Petersen