Thread (71 messages) 71 messages, 8 authors, 2013-07-27

Re: [PATCH 45/50] sound: usb: usx2y: spin_lock in complete() cleanup

From: Ming Lei <hidden>
Date: 2013-07-11 14:53:02
Also in: alsa-devel, linux-media, linux-wireless, netdev

On Thu, Jul 11, 2013 at 10:34 PM, Takashi Iwai [off-list ref] wrote:
At Thu, 11 Jul 2013 22:13:35 +0800,
Ming Lei wrote:
quoted
On Thu, Jul 11, 2013 at 9:50 PM, Takashi Iwai [off-list ref] wrote:
quoted
At Thu, 11 Jul 2013 17:08:30 +0400,
Sergei Shtylyov wrote:
quoted
On 11-07-2013 13:06, Ming Lei wrote:
quoted
Complete() will be run with interrupt enabled, so change to
spin_lock_irqsave().
    Changelog doesn't match the patch.
Yep, but moreover...
quoted
quoted
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <redacted>
Cc: alsa-devel@alsa-project.org
Signed-off-by: Ming Lei <redacted>
---
  sound/usb/usx2y/usbusx2yaudio.c |    4 ++++
  1 file changed, 4 insertions(+)
quoted
diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c
index 4967fe9..e2ee893 100644
--- a/sound/usb/usx2y/usbusx2yaudio.c
+++ b/sound/usb/usx2y/usbusx2yaudio.c
@@ -273,7 +273,11 @@ static void usX2Y_clients_stop(struct usX2Ydev *usX2Y)
            struct snd_usX2Y_substream *subs = usX2Y->subs[s];
            if (subs) {
                    if (atomic_read(&subs->state) >= state_PRERUNNING) {
+                           unsigned long flags;
+
+                           local_irq_save(flags);
                            snd_pcm_stop(subs->pcm_substream, SNDRV_PCM_STATE_XRUN);
+                           local_irq_restore(flags);
                    }
... actually this snd_pcm_stop() call should have been covered by
snd_pcm_stream_lock().  Maybe it'd be enough to have a single patch
together with the change, i.e. wrapping with
snd_pcm_stream_lock_irqsave().
Please use snd_pcm_stream_lock_irqsave() so that I can avoid sending
out similar patch later, :-)
quoted
I'll prepare the patch for 3.11 independently from your patch series,
so please drop this one.
OK, thanks for dealing with that.
quoted

BTW, the word "cleanup" in the subject is inappropriate.  This is
rather a fix together with the core change.
It is a cleanup since the patchset only addresses lock problem which
is caused by the tasklet conversion.
Well, the conversion to irqsave() is needed for the future drop of
irq_save() in the caller, right?  Then this isn't a cleanup but a
preparation for movement ahead.
Sounds more accurate, and I will change the title in next round, :-)

Thanks,
--
Ming Lei
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help