| Message ID | 1624054985-31365-1-git-send-email-bbhatt@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | bus: mhi: core: Disable pre-emption for data events tasklet processing | expand |
On Fri, Jun 18, 2021 at 03:23:05PM -0700, Bhaumik Bhatt wrote: > With spin_lock_bh(), it is possible that a tasklet processing data > events gets scheduled out if another higher priority tasklet is > ready to run. While the tasklet is sleeping, it can hold the event > ring spinlock and block another tasklet, for example, one processing > an M0 state change event, from acquiring it. This can starve the > core which blocks in an attempt to acquire the spinlock until it > gets released. Fix this issue by disabling pre-emption on the core > processing data events and allow it to run to completion such that > other tasklets do not block for long periods. > Are you sure? IIUC, the tasklets priority is only used while trying to schedule the next pending tasklet. But I don't think a high priority tasklet can preempt the low priority one. Please correct me if I'm wrong. Thanks, Mani > Fixes: 1d3173a3bae7 ("bus: mhi: core: Add support for processing events from client device") > Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org> > --- > drivers/bus/mhi/core/main.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c > index 3775c77..02c8c09 100644 > --- a/drivers/bus/mhi/core/main.c > +++ b/drivers/bus/mhi/core/main.c > @@ -1036,11 +1036,12 @@ void mhi_ev_task(unsigned long data) > { > struct mhi_event *mhi_event = (struct mhi_event *)data; > struct mhi_controller *mhi_cntrl = mhi_event->mhi_cntrl; > + unsigned long flags; > > /* 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) > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >
On Sat, 2021-06-19 at 13:14 +0530, Manivannan Sadhasivam wrote: > On Fri, Jun 18, 2021 at 03:23:05PM -0700, Bhaumik Bhatt wrote: > > > > With spin_lock_bh(), it is possible that a tasklet processing data > > events gets scheduled out if another higher priority tasklet is Needs some re-wording. What we observed was threded irq storms and in the past we observed some RT tasks preempting MHI tasklet for long time. > > ready to run. While the tasklet is sleeping, it can hold the event Not allowed to sleep in tasklet context. > > ring spinlock and block another tasklet, for example, one > > processing > > an M0 state change event, from acquiring it. This can starve the > > core which blocks in an attempt to acquire the spinlock until it > > gets released. Fix this issue by disabling pre-emption on the core > > processing data events and allow it to run to completion such that > > other tasklets do not block for long periods. > > > Are you sure? > > IIUC, the tasklets priority is only used while trying to schedule the > next pending tasklet. But I don't think a high priority tasklet can > preempt the low priority one. > > Please correct me if I'm wrong. > > Thanks, > Mani > [..]
diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c index 3775c77..02c8c09 100644 --- a/drivers/bus/mhi/core/main.c +++ b/drivers/bus/mhi/core/main.c @@ -1036,11 +1036,12 @@ void mhi_ev_task(unsigned long data) { struct mhi_event *mhi_event = (struct mhi_event *)data; struct mhi_controller *mhi_cntrl = mhi_event->mhi_cntrl; + unsigned long flags; /* 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)
With spin_lock_bh(), it is possible that a tasklet processing data events gets scheduled out if another higher priority tasklet is ready to run. While the tasklet is sleeping, it can hold the event ring spinlock and block another tasklet, for example, one processing an M0 state change event, from acquiring it. This can starve the core which blocks in an attempt to acquire the spinlock until it gets released. Fix this issue by disabling pre-emption on the core processing data events and allow it to run to completion such that other tasklets do not block for long periods. Fixes: 1d3173a3bae7 ("bus: mhi: core: Add support for processing events from client device") Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org> --- drivers/bus/mhi/core/main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)