public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
From: Omar Ramirez Luna <omar.ramirez@ti.com>
To: "Menon, Nishanth" <nm@ti.com>
Cc: Ameya Palande <ameya.palande@nokia.com>,
	"Chitriki Rudramuni, Deepak" <deepak.chitriki@ti.com>,
	linux-omap <linux-omap@vger.kernel.org>,
	"Guzman Lugo, Fernando" <x0095840@ti.com>
Subject: Re: [PATCH v4] DSPBRIDGE: Fix to avoid possible recursive locking
Date: Fri, 19 Feb 2010 14:02:08 -0600	[thread overview]
Message-ID: <4B7EEE40.3020003@ti.com> (raw)
In-Reply-To: <7A436F7769CA33409C6B44B358BFFF0C0130EBC3B9@dlee02.ent.ti.com>

On 2/18/2010 2:36 PM, Menon, Nishanth wrote:
>
[...]
>>
>> 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.
>

I wrongly pushed Deepak's fix when updating dspbridge, when I revert the 
old patch it will remove the other line too.

The partial fix is solving the kernel warning but is useless for the 
flooding of DSP messages, because the line left which is SYNC_SetEvent 
needs an actual event to be waiting, so right now it is not doing 
anything (in the original patch wasn't doing anything either), so before 
we forget that the line left there is useless I'm reverting the patch.

Regards,

Omar

      reply	other threads:[~2010-02-19 20:02 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-11 21:54 [PATCH v4] DSPBRIDGE: Fix to avoid possible recursive locking Deepak Chitriki
2010-02-11 22:20 ` Ameya Palande
2010-02-17 16:09   ` Menon, Nishanth
2010-02-18 18:26     ` Omar Ramirez Luna
2010-02-18 20:36       ` Menon, Nishanth
2010-02-19 20:02         ` Omar Ramirez Luna [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B7EEE40.3020003@ti.com \
    --to=omar.ramirez@ti.com \
    --cc=ameya.palande@nokia.com \
    --cc=deepak.chitriki@ti.com \
    --cc=linux-omap@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=x0095840@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox