From: Takashi Iwai <tiwai@suse.de> To: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Cc: Ming Lei <ming.lei@canonical.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, linux-usb@vger.kernel.org, Oliver Neukum <oliver@neukum.org>, Alan Stern <stern@rowland.harvard.edu>, linux-input@vger.kernel.org, linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org, linux-wireless@vger.kernel.org, linux-media@vger.kernel.org, alsa-devel@alsa-project.org, Jaroslav Kysela <perex@perex.cz> Subject: Re: [PATCH 45/50] sound: usb: usx2y: spin_lock in complete() cleanup Date: Thu, 11 Jul 2013 15:50:57 +0200 [thread overview] Message-ID: <s5hip0hb3y6.wl%tiwai@suse.de> (raw) In-Reply-To: <51DEAE4E.90204@cogentembedded.com> At Thu, 11 Jul 2013 17:08:30 +0400, Sergei Shtylyov wrote: > > On 11-07-2013 13:06, Ming Lei wrote: > > > Complete() will be run with interrupt enabled, so change to > > spin_lock_irqsave(). > > Changelog doesn't match the patch. Yep, but moreover... > > Cc: Jaroslav Kysela <perex@perex.cz> > > Cc: Takashi Iwai <tiwai@suse.de> > > Cc: alsa-devel@alsa-project.org > > Signed-off-by: Ming Lei <ming.lei@canonical.com> > > --- > > sound/usb/usx2y/usbusx2yaudio.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > 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(). I'll prepare the patch for 3.11 independently from your patch series, so please drop this one. BTW, the word "cleanup" in the subject is inappropriate. This is rather a fix together with the core change. And, I wonder whether we can take a safer approach. When the caller condition changed, we often introduced either a different ops (e.g. ioctl case) or a flag for the new condition, at least during the transition period. Last but not least, is a conversion to tasklet really preferred? tasklet is rather an obsoleted infrastructure nowadays, and people don't recommend to use it any longer, AFAIK. thanks, Takashi
WARNING: multiple messages have this Message-ID (diff)
From: Takashi Iwai <tiwai-l3A5Bk7waGM@public.gmane.org> To: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org> Cc: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>, Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>, linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>, Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>, linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org, Jaroslav Kysela <perex-/Fr2/VpizcU@public.gmane.org> Subject: Re: [PATCH 45/50] sound: usb: usx2y: spin_lock in complete() cleanup Date: Thu, 11 Jul 2013 15:50:57 +0200 [thread overview] Message-ID: <s5hip0hb3y6.wl%tiwai@suse.de> (raw) In-Reply-To: <51DEAE4E.90204-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org> At Thu, 11 Jul 2013 17:08:30 +0400, Sergei Shtylyov wrote: > > On 11-07-2013 13:06, Ming Lei wrote: > > > Complete() will be run with interrupt enabled, so change to > > spin_lock_irqsave(). > > Changelog doesn't match the patch. Yep, but moreover... > > Cc: Jaroslav Kysela <perex-/Fr2/VpizcU@public.gmane.org> > > Cc: Takashi Iwai <tiwai-l3A5Bk7waGM@public.gmane.org> > > Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org > > Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> > > --- > > sound/usb/usx2y/usbusx2yaudio.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > 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(). I'll prepare the patch for 3.11 independently from your patch series, so please drop this one. BTW, the word "cleanup" in the subject is inappropriate. This is rather a fix together with the core change. And, I wonder whether we can take a safer approach. When the caller condition changed, we often introduced either a different ops (e.g. ioctl case) or a flag for the new condition, at least during the transition period. Last but not least, is a conversion to tasklet really preferred? tasklet is rather an obsoleted infrastructure nowadays, and people don't recommend to use it any longer, AFAIK. thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2013-07-11 13:49 UTC|newest] Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-07-11 9:05 [PATCH 00/50] USB: cleanup spin_lock in URB->complete() Ming Lei 2013-07-11 9:05 ` [PATCH 01/50] USB: devio: spin_lock in complete() cleanup Ming Lei 2013-07-11 9:05 ` [PATCH 02/50] USB: cdc-wdm: " Ming Lei 2013-07-11 9:05 ` Ming Lei 2013-07-11 9:05 ` [PATCH 03/50] USB: usblp: " Ming Lei 2013-07-11 9:05 ` [PATCH 04/50] USB: adutux: " Ming Lei 2013-07-11 9:05 ` [PATCH 05/50] USB: misc: uss720: " Ming Lei 2013-07-11 12:15 ` Sergei Shtylyov 2013-07-11 12:15 ` Sergei Shtylyov 2013-07-11 9:05 ` [PATCH 06/50] USB: iowarrior: " Ming Lei 2013-07-11 9:05 ` [PATCH 07/50] USB: ldusb: " Ming Lei 2013-07-11 9:05 ` Ming Lei 2013-07-11 9:05 ` [PATCH 08/50] USB: legousbtower: " Ming Lei 2013-07-11 12:18 ` Sergei Shtylyov 2013-07-11 12:18 ` Sergei Shtylyov 2013-07-11 12:36 ` Oliver Neukum 2013-07-11 13:47 ` Sergei Shtylyov 2013-07-11 12:42 ` Ming Lei 2013-07-11 12:42 ` Ming Lei 2013-07-11 9:05 ` [PATCH 09/50] USB: usbtest: " Ming Lei 2013-07-11 9:05 ` [PATCH 10/50] USB: serial: cyberjack: " Ming Lei 2013-07-11 9:05 ` [PATCH 11/50] USB: serial: digi_acceleportldusb: " Ming Lei 2013-07-11 9:05 ` [PATCH 12/50] USB: serial: io_edgeport: " Ming Lei 2013-07-11 9:05 ` [PATCH 13/50] USB: serial: io_ti: " Ming Lei 2013-07-11 9:05 ` [PATCH 14/50] USB: serial: mos7720: " Ming Lei 2013-07-11 9:05 ` Ming Lei 2013-07-11 9:05 ` [PATCH 15/50] USB: serial: mos77840: " Ming Lei 2013-07-11 9:05 ` [PATCH 16/50] USB: serial: quatech2: " Ming Lei 2013-07-11 9:05 ` [PATCH 17/50] USB: serial: sierra: " Ming Lei 2013-07-11 13:02 ` Sergei Shtylyov 2013-07-11 9:05 ` [PATCH 18/50] USB: serial: symbolserial: " Ming Lei 2013-07-11 9:05 ` [PATCH 19/50] USB: serial: ti_usb_3410_5052: " Ming Lei 2013-07-11 9:05 ` [PATCH 20/50] USB: serial: usb_wwan: " Ming Lei 2013-07-11 9:05 ` Ming Lei 2013-07-11 9:05 ` [PATCH 21/50] hid: usbhid: " Ming Lei 2013-07-11 9:05 ` [PATCH 22/50] BT: btusb: " Ming Lei 2013-07-11 9:05 ` [PATCH 23/50] BT: bfusb: read_lock " Ming Lei 2013-07-11 9:05 ` [PATCH 24/50] input: cm109: spin_lock " Ming Lei 2013-07-11 9:05 ` [PATCH 25/50] ISDN: hfcsusb: " Ming Lei 2013-07-11 9:05 ` [PATCH 26/50] USBNET: cdc-phonet: " Ming Lei 2013-07-11 9:05 ` Ming Lei 2013-07-11 9:05 ` [PATCH 27/50] USBNET: hso: " Ming Lei 2013-07-11 9:05 ` Ming Lei 2013-07-11 9:05 ` [PATCH 28/50] USBNET: kaweth: " Ming Lei 2013-07-11 9:05 ` Ming Lei 2013-07-11 9:05 ` [PATCH 29/50] USBNET: rtl8150: " Ming Lei 2013-07-11 9:05 ` Ming Lei 2013-07-11 9:05 ` [PATCH 30/50] wireless: ath9k: " Ming Lei 2013-07-11 9:05 ` [PATCH 31/50] wireless: zd1211rw: " Ming Lei 2013-07-11 9:05 ` [PATCH 32/50] wireless: ath: carl9170: " Ming Lei 2013-07-11 9:05 ` [PATCH 33/50] wireless: libertas: " Ming Lei 2013-07-11 9:05 ` [PATCH 34/50] wireless: libertas_tf: " Ming Lei 2013-07-11 9:05 ` [PATCH 35/50] media: usb: cx231xx: " Ming Lei 2013-07-26 14:28 ` Hans Verkuil 2013-07-27 9:34 ` Ming Lei 2013-07-11 9:05 ` [PATCH 36/50] media: usb: em28xx: " Ming Lei 2013-07-11 12:42 ` Devin Heitmueller 2013-07-11 9:06 ` [PATCH 37/50] media: usb: sn9x102: " Ming Lei 2013-07-11 9:06 ` [PATCH 38/50] media: usb: tlg2300: " Ming Lei 2013-07-11 9:06 ` [PATCH 39/50] media: usb: tm6000: " Ming Lei 2013-07-11 9:06 ` [PATCH 40/50] media: dvb-core: " Ming Lei 2013-07-11 9:06 ` [PATCH 41/50] media: usb: em28xx: make sure irq disabled before acquiring lock Ming Lei 2013-07-11 9:06 ` [PATCH 42/50] media: usb: tlg2300: spin_lock in complete() cleanup Ming Lei 2013-07-11 13:13 ` Sergei Shtylyov 2013-07-11 9:06 ` [PATCH 43/50] sound: usb: midi: " Ming Lei 2013-07-11 9:06 ` [PATCH 44/50] sound: usb: caiaq: " Ming Lei 2013-07-11 13:12 ` Sergei Shtylyov 2013-07-11 14:06 ` Daniel Mack 2013-07-11 9:06 ` [PATCH 45/50] sound: usb: usx2y: " Ming Lei 2013-07-11 13:08 ` Sergei Shtylyov 2013-07-11 13:50 ` Takashi Iwai [this message] 2013-07-11 13:50 ` Takashi Iwai 2013-07-11 14:13 ` Ming Lei 2013-07-11 14:34 ` Takashi Iwai 2013-07-11 14:34 ` Takashi Iwai 2013-07-11 14:52 ` Ming Lei 2013-07-11 9:06 ` [PATCH 46/50] Sound: usb: ua101: " Ming Lei 2013-07-11 9:06 ` Ming Lei 2013-07-11 13:10 ` Sergei Shtylyov 2013-07-11 9:06 ` [PATCH 47/50] staging: btmtk_usb: " Ming Lei 2013-07-11 9:06 ` Ming Lei 2013-07-11 9:06 ` [PATCH 48/50] staging: bcm: " Ming Lei 2013-07-11 9:06 ` [PATCH 49/50] staging: ced1401: " Ming Lei 2013-07-11 9:06 ` [PATCH 50/50] staging: vt6656: " Ming Lei 2013-07-14 13:17 ` [PATCH 00/50] USB: cleanup spin_lock in URB->complete() Andy Walls 2013-07-14 14:33 ` Ming Lei
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=s5hip0hb3y6.wl%tiwai@suse.de \ --to=tiwai@suse.de \ --cc=alsa-devel@alsa-project.org \ --cc=gregkh@linuxfoundation.org \ --cc=linux-bluetooth@vger.kernel.org \ --cc=linux-input@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ --cc=linux-wireless@vger.kernel.org \ --cc=ming.lei@canonical.com \ --cc=netdev@vger.kernel.org \ --cc=oliver@neukum.org \ --cc=perex@perex.cz \ --cc=sergei.shtylyov@cogentembedded.com \ --cc=stern@rowland.harvard.edu \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.