[PATCH 0/3] Reduce interrupt latency

[PATCH 0/3] Reduce interrupt latency

[Bart Van Assche]

Hi Martin,

On Android systems it is important to keep the time spent in interrupts short.
This keeps the user interface responsive and prevents audio stuttering. Hence
this patch series to reduce the time spent in the UFS interrupt handler. Please
consider this patch series for the next merge window.

Thanks,

Bart.

Bart Van Assche (3):
  ufs: core: Fix ufshcd_mcq_force_compl_one()
  ufs: core: Introduce ufshcd_mcq_poll_cqe_lock_n()
  ufs: qcom: Reduce interrupt latency

 drivers/ufs/core/ufs-mcq.c     | 32 +++++++++++---------------------
 drivers/ufs/core/ufshcd-priv.h |  2 --
 drivers/ufs/core/ufshcd.c      |  2 +-
 drivers/ufs/host/ufs-qcom.c    | 33 +++++++++++++++++++++++++++++----
 include/ufs/ufshcd.h           |  3 +++
 5 files changed, 44 insertions(+), 28 deletions(-)

[PATCH 1/3] ufs: core: Fix ufshcd_mcq_force_compl_one()

[Bart Van Assche]

There are two issues with the code in ufshcd_mcq_compl_all_cqes_lock():
- The completion queue is processed without checking first whether it is
  empty.
- It is attempted to process hwq->max_entries while there can be at most
  hwq->max_entries - 1 entries on a completion queue.

Fix this by replacing the only ufshcd_mcq_compl_all_cqes_lock() call with
a call to ufshcd_mcq_poll_cqe_lock().

Fixes: ab248643d3d6 ("scsi: ufs: core: Add error handling for MCQ mode")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufs-mcq.c     | 18 ------------------
 drivers/ufs/core/ufshcd-priv.h |  2 --
 drivers/ufs/core/ufshcd.c      |  2 +-
 3 files changed, 1 insertion(+), 21 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 1b3062577945..8ccde4571859 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -322,24 +322,6 @@ static void ufshcd_mcq_process_cqe(struct ufs_hba *hba,
 	}
 }
 
-void ufshcd_mcq_compl_all_cqes_lock(struct ufs_hba *hba,
-				    struct ufs_hw_queue *hwq)
-{
-	unsigned long flags;
-	u32 entries = hwq->max_entries;
-
-	spin_lock_irqsave(&hwq->cq_lock, flags);
-	while (entries > 0) {
-		ufshcd_mcq_process_cqe(hba, hwq);
-		ufshcd_mcq_inc_cq_head_slot(hwq);
-		entries--;
-	}
-
-	ufshcd_mcq_update_cq_tail_slot(hwq);
-	hwq->cq_head_slot = hwq->cq_tail_slot;
-	spin_unlock_irqrestore(&hwq->cq_lock, flags);
-}
-
 unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
 				       struct ufs_hw_queue *hwq)
 {
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index 0a72148cb053..96fcd935790b 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -71,8 +71,6 @@ int ufshcd_get_hba_mac(struct ufs_hba *hba);
 int ufshcd_mcq_memory_alloc(struct ufs_hba *hba);
 struct ufs_hw_queue *ufshcd_mcq_req_to_hwq(struct ufs_hba *hba,
 					   struct request *req);
-void ufshcd_mcq_compl_all_cqes_lock(struct ufs_hba *hba,
-				    struct ufs_hw_queue *hwq);
 bool ufshcd_cmd_inflight(struct scsi_cmnd *cmd);
 int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag);
 int ufshcd_mcq_abort(struct scsi_cmnd *cmd);
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index cb81aa94d5c4..28b8e7291394 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -5855,7 +5855,7 @@ static bool ufshcd_mcq_force_compl_one(struct request *rq, void *priv)
 	if (blk_mq_is_reserved_rq(rq) || !hwq)
 		return true;
 
