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

Menon, Nishanth
2010-02-18T20:36:09Z

> -----Original Message-----
> From: Ramirez Luna, Omar
> Sent: Thursday, February 18, 2010 8:27 PM
> To: Menon, Nishanth
> Cc: Ameya Palande; Chitriki Rudramuni, Deepak; linux-omap; Guzman Lugo,
> Fernando
> Subject: Re: [PATCH v4] DSPBRIDGE: Fix to avoid possible recursive locking
>
> On 2/17/2010 10:09 AM, Menon, Nishanth wrote:
> > Hi,
> >
> >> From: Ameya Palande [mailto:ameya.palande@nokia.com]
> >> Sent: Friday, February 12, 2010 12:21 AM
> >> To: Chitriki Rudramuni, Deepak
> >> Cc: linux-omap; Ramirez Luna, Omar; Menon, Nishanth
> >> Subject: Re: [PATCH v4] DSPBRIDGE: Fix to avoid possible recursive
> locking
> >>
> >> On Thu, 2010-02-11 at 22:54 +0100, ext Deepak Chitriki wrote:
> >>> Removed NTFY_Notify() in WMD_MSG_Get() to avoid locking contention
> >>> as NTFY_Notify() is already invoked in InputMsg().
> >>>
> >>> 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>
> >>
> >> Acked-by: Ameya Palande<ameya.palande@nokia.com>
> >
> > Could this patch be pushed if there are no more comments?
>
> Instead of pushing this patch I'll be reverting this one:
>
> http://marc.info/?l=linux-omap&m=125694410627720&w=2
>
> Explanation:
>
> After reviewing the scenario it seems that the patch "fixing a flood of
> messages usecase" doesn't seem to be the proper way to fix it.
>
> If the dsp sends 10 messages in a shot for the same client and bridge
> process them without interrupt, if the client is using
> DSPManager_WaitForEvents, InputMsg function will notify of all of them
> but "WaitForEvents" will only catch one notification, then client will
> call GetMsg ioctl and pick one message from the queue leaving 9 of them
> still there, whenever WaitForEvents is executed again there won't be a
> Notification if the dsp stopped sending messages to the same queue, so
> those 9 messages never will be retrieved because WaitForEvents will
> timeout.
>
> The fix was only to re-notify the client that a message was still
> available in its queue and that it needs to pick it up, all of this
> during GetMsg, however this seems to be triggering a kernel warning
> because of nested spinlocks called for separate objects.
>
> The reason to avoid pushing this patch is that it is leaving a part of
> the code introduced by the "flooding" fix.
>
> Please let me know if anyone has comments.

So, do we have a clean complete fix? Why not leave the partial fix which we have in place and incorporate the clean fix on top of that? I would prefer to have something here instead of nothing at all.. but that is just me.

>
> - omar

Regards,
Nishanth Menon