* [PATCH] DSPBRIDGE: Fix to avoid possible recursive locking
@ 2010-02-10 1:01 Deepak Chitriki
2010-02-10 13:27 ` Ameya Palande
0 siblings, 1 reply; 3+ messages in thread
From: Deepak Chitriki @ 2010-02-10 1:01 UTC (permalink / raw)
To: linux-omap
Cc: Deepak Chitriki, Ameya Palande, Omar Ramirez Luna, Nishanth Menon
This patch fixes possible recursive locking detection.The implementation
in which the spinlock is acquired and released is rectified in WMD_MSG_Get()
to avoid locking contention.
Added SYNC_EnterCS() and SYNC_LeaveCS()in WMD_MSG_Get()function.
Cc: Ameya Palande <ameya.palande@nokia.com>
Cc: Omar Ramirez Luna <omar.ramirez@ti.com>
Cc: Nishanth Menon <nm@ti.com>
Signed-off-by: Deepak Chitriki <deepak.chitriki@ti.com>
---
drivers/dsp/bridge/wmd/msg_sm.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/dsp/bridge/wmd/msg_sm.c b/drivers/dsp/bridge/wmd/msg_sm.c
index 50201e5..8faf5ad 100644
--- a/drivers/dsp/bridge/wmd/msg_sm.c
+++ b/drivers/dsp/bridge/wmd/msg_sm.c
@@ -300,8 +300,10 @@ DSP_STATUS WMD_MSG_Get(struct MSG_QUEUE *hMsgQueue,
if (LST_IsEmpty(hMsgQueue->msgUsedList))
SYNC_ResetEvent(hMsgQueue->hSyncEvent);
else {
+ (void)SYNC_LeaveCS(hMsgMgr->hSyncCS);
NTFY_Notify(hMsgQueue->hNtfy,
DSP_NODEMESSAGEREADY);
+ (void)SYNC_EnterCS(hMsgMgr->hSyncCS);
SYNC_SetEvent(hMsgQueue->hSyncEvent);
}
@@ -352,8 +354,10 @@ DSP_STATUS WMD_MSG_Get(struct MSG_QUEUE *hMsgQueue,
hMsgQueue->refCount--;
/* Reset the event if there are still queued messages */
if (!LST_IsEmpty(hMsgQueue->msgUsedList)) {
+ (void)SYNC_LeaveCS(hMsgMgr->hSyncCS);
NTFY_Notify(hMsgQueue->hNtfy,
DSP_NODEMESSAGEREADY);
+ (void)SYNC_EnterCS(hMsgMgr->hSyncCS);
SYNC_SetEvent(hMsgQueue->hSyncEvent);
}
/* Exit critical section */
--
1.6.3.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] DSPBRIDGE: Fix to avoid possible recursive locking
2010-02-10 1:01 [PATCH] DSPBRIDGE: Fix to avoid possible recursive locking Deepak Chitriki
@ 2010-02-10 13:27 ` Ameya Palande
2010-02-10 23:14 ` Chitriki Rudramuni, Deepak
0 siblings, 1 reply; 3+ messages in thread
From: Ameya Palande @ 2010-02-10 13:27 UTC (permalink / raw)
To: ext Deepak Chitriki; +Cc: linux-omap, Omar Ramirez Luna, Nishanth Menon
Hi Deepak,
On Wed, 2010-02-10 at 02:01 +0100, ext Deepak Chitriki wrote:
> This patch fixes possible recursive locking detection.The implementation
> in which the spinlock is acquired and released is rectified in WMD_MSG_Get()
> to avoid locking contention.
> Added SYNC_EnterCS() and SYNC_LeaveCS()in WMD_MSG_Get()function.
>
> Cc: Ameya Palande <ameya.palande@nokia.com>
> Cc: Omar Ramirez Luna <omar.ramirez@ti.com>
> Cc: Nishanth Menon <nm@ti.com>
>
> Signed-off-by: Deepak Chitriki <deepak.chitriki@ti.com>
> ---
> drivers/dsp/bridge/wmd/msg_sm.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/dsp/bridge/wmd/msg_sm.c b/drivers/dsp/bridge/wmd/msg_sm.c
> index 50201e5..8faf5ad 100644
> --- a/drivers/dsp/bridge/wmd/msg_sm.c
> +++ b/drivers/dsp/bridge/wmd/msg_sm.c
> @@ -300,8 +300,10 @@ DSP_STATUS WMD_MSG_Get(struct MSG_QUEUE *hMsgQueue,
> if (LST_IsEmpty(hMsgQueue->msgUsedList))
> SYNC_ResetEvent(hMsgQueue->hSyncEvent);
> else {
> + (void)SYNC_LeaveCS(hMsgMgr->hSyncCS);
> NTFY_Notify(hMsgQueue->hNtfy,
> DSP_NODEMESSAGEREADY);
> + (void)SYNC_EnterCS(hMsgMgr->hSyncCS);
> SYNC_SetEvent(hMsgQueue->hSyncEvent);
> }
>
> @@ -352,8 +354,10 @@ DSP_STATUS WMD_MSG_Get(struct MSG_QUEUE *hMsgQueue,
> hMsgQueue->refCount--;
> /* Reset the event if there are still queued messages */
> if (!LST_IsEmpty(hMsgQueue->msgUsedList)) {
> + (void)SYNC_LeaveCS(hMsgMgr->hSyncCS);
> NTFY_Notify(hMsgQueue->hNtfy,
> DSP_NODEMESSAGEREADY);
> + (void)SYNC_EnterCS(hMsgMgr->hSyncCS);
> SYNC_SetEvent(hMsgQueue->hSyncEvent);
> }
> /* Exit critical section */
Can you explain the need of calling NTFY_Notify() in WMD_MSG_Get()?
I can see that the InputMsg calls NTFY_Notify() already!
Can we get rid of NTFY_Notify() from WMD_MSG_Get() all together?
Cheers,
Ameya.
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH] DSPBRIDGE: Fix to avoid possible recursive locking
2010-02-10 13:27 ` Ameya Palande
@ 2010-02-10 23:14 ` Chitriki Rudramuni, Deepak
0 siblings, 0 replies; 3+ messages in thread
From: Chitriki Rudramuni, Deepak @ 2010-02-10 23:14 UTC (permalink / raw)
To: Ameya Palande; +Cc: linux-omap, Ramirez Luna, Omar, Menon, Nishanth
Hi Ameya,
Yes I agree with your comments.
As I understand NTFY_Notify() is called again inside WMD_MSG_Get() to make sure that notification is done in case if message queue is not empty.Since notification is already done once in InputMsg() while copying message to message queue,it doesn't make sense to notify again in
WMD_MSG_Get().I guess this is not needed.I did a quick sanity testing removing NTFY_Notify() from WMD_MSG_Get() and no issues.
I will rework the patch and update.
Thanks,
Deepak
______________________________________
From: Ameya Palande [ameya.palande@nokia.com]
Sent: Wednesday, February 10, 2010 6:57 PM
To: Chitriki Rudramuni, Deepak
Cc: linux-omap; Ramirez Luna, Omar; Menon, Nishanth
Subject: Re: [PATCH] DSPBRIDGE: Fix to avoid possible recursive locking
Hi Deepak,
On Wed, 2010-02-10 at 02:01 +0100, ext Deepak Chitriki wrote:
> This patch fixes possible recursive locking detection.The implementation
> in which the spinlock is acquired and released is rectified in WMD_MSG_Get()
> to avoid locking contention.
> Added SYNC_EnterCS() and SYNC_LeaveCS()in WMD_MSG_Get()function.
>
> Cc: Ameya Palande <ameya.palande@nokia.com>
> Cc: Omar Ramirez Luna <omar.ramirez@ti.com>
> Cc: Nishanth Menon <nm@ti.com>
>
> Signed-off-by: Deepak Chitriki <deepak.chitriki@ti.com>
> ---
> drivers/dsp/bridge/wmd/msg_sm.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/dsp/bridge/wmd/msg_sm.c b/drivers/dsp/bridge/wmd/msg_sm.c
> index 50201e5..8faf5ad 100644
> --- a/drivers/dsp/bridge/wmd/msg_sm.c
> +++ b/drivers/dsp/bridge/wmd/msg_sm.c
> @@ -300,8 +300,10 @@ DSP_STATUS WMD_MSG_Get(struct MSG_QUEUE *hMsgQueue,
> if (LST_IsEmpty(hMsgQueue->msgUsedList))
> SYNC_ResetEvent(hMsgQueue->hSyncEvent);
> else {
> + (void)SYNC_LeaveCS(hMsgMgr->hSyncCS);
> NTFY_Notify(hMsgQueue->hNtfy,
> DSP_NODEMESSAGEREADY);
> + (void)SYNC_EnterCS(hMsgMgr->hSyncCS);
> SYNC_SetEvent(hMsgQueue->hSyncEvent);
> }
>
> @@ -352,8 +354,10 @@ DSP_STATUS WMD_MSG_Get(struct MSG_QUEUE *hMsgQueue,
> hMsgQueue->refCount--;
> /* Reset the event if there are still queued messages */
> if (!LST_IsEmpty(hMsgQueue->msgUsedList)) {
> + (void)SYNC_LeaveCS(hMsgMgr->hSyncCS);
> NTFY_Notify(hMsgQueue->hNtfy,
> DSP_NODEMESSAGEREADY);
> + (void)SYNC_EnterCS(hMsgMgr->hSyncCS);
> SYNC_SetEvent(hMsgQueue->hSyncEvent);
> }
> /* Exit critical section */
Can you explain the need of calling NTFY_Notify() in WMD_MSG_Get()?
I can see that the InputMsg calls NTFY_Notify() already!
Can we get rid of NTFY_Notify() from WMD_MSG_Get() all together?
Cheers,
Ameya.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-02-10 23:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-02-10 1:01 [PATCH] DSPBRIDGE: Fix to avoid possible recursive locking Deepak Chitriki
2010-02-10 13:27 ` Ameya Palande
2010-02-10 23:14 ` Chitriki Rudramuni, Deepak
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.