On 9/29/2023 11:25 PM, Jeffrey Hugo wrote:
On 9/24/2023 10:08 PM, Qiang Yu wrote:
On 9/22/2023 10:50 PM, Jeffrey Hugo wrote:
On 9/13/2023 2:47 AM, Qiang Yu wrote:
From: Hemant Kumar <quic_hemantk@xxxxxxxxxxx>
Take irqsave lock after TRE is generated to avoid deadlock due to core
getting interrupts enabled as local_bh_enable must not be called with
irqs disabled based on upstream patch.
Where is local_bh_enable() being called? What patch? What is
upstream of the codebase you submitted this to? Why is it safe to
call mhi_gen_tre() without the lock?
This patch is to fix the issue included by "[PATCH v2 1/2] bus: mhi:
host: Add spinlock to protect WP access when queueing TREs". In that
patch, we add write_lock_bh/write_unlock_bh in mhi_gen_tre().
However, before mhi_gen_tre() is invoked, mhi_cntrl->pm_lock is
getted, line 1125, and it is a spin lock. So it becomes we want to
get and release bh lock after spin lock. __local_bh_enable_ip is
called as part of write_unlock_bh
and local_bh_enable. When CONFIG_TRACE_IRQFLAGS is enabled, irq will
be enabled once __local_bh_enable_ip is called. The commit message is
not clear and confusing, will change it in [patch v3].
In addition to clarifying the commit message, I recommend looking at
adding this to the other patch. It seems very odd to review a series
where one patch introduces a known issue, and a following patch
corrects that issue. It would be better to not introduce the issue in
the first place.
OK, will squash two patches into one patch after we achieve an agreement.