-	ufshcd_mcq_compl_all_cqes_lock(hba, hwq);
+	ufshcd_mcq_poll_cqe_lock(hba, hwq);
 
 	/*
 	 * For those cmds of which the cqes are not present in the cq, complete

[PATCH 2/3] ufs: core: Introduce ufshcd_mcq_poll_cqe_lock_n()

[Bart Van Assche]

Introduce a new function for processing completions and that accepts an
upper limit for the number of completions to poll. Tell
ufshcd_mcq_poll_cqe_lock() to poll at most hwq->max_entries. This is
sufficient to poll all pending completions since there are never more
than hwq->max_entries - 1 completions on a completion queue. This patch
prepares for reducing the interrupt latency.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufs-mcq.c | 14 +++++++++++---
 include/ufs/ufshcd.h       |  3 +++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 8ccde4571859..afff0784555f 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -322,15 +322,16 @@ static void ufshcd_mcq_process_cqe(struct ufs_hba *hba,
 	}
 }
 
-unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
-				       struct ufs_hw_queue *hwq)
+unsigned long ufshcd_mcq_poll_cqe_lock_n(struct ufs_hba *hba,
+					 struct ufs_hw_queue *hwq,
+					 unsigned int max_compl)
 {
 	unsigned long completed_reqs = 0;
 	unsigned long flags;
 
 	spin_lock_irqsave(&hwq->cq_lock, flags);
 	ufshcd_mcq_update_cq_tail_slot(hwq);
-	while (!ufshcd_mcq_is_cq_empty(hwq)) {
+	while (!ufshcd_mcq_is_cq_empty(hwq) && completed_reqs < max_compl) {
 		ufshcd_mcq_process_cqe(hba, hwq);
 		ufshcd_mcq_inc_cq_head_slot(hwq);
 		completed_reqs++;
@@ -342,6 +343,13 @@ unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
 
 	return completed_reqs;
 }
+EXPORT_SYMBOL_GPL(ufshcd_mcq_poll_cqe_lock_n);
+
+unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
+				       struct ufs_hw_queue *hwq)
+{
+	return ufshcd_mcq_poll_cqe_lock_n(hba, hwq, hwq->max_entries);
+}
 EXPORT_SYMBOL_GPL(ufshcd_mcq_poll_cqe_lock);
 
 void ufshcd_mcq_make_queues_operational(struct ufs_hba *hba)
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index cfbc75d8df83..5679d93353ac 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1475,6 +1475,9 @@ void ufshcd_mcq_config_mac(struct ufs_hba *hba, u32 max_active_cmds);
 unsigned int ufshcd_mcq_queue_cfg_addr(struct ufs_hba *hba);
 u32 ufshcd_mcq_read_cqis(struct ufs_hba *hba, int i);
 void ufshcd_mcq_write_cqis(struct ufs_hba *hba, u32 val, int i);
+unsigned long ufshcd_mcq_poll_cqe_lock_n(struct ufs_hba *hba,
+					 struct ufs_hw_queue *hwq,
+					 unsigned int max_compl);
 unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
 					 struct ufs_hw_queue *hwq);
 void ufshcd_mcq_make_queues_operational(struct ufs_hba *hba);

[PATCH 3/3] ufs: qcom: Reduce interrupt latency

[Bart Van Assche]

Defer completion processing to thread context on slower CPU cores to
prevent interrupt latency spikes. On the fastest CPU cores, keep
processing all completions in interrupt context.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/host/ufs-qcom.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 5a58ffef3d27..7cacc0ec0624 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -2370,6 +2370,16 @@ struct ufs_qcom_irq {
 	struct ufs_hba		*hba;
 };
 
+static irqreturn_t ufs_qcom_mcq_threaded_esi_handler(int irq, void *data)
+{
+	struct ufs_qcom_irq *qi = data;
+	struct ufs_hba *hba = qi->hba;
+
+	ufshcd_mcq_poll_cqe_lock(hba, &hba->uhq[qi->idx]);
+
+	return IRQ_HANDLED;
+}
+
 static irqreturn_t ufs_qcom_mcq_esi_handler(int irq, void *data)
 {
 	struct ufs_qcom_irq *qi = data;
@@ -2377,9 +2387,22 @@ static irqreturn_t ufs_qcom_mcq_esi_handler(int irq, void *data)
 	struct ufs_hw_queue *hwq = &hba->uhq[qi->idx];
 
 	ufshcd_mcq_write_cqis(hba, 0x1, qi->idx);
-	ufshcd_mcq_poll_cqe_lock(hba, hwq);
 
-	return IRQ_HANDLED;
+	if (arch_scale_cpu_capacity(raw_smp_processor_id()) ==
+	    SCHED_CAPACITY_SCALE) {
+		ufshcd_mcq_poll_cqe_lock(hba, hwq);
+		return IRQ_HANDLED;
+	}
+
+	if (ufshcd_mcq_poll_cqe_lock_n(hba, hwq, 4) < 4)
+		return IRQ_HANDLED;
+
+	/*
+	 * Defer further completion processing to thread context because
+	 * processing a large number of completions in interrupt context on
+	 * slower CPU cores can result in unacceptably high interrupt latencies.
+	 */
+	return IRQ_WAKE_THREAD;
 }
 
 static int ufs_qcom_config_esi(struct ufs_hba *hba)
