Re: [PATCH] DSPBRIDGE: Fix to avoid possible recursive locking

2010-02-10 Thread Ameya Palande
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.pala...@nokia.com
 Cc: Omar Ramirez Luna omar.rami...@ti.com
 Cc: Nishanth Menon n...@ti.com
 
 Signed-off-by: Deepak Chitriki deepak.chitr...@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.

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] DSPBRIDGE: Fix to avoid possible recursive locking

2010-02-10 Thread Chitriki Rudramuni, Deepak
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.pala...@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.pala...@nokia.com
 Cc: Omar Ramirez Luna omar.rami...@ti.com
 Cc: Nishanth Menon n...@ti.com

 Signed-off-by: Deepak Chitriki deepak.chitr...@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.--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RESEND] [PATCH] DSPBRIDGE: Fix to avoid possible recursive locking

2010-02-10 Thread Chitriki Rudramuni, Deepak
Please discard this patch.The rework patch will be available in v1.

Thanks,
Deepak

From: linux-omap-ow...@vger.kernel.org [linux-omap-ow...@vger.kernel.org] On 
Behalf Of Chitriki Rudramuni, Deepak
Sent: Thursday, February 11, 2010 4:44 AM
To: Ameya Palande
Cc: linux-omap; Ramirez Luna, Omar; Menon, Nishanth
Subject: RE: [PATCH] DSPBRIDGE: Fix to avoid possible recursive locking

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.pala...@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.pala...@nokia.com
 Cc: Omar Ramirez Luna omar.rami...@ti.com
 Cc: Nishanth Menon n...@ti.com

 Signed-off-by: Deepak Chitriki deepak.chitr...@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.--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] DSPBRIDGE: Fix to avoid possible recursive locking

2010-02-09 Thread Deepak Chitriki
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.pala...@nokia.com
Cc: Omar Ramirez Luna omar.rami...@ti.com
Cc: Nishanth Menon n...@ti.com

Signed-off-by: Deepak Chitriki deepak.chitr...@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

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html