mhi.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH] bus: mhi: host: Disable preemption while processing data events
@ 2022-11-03  7:55 Qiang Yu
  2022-11-07 11:22 ` Manivannan Sadhasivam
  2022-11-07 15:00 ` Jeffrey Hugo
  0 siblings, 2 replies; 4+ messages in thread
From: Qiang Yu @ 2022-11-03  7:55 UTC (permalink / raw)
  To: mani, loic.poulain
  Cc: mhi, linux-arm-msm, linux-kernel, quic_cang, mrana, Qiang Yu

If data processing of an event is scheduled out because core
is busy handling multiple irqs, this can starves the processing
of MHI M0 state change event on another core. Fix this issue by
disabling irq on the core processing data events.

Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
---
 drivers/bus/mhi/host/main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index f3aef77a..b58698d 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -1027,13 +1027,14 @@ int mhi_process_data_event_ring(struct mhi_controller *mhi_cntrl,
 
 void mhi_ev_task(unsigned long data)
 {
+	unsigned long flags;
 	struct mhi_event *mhi_event = (struct mhi_event *)data;
 	struct mhi_controller *mhi_cntrl = mhi_event->mhi_cntrl;
 
 	/* process all pending events */
-	spin_lock_bh(&mhi_event->lock);
+	spin_lock_irqsave(&mhi_event->lock, flags);
 	mhi_event->process_event(mhi_cntrl, mhi_event, U32_MAX);
-	spin_unlock_bh(&mhi_event->lock);
+	spin_unlock_irqrestore(&mhi_event->lock, flags);
 }
 
 void mhi_ctrl_ev_task(unsigned long data)
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] bus: mhi: host: Disable preemption while processing data events
  2022-11-03  7:55 [PATCH] bus: mhi: host: Disable preemption while processing data events Qiang Yu
@ 2022-11-07 11:22 ` Manivannan Sadhasivam
  2022-11-07 15:00 ` Jeffrey Hugo
  1 sibling, 0 replies; 4+ messages in thread
From: Manivannan Sadhasivam @ 2022-11-07 11:22 UTC (permalink / raw)
  To: Qiang Yu
  Cc: mani, loic.poulain, mhi, linux-arm-msm, linux-kernel, quic_cang, mrana

On Thu, Nov 03, 2022 at 03:55:11PM +0800, Qiang Yu wrote:
> If data processing of an event is scheduled out because core
> is busy handling multiple irqs, this can starves the processing

s/starves/starve

> of MHI M0 state change event on another core. Fix this issue by
> disabling irq on the core processing data events.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> ---
>  drivers/bus/mhi/host/main.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
> index f3aef77a..b58698d 100644
> --- a/drivers/bus/mhi/host/main.c
> +++ b/drivers/bus/mhi/host/main.c
> @@ -1027,13 +1027,14 @@ int mhi_process_data_event_ring(struct mhi_controller *mhi_cntrl,
>  
>  void mhi_ev_task(unsigned long data)
>  {
> +	unsigned long flags;
>  	struct mhi_event *mhi_event = (struct mhi_event *)data;
>  	struct mhi_controller *mhi_cntrl = mhi_event->mhi_cntrl;
>  
>  	/* process all pending events */
> -	spin_lock_bh(&mhi_event->lock);
> +	spin_lock_irqsave(&mhi_event->lock, flags);

Please add a comment stating why we are disabling local IRQs here. This will
help in understanding the code better.

Thanks,
Mani

>  	mhi_event->process_event(mhi_cntrl, mhi_event, U32_MAX);
> -	spin_unlock_bh(&mhi_event->lock);
> +	spin_unlock_irqrestore(&mhi_event->lock, flags);
>  }
>  
>  void mhi_ctrl_ev_task(unsigned long data)
> -- 
> 2.7.4
> 
> 

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] bus: mhi: host: Disable preemption while processing data events
  2022-11-03  7:55 [PATCH] bus: mhi: host: Disable preemption while processing data events Qiang Yu
  2022-11-07 11:22 ` Manivannan Sadhasivam
@ 2022-11-07 15:00 ` Jeffrey Hugo
  2022-11-08  6:54   ` Qiang Yu
  1 sibling, 1 reply; 4+ messages in thread
From: Jeffrey Hugo @ 2022-11-07 15:00 UTC (permalink / raw)
  To: Qiang Yu, mani, loic.poulain
  Cc: mhi, linux-arm-msm, linux-kernel, quic_cang, mrana

On 11/3/2022 1:55 AM, Qiang Yu wrote:
> If data processing of an event is scheduled out because core
> is busy handling multiple irqs, this can starves the processing
> of MHI M0 state change event on another core. Fix this issue by
> disabling irq on the core processing data events.

Can you elaborate on the scenario more?

If a device hasn't entered M0, then what event could be processed?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] bus: mhi: host: Disable preemption while processing data events
  2022-11-07 15:00 ` Jeffrey Hugo
@ 2022-11-08  6:54   ` Qiang Yu
  0 siblings, 0 replies; 4+ messages in thread
From: Qiang Yu @ 2022-11-08  6:54 UTC (permalink / raw)
  To: Jeffrey Hugo, mani, loic.poulain
  Cc: mhi, linux-arm-msm, linux-kernel, quic_cang, mrana


On 11/7/2022 11:00 PM, Jeffrey Hugo wrote:
> On 11/3/2022 1:55 AM, Qiang Yu wrote:
>> If data processing of an event is scheduled out because core
>> is busy handling multiple irqs, this can starves the processing
>> of MHI M0 state change event on another core. Fix this issue by
>> disabling irq on the core processing data events.
>
> Can you elaborate on the scenario more?
>
> If a device hasn't entered M0, then what event could be processed?
The scenario is following:
1. On Core 2 IPCR channel 20 transfer completion event MSI arrived which 
is processed by holding Event ring 1 lock by calling spin_lock_bh() in 
normal priority tasklet.
2. On Core 2 multiple irqs come in and scheduled out the event ring 
processing done by #1. Due to that event ring lock remained acquired but 
event ring processing for IPCR got scheduled out.
3. On Core 1, primary event ring MSI arrived for M0, which is handled by 
a separate MHI hi-priority tasklet. As part of M0 state transition,
tasklet goes over event ring 1 and tries to acquire event ring 1 lock in 
order to ring the event ring DB, but just gets stuck in busy wait due to 
#2.
At this point tasklet is at the mercy of the muiltiple irqs handle 
(which scheduled out the MHI tasklet of #1) to finish and let the MHI 
normal priority tasklet to run.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-11-08  6:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-03  7:55 [PATCH] bus: mhi: host: Disable preemption while processing data events Qiang Yu
2022-11-07 11:22 ` Manivannan Sadhasivam
2022-11-07 15:00 ` Jeffrey Hugo
2022-11-08  6:54   ` Qiang Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).