@@ -2415,8 +2438,10 @@ static int ufs_qcom_config_esi(struct ufs_hba *hba)
 		qi[idx].idx = idx;
 		qi[idx].hba = hba;
 
-		ret = devm_request_irq(hba->dev, qi[idx].irq, ufs_qcom_mcq_esi_handler,
-				       IRQF_SHARED, "qcom-mcq-esi", qi + idx);
+		ret = devm_request_threaded_irq(hba->dev, qi[idx].irq,
+			ufs_qcom_mcq_esi_handler,
+			ufs_qcom_mcq_threaded_esi_handler,
+			IRQF_SHARED | IRQF_ONESHOT, "qcom-mcq-esi", qi + idx);
 		if (ret) {
 			dev_err(hba->dev, "%s: Failed to request IRQ for %d, err = %d\n",
 				__func__, qi[idx].irq, ret);

Re: [PATCH 0/3] Reduce interrupt latency

[Bart Van Assche]

On 3/30/26 11:33 AM, Bart Van Assche wrote:
> On Android systems it is important to keep the time spent in interrupts short.
> This keeps the user interface responsive and prevents audio stuttering. Hence
> this patch series to reduce the time spent in the UFS interrupt handler. Please
> consider this patch series for the next merge window.
(replying to my own email)

Hi,

Can anyone help me to test this patch series on an MCQ Qualcomm system? 
I do not have access to such a setup.

Thanks,

Bart.

RE: [PATCH 0/3] Reduce interrupt latency

[Can Guo]

Hi Bart,

I am adding Xiaosen to help on your request. Please expect some turnaround time.

Thanks.
Best Regards,
Can Guo

-----Original Message-----
From: Bart Van Assche <bvanassche@acm.org> 
Sent: Tuesday, March 31, 2026 2:36 AM
To: Can Guo (QUIC) <quic_cang@quicinc.com>
Cc: linux-scsi@vger.kernel.org; Martin K . Petersen <martin.petersen@oracle.com>
Subject: Re: [PATCH 0/3] Reduce interrupt latency

WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.

On 3/30/26 11:33 AM, Bart Van Assche wrote:
> On Android systems it is important to keep the time spent in interrupts short.
> This keeps the user interface responsive and prevents audio 
> stuttering. Hence this patch series to reduce the time spent in the 
> UFS interrupt handler. Please consider this patch series for the next merge window.
(replying to my own email)

Hi,

Can anyone help me to test this patch series on an MCQ Qualcomm system?
I do not have access to such a setup.

Thanks,

Bart.

Re: [PATCH 3/3] ufs: qcom: Reduce interrupt latency

[Manivannan Sadhasivam]

+ Nitin

On Mon, Mar 30, 2026 at 11:33:05AM -0700, Bart Van Assche wrote:
> Defer completion processing to thread context on slower CPU cores to
> prevent interrupt latency spikes. On the fastest CPU cores, keep
> processing all completions in interrupt context.
> 

By default, all interrupts are pinned to CPU0. So unless some userspace entity
like irqbalance changes the CPU affinity, all the interrupts will be serviced
in the threaded context on CPU0, which will negatively impact performance with
this patch.

I think from the kernel driver, we should just set the IRQ affinity hint as
Nitin tried [1] and let the userspace to balance IRQ load based on the activity.

- Mani

[1] https://lore.kernel.org/all/20260122141331.239354-2-nitin.rawat@oss.qualcomm.com

> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/ufs/host/ufs-qcom.c | 33 +++++++++++++++++++++++++++++----
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 5a58ffef3d27..7cacc0ec0624 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -2370,6 +2370,16 @@ struct ufs_qcom_irq {
>  	struct ufs_hba		*hba;
>  };
>  
> +static irqreturn_t ufs_qcom_mcq_threaded_esi_handler(int irq, void *data)
> +{
> +	struct ufs_qcom_irq *qi = data;
> +	struct ufs_hba *hba = qi->hba;
> +
> +	ufshcd_mcq_poll_cqe_lock(hba, &hba->uhq[qi->idx]);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static irqreturn_t ufs_qcom_mcq_esi_handler(int irq, void *data)
>  {
>  	struct ufs_qcom_irq *qi = data;
> @@ -2377,9 +2387,22 @@ static irqreturn_t ufs_qcom_mcq_esi_handler(int irq, void *data)
>  	struct ufs_hw_queue *hwq = &hba->uhq[qi->idx];
>  
>  	ufshcd_mcq_write_cqis(hba, 0x1, qi->idx);
> -	ufshcd_mcq_poll_cqe_lock(hba, hwq);
>  
> -	return IRQ_HANDLED;
> +	if (arch_scale_cpu_capacity(raw_smp_processor_id()) ==
> +	    SCHED_CAPACITY_SCALE) {
> +		ufshcd_mcq_poll_cqe_lock(hba, hwq);
> +		return IRQ_HANDLED;
> +	}
> +
> +	if (ufshcd_mcq_poll_cqe_lock_n(hba, hwq, 4) < 4)
> +		return IRQ_HANDLED;
> +
> +	/*
> +	 * Defer further completion processing to thread context because
> +	 * processing a large number of completions in interrupt context on
> +	 * slower CPU cores can result in unacceptably high interrupt latencies.
> +	 */
> +	return IRQ_WAKE_THREAD;
>  }
>  
>  static int ufs_qcom_config_esi(struct ufs_hba *hba)
> @@ -2415,8 +2438,10 @@ static int ufs_qcom_config_esi(struct ufs_hba *hba)
>  		qi[idx].idx = idx;
>  		qi[idx].hba = hba;
>  
> -		ret = devm_request_irq(hba->dev, qi[idx].irq, ufs_qcom_mcq_esi_handler,
> -				       IRQF_SHARED, "qcom-mcq-esi", qi + idx);
> +		ret = devm_request_threaded_irq(hba->dev, qi[idx].irq,
> +			ufs_qcom_mcq_esi_handler,
> +			ufs_qcom_mcq_threaded_esi_handler,
> +			IRQF_SHARED | IRQF_ONESHOT, "qcom-mcq-esi", qi + idx);
>  		if (ret) {
>  			dev_err(hba->dev, "%s: Failed to request IRQ for %d, err = %d\n",
>  				__func__, qi[idx].irq, ret);

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 1/3] ufs: core: Fix ufshcd_mcq_force_compl_one()

[Peter Wang (王信友)]

On Mon, 2026-03-30 at 11:33 -0700, Bart Van Assche wrote:
> There are two issues with the code in
> ufshcd_mcq_compl_all_cqes_lock():
> - The completion queue is processed without checking first whether it
> is
>   empty.
> 

Hi Bart,

Does ufshcd_mcq_compl_all_cqes_lock not check whether the CQ
is empty because there could be cases where the CQ is empty
but there are still ongoing requests?


> 
> @@ -5855,7 +5855,7 @@ static bool ufshcd_mcq_force_compl_one(struct
> request *rq, void *priv)
>         if (blk_mq_is_reserved_rq(rq) || !hwq)
>                 return true;
> 
> -       ufshcd_mcq_compl_all_cqes_lock(hba, hwq);
> +       ufshcd_mcq_poll_cqe_lock(hba, hwq);
> 
> 

Since ufshcd_mcq_force_compl_one and ufshcd_mcq_compl_one
are very similar, would it be possible to merge them into 
one function, with a parameter to handle the force completion
case?

Thanks.
Peter

Re: [PATCH 2/3] ufs: core: Introduce ufshcd_mcq_poll_cqe_lock_n()

[Peter Wang (王信友)]

On Mon, 2026-03-30 at 11:33 -0700, Bart Van Assche wrote:
> Introduce a new function for processing completions and that accepts
> an
> upper limit for the number of completions to poll. Tell
> ufshcd_mcq_poll_cqe_lock() to poll at most hwq->max_entries. This is
> sufficient to poll all pending completions since there are never more
> than hwq->max_entries - 1 completions on a completion queue. This
> patch
> prepares for reducing the interrupt latency.

Hi Bart,

Could it be more than (hwq->max_entries - 1) if the host keeps
sending requests to the same hardware queue from different CPU cores?

Thanks.
Peter

Re: [PATCH 1/3] ufs: core: Fix ufshcd_mcq_force_compl_one()

[Bart Van Assche]

On 3/31/26 2:41 AM, Peter Wang (王信友) wrote:
> Does ufshcd_mcq_compl_all_cqes_lock not check whether the CQ
> is empty because there could be cases where the CQ is empty
> but there are still ongoing requests?

Hi Peter,

ufshcd_mcq_compl_all_cqes_lock() doesn't read the CQ head pointer.
Processing completion queue elements without reading the CQ head pointer
first is not safe.

> Since ufshcd_mcq_force_compl_one and ufshcd_mcq_compl_one
> are very similar, would it be possible to merge them into
> one function, with a parameter to handle the force completion
> case?

I think addressing that question falls outside the scope of this patch
series.

Thanks,

Bart.

Re: [PATCH 2/3] ufs: core: Introduce ufshcd_mcq_poll_cqe_lock_n()

[Bart Van Assche]

On 3/31/26 2:43 AM, Peter Wang (王信友) wrote:
> On Mon, 2026-03-30 at 11:33 -0700, Bart Van Assche wrote:
>> Introduce a new function for processing completions and that accepts
>> an
>> upper limit for the number of completions to poll. Tell
>> ufshcd_mcq_poll_cqe_lock() to poll at most hwq->max_entries. This is
>> sufficient to poll all pending completions since there are never more
>> than hwq->max_entries - 1 completions on a completion queue. This
>> patch
>> prepares for reducing the interrupt latency.
> 
> Could it be more than (hwq->max_entries - 1) if the host keeps
> sending requests to the same hardware queue from different CPU cores?

No, because the completion queue head is read before processing
completion queue entries starts. No matter how many completions are
pushed onto the completion queue while ufshcd_mcq_poll_cqe_lock() is
in progress, it won't process more than (hwq->max_entries - 1) because
that's the maximum difference between the tail and the head pointers.

Thanks,

Bart.

Re: [PATCH 3/3] ufs: qcom: Reduce interrupt latency

[Bart Van Assche]

On 3/31/26 12:09 AM, Manivannan Sadhasivam wrote:
> + Nitin
> 
> On Mon, Mar 30, 2026 at 11:33:05AM -0700, Bart Van Assche wrote:
>> Defer completion processing to thread context on slower CPU cores to
>> prevent interrupt latency spikes. On the fastest CPU cores, keep
>> processing all completions in interrupt context.
> 
> By default, all interrupts are pinned to CPU0. So unless some userspace entity
> like irqbalance changes the CPU affinity, all the interrupts will be serviced
> in the threaded context on CPU0

That's an unusual approach. All other blk-mq drivers I know of spread 
completion interrupt over CPU cores. Pinning all completion interrupts
to a single CPU core is risky because it may cause that CPU core to
spend all of its time handling interrupts with no time left for running
kernel or user-space threads.

 > which will negatively impact performance with this patch.

Hmm ... the implementation of this patch is such that latency is not 
affected for queue depth 1. It will have a latency impact for queue
depths above 4 but I don't think that just by reading the code it can
be concluded whether or not IOPS will be affected. This patch could have
an impact similar to enabling interrupt coalescing. Interrupt coalescing
increases latency but typically also increases IOPS.
> I think from the kernel driver, we should just set the IRQ affinity hint as
> Nitin tried [1] and let the userspace to balance IRQ load based on the activity.
> 
> - Mani
> 
> [1] https://lore.kernel.org/all/20260122141331.239354-2-nitin.rawat@oss.qualcomm.com
  Thanks for the link. Please consider calling 
devm_platform_get_irqs_affinity() or one of its variants instead of
open-coding already existing code for spreading interrupts across CPU 
cores. As you may know there are two types of interrupts in the Linux
kernel: non-managed and managed. For non-managed interrupts, a default
CPU affinity is assigned when the interrupt is requested and the CPU
affinity can be modified from user space. If all CPUs in the affinity
mask of a non-managed interrupt go offline, the hotplug code changes the
affinity mask to the remaining online CPUs.

Managed interrupts are spread evenly across CPUs by the code that
requests these interrupts. User space code cannot modify the affinity
mask of managed interrupts. If the last CPU in the affinity mask of a
managed interrupt goes offline then the interrupt is shut down. If the
first CPU in the affinity mask becomes online again then the interrupt
is started up again.

I think the latter behavior is what is needed for UFS host controllers.

Thanks,

Bart.

Re: [PATCH 1/3] ufs: core: Fix ufshcd_mcq_force_compl_one()

[Peter Wang (王信友)]

On Tue, 2026-03-31 at 12:29 -0700, Bart Van Assche wrote:
> Hi Peter,
> 
> ufshcd_mcq_compl_all_cqes_lock() doesn't read the CQ head pointer.
> Processing completion queue elements without reading the CQ head
> pointer
> first is not safe.
> 


Hi Bart,

Yes, but I'm curious, will processing all entries, 
although inefficient, really cause any problems?


> > Since ufshcd_mcq_force_compl_one and ufshcd_mcq_compl_one
> > are very similar, would it be possible to merge them into
> > one function, with a parameter to handle the force completion
> > case?
> 
> I think addressing that question falls outside the scope of this
> patch
> series.
> 

Yes, it falls outside the scope. I just noticed that when this
patch is applied, these two functions become similar.

Thanks.
Peter

Re: [PATCH 2/3] ufs: core: Introduce ufshcd_mcq_poll_cqe_lock_n()

[Peter Wang (王信友)]

On Tue, 2026-03-31 at 12:31 -0700, Bart Van Assche wrote:
> No, because the completion queue head is read before processing
> completion queue entries starts. No matter how many completions are
> pushed onto the completion queue while ufshcd_mcq_poll_cqe_lock() is
> in progress, it won't process more than (hwq->max_entries - 1)
> because
> that's the maximum difference between the tail and the head pointers.
> 
> Thanks,
> 
> Bart.

Hi Bart,

Yes, I got it. But I think you mean that "tail" is read
before processing, right?

Thanks
Peter

Re: [PATCH 1/3] ufs: core: Fix ufshcd_mcq_force_compl_one()

[Bart Van Assche]

On 4/1/26 6:10 AM, Peter Wang (王信友) wrote:
> Yes, but I'm curious, will processing all entries,
> although inefficient, really cause any problems?

I will drop this patch because I realized that it is too risky.
ufshcd_mcq_compl_all_cqes_lock() is called after the host controller has
been disabled. ufshcd_mcq_poll_cqe_lock() reads the CQ tail pointer 
while ufshcd_mcq_compl_all_cqes_lock() does not read the CQ tail 
pointer. I'm not sure that it is safe to read the CQ tail pointer while
the host controller is disabled.

Processing all CQ entries in ufshcd_mcq_compl_all_cqes_lock() should be
safe because ufshcd_mcq_process_cqe() marks a CQE as invalid after it
has been processed.

Thanks,

Bart.

Re: [PATCH 2/3] ufs: core: Introduce ufshcd_mcq_poll_cqe_lock_n()

[Bart Van Assche]

On 4/1/26 6:11 AM, Peter Wang (王信友) wrote:
> Yes, I got it. But I think you mean that "tail" is read
> before processing, right?

Yes.

Re: [PATCH 1/3] ufs: core: Fix ufshcd_mcq_force_compl_one()

[Peter Wang (王信友)]

On Wed, 2026-04-01 at 08:51 -0700, Bart Van Assche wrote
> 
> I will drop this patch because I realized that it is too risky.
> ufshcd_mcq_compl_all_cqes_lock() is called after the host controller
> has
> been disabled. ufshcd_mcq_poll_cqe_lock() reads the CQ tail pointer
> while ufshcd_mcq_compl_all_cqes_lock() does not read the CQ tail
> pointer. I'm not sure that it is safe to read the CQ tail pointer
> while
> the host controller is disabled.
> 
> Processing all CQ entries in ufshcd_mcq_compl_all_cqes_lock() should
> be
> safe because ufshcd_mcq_process_cqe() marks a CQE as invalid after it
> has been processed.
> 
> Thanks,
> 
> Bart.

Hi Bart,

Yes. Normally, if the host is still working,
ufshcd_mcq_compl_all_cqes_lock
and ufshcd_mcq_poll_cqe_lock pretty much do the same thing, the only 
difference is that the original one isn’t as efficient.

Also, as long as the host is fine, most of the time we just use 
the method below to force it to complete when this happens.

	/*
	 * For those cmds of which the cqes are not present in the cq,
complete
	 * them explicitly.
	 */

Thanks
Peter

Re: [PATCH 2/3] ufs: core: Introduce ufshcd_mcq_poll_cqe_lock_n()

[Peter Wang (王信友)]

On Mon, 2026-03-30 at 11:33 -0700, Bart Van Assche wrote:
> Introduce a new function for processing completions and that accepts
> an
> upper limit for the number of completions to poll. Tell
> ufshcd_mcq_poll_cqe_lock() to poll at most hwq->max_entries. This is
> sufficient to poll all pending completions since there are never more
> than hwq->max_entries - 1 completions on a completion queue. This
> patch
> prepares for reducing the interrupt latency.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---

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

Re: [PATCH 3/3] ufs: qcom: Reduce interrupt latency

[Manivannan Sadhasivam]

On Tue, Mar 31, 2026 at 12:44:52PM -0700, Bart Van Assche wrote:
> On 3/31/26 12:09 AM, Manivannan Sadhasivam wrote:
> > + Nitin
> > 
> > On Mon, Mar 30, 2026 at 11:33:05AM -0700, Bart Van Assche wrote:
> > > Defer completion processing to thread context on slower CPU cores to
> > > prevent interrupt latency spikes. On the fastest CPU cores, keep
> > > processing all completions in interrupt context.
> > 
> > By default, all interrupts are pinned to CPU0. So unless some userspace entity
> > like irqbalance changes the CPU affinity, all the interrupts will be serviced
> > in the threaded context on CPU0
> 
> That's an unusual approach. All other blk-mq drivers I know of spread
> completion interrupt over CPU cores. Pinning all completion interrupts
> to a single CPU core is risky because it may cause that CPU core to
> spend all of its time handling interrupts with no time left for running
> kernel or user-space threads.
> 

What I mentioned above is the default platform behavior where the interrupt
controller assigns default affinity of all interrupts to CPU0 unless device
drivers/userspace ask explicitly to spread the affinity.

> > which will negatively impact performance with this patch.
> 
> Hmm ... the implementation of this patch is such that latency is not
> affected for queue depth 1. It will have a latency impact for queue
> depths above 4 but I don't think that just by reading the code it can
> be concluded whether or not IOPS will be affected. This patch could have
> an impact similar to enabling interrupt coalescing. Interrupt coalescing
> increases latency but typically also increases IOPS.

If I understand this patch correctly, irrespective of the queue depth, if the
default affinity is CPU0, then this patch will always cause the interrupts to be
processed by the threaded handler since CPU0 is not the fastest CPU core in the
hierarchy.

> > I think from the kernel driver, we should just set the IRQ affinity hint as
> > Nitin tried [1] and let the userspace to balance IRQ load based on the activity.
> > 
> > - Mani
> > 
> > [1] https://lore.kernel.org/all/20260122141331.239354-2-nitin.rawat@oss.qualcomm.com
>  Thanks for the link. Please consider calling
> devm_platform_get_irqs_affinity() or one of its variants instead of
> open-coding already existing code for spreading interrupts across CPU cores.
> As you may know there are two types of interrupts in the Linux
> kernel: non-managed and managed. For non-managed interrupts, a default
> CPU affinity is assigned when the interrupt is requested and the CPU
> affinity can be modified from user space. If all CPUs in the affinity
> mask of a non-managed interrupt go offline, the hotplug code changes the
> affinity mask to the remaining online CPUs.
> 
> Managed interrupts are spread evenly across CPUs by the code that
> requests these interrupts. User space code cannot modify the affinity
> mask of managed interrupts. If the last CPU in the affinity mask of a
> managed interrupt goes offline then the interrupt is shut down. If the
> first CPU in the affinity mask becomes online again then the interrupt
> is started up again.
> 
> I think the latter behavior is what is needed for UFS host controllers.
> 

I'm not too familiar with Android internals, but isn't that Android has some
irqbalance or similar utilities that manage the UFS interrupts based on system
load? Managed interrupts work better if we want the interrupts to be managed
by the kernel, without user intervention. Not sure if that's what we really want
for UFS.

I certainly cannot comment on that.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 3/3] ufs: qcom: Reduce interrupt latency

[Bart Van Assche]

On 4/3/26 8:11 PM, Manivannan Sadhasivam wrote:
> I'm not too familiar with Android internals, but isn't that Android has some
> irqbalance or similar utilities that manage the UFS interrupts based on system
> load?

It seems to me that irqbalance or any similar software is not compatible
with the Android goal of saving energy. An important goal in Android on
systems with heterogeneous CPU cores is to wake up the larger CPU cores
as infrequently as possible since larger CPU cores use more power.
irqbalance might assign interrupts to a larger CPU core even if a little
core can handle the workload.

 > Managed interrupts work better if we want the interrupts to be 
managed> by the kernel, without user intervention. Not sure if that's 
what we really want
> for UFS.

All blk-mq drivers I'm familiar with use managed interrupts. Why should
the UFS driver use a different approach? When using managed interrupts,
if the number of completion queues is larger than or equal to the number
of CPU cores and if rq_affinity=2, the completion interrupt will be sent
to the CPU core that submitted the I/O. With this approach the number of
cache misses triggered by the I/O completion code is minimized. Data
that is needed by the completion handler, e.g. struct scsi_cmnd, is
most likely still present in the cache of the CPU core that handles the
completion when the completion is handled.

Thanks,

Bart.

Re: RE: [PATCH 0/3] Reduce interrupt latency

[Xiaosen]

Hi Bart,

This patch series work fine on Qualcomm MCQ devices.

Tested-and-reviewed-by: Xiaosen He <xiaosen.he@oss.qualcomm.com>

On 3/31/2026 10:01 AM, Can Guo wrote:
> Hi Bart,
>
> I am adding Xiaosen to help on your request. Please expect some turnaround time.
>
> Thanks.
> Best Regards,
> Can Guo
>
> -----Original Message-----
> From: Bart Van Assche <bvanassche@acm.org>
> Sent: Tuesday, March 31, 2026 2:36 AM
> To: Can Guo (QUIC) <quic_cang@quicinc.com>
> Cc: linux-scsi@vger.kernel.org; Martin K . Petersen <martin.petersen@oracle.com>
> Subject: Re: [PATCH 0/3] Reduce interrupt latency
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>
> On 3/30/26 11:33 AM, Bart Van Assche wrote:
>> On Android systems it is important to keep the time spent in interrupts short.
>> This keeps the user interface responsive and prevents audio
>> stuttering. Hence this patch series to reduce the time spent in the
>> UFS interrupt handler. Please consider this patch series for the next merge window.
> (replying to my own email)
>
> Hi,
>
> Can anyone help me to test this patch series on an MCQ Qualcomm system?
> I do not have access to such a setup.
>
> Thanks,
>
> Bart.