Re: [PATCH v2 2/2] bus: mhi: host: Take irqsave lock after TRE is generated

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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.



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux