From: Josh Boyer <jwboyer@redhat.com>
To: Daniel Mack <zonque@gmail.com>
Cc: Takashi Iwai <tiwai@suse.de>, Bruno Wolff III <bruno@wolff.to>,
Jaroslav Kysela <perex@perex.cz>,
alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org
Subject: Re: Logitech USB headset not working in 3.6-rc3
Date: Wed, 29 Aug 2012 13:07:53 -0400 [thread overview]
Message-ID: <20120829170753.GA1895@zod.bos.redhat.com> (raw)
In-Reply-To: <503E19F2.3080302@gmail.com>
On Wed, Aug 29, 2012 at 03:32:34PM +0200, Daniel Mack wrote:
> On 29.08.2012 15:29, Takashi Iwai wrote:
> > At Wed, 29 Aug 2012 13:26:25 +0200,
> > Daniel Mack wrote:
> >>
> >> [1 <text/plain; ISO-8859-1 (7bit)>]
> >> On 25.08.2012 14:17, Josh Boyer wrote:
> >>> On Sat, Aug 25, 2012 at 02:13:58PM +0200, Daniel Mack wrote:
> >>>> On 25.08.2012 14:07, Bruno Wolff III wrote:
> >>>>> On Sat, Aug 25, 2012 at 14:02:51 +0200,
> >>>>> Daniel Mack <zonque@gmail.com> wrote:
> >>>>>>
> >>>>>> Can you revert commit e9ba389c5 ("ALSA: usb-audio: Fix
> >>>>>> scheduling-while-atomic bug in PCM capture stream") and see if that
> >>>>>
> >>>>> I can try that, but it takes a long time to build a new kernel on my
> >>>>> old hardware.
> >>>>>
> >>>>>> helps? If not, can you summarize again which kernels still work for you
> >>>>>> and which don't?
> >>>>>
> >>>>> The latest kernel that works is 3.6.0-0.rc2.git1.2.fc18. The earliest that
> >>>>> doesn't work is 3.6.0-0.rc2.git2.1.fc18.
> >>>>>
> >>>>
> >>>> The report you sent doesn't look like it could be caused by e9ba389c5.
> >>>> It fixes a kernel Ooops. But as it is the only relevant patch in that
> >>>> area, it would be interesting if reverting it fixes anything.
> >>>
> >>> Yep, agreed. If this revert kernel doesn't work, we're likely down to a
> >>> git bisect, Bruno.
> >>>
> >>>> Btw - thanks a lot for testing -rc kernels, much appreciated!
> >>>
> >>> Indeed!
> >>
> >> Could you please try this patch on top of Takashi's? Thanks again!
> >>
> >>
> >> Daniel
> >>
> >> [2 0001-ALSA-snd-usb-Fix-URB-cancellation-at-stream-start.patch <text/x-patch (7bit)>]
> >> >From 6617bb2463ae3fad21390b59cc2a71f96b9e9ca8 Mon Sep 17 00:00:00 2001
> >> From: Daniel Mack <zonque@gmail.com>
> >> Date: Wed, 29 Aug 2012 13:17:05 +0200
> >> Subject: [PATCH] ALSA: snd-usb: Fix URB cancellation at stream start
> >>
> >> Commit e9ba389c5 ("ALSA: usb-audio: Fix scheduling-while-atomic bug in
> >> PCM capture stream") fixed a scheduling-while-atomic bug that happened
> >> when snd_usb_endpoint_start was called from the trigger callback, which
> >> is an atmic context. However, the patch breaks the idea of the endpoints
> >> reference counting, which is the reason why the driver has been
> >> refactored lately.
> >>
> >> Revert that commit and let snd_usb_endpoint_start() take care of the URB
> >> cancellation again. As this function is called from both atomic and
> >> non-atomic context, add a flag to denote whether the function may sleep.
> >>
> >> Signed-off-by: Daniel Mack <zonque@gmail.com>
> >> Cc: stable@kernel.org [3.5+]
> >> ---
> >> sound/usb/endpoint.c | 10 ++++++++--
> >> sound/usb/endpoint.h | 2 +-
> >> sound/usb/pcm.c | 13 +++++--------
> >> 3 files changed, 14 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> >> index c411812..678456c 100644
> >> --- a/sound/usb/endpoint.c
> >> +++ b/sound/usb/endpoint.c
> >> @@ -799,7 +799,9 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
> >> /**
> >> * snd_usb_endpoint_start: start an snd_usb_endpoint
> >> *
> >> - * @ep: the endpoint to start
> >> + * @ep: the endpoint to start
> >> + * @can_sleep: flag indicating whether the operation is executed in
> >> + * non-atomic context
> >> *
> >> * A call to this function will increment the use count of the endpoint.
> >> * In case it is not already running, the URBs for this endpoint will be
> >> @@ -809,7 +811,7 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
> >> *
> >> * Returns an error if the URB submission failed, 0 in all other cases.
> >> */
> >> -int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
> >> +int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, int can_sleep)
> >> {
> >> int err;
> >> unsigned int i;
> >> @@ -821,6 +823,10 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
> >> if (++ep->use_count != 1)
> >> return 0;
> >>
> >> + /* just to be sure */
> >> + deactivate_urbs(ep, 0, can_sleep);
> >> + wait_clear_urbs(ep);
> >
> > It'd be safer to protect the call of wait_clear_urbs() when
> > can_sleep=0.
>
> Right. New patch attached.
I'll try and get another kernel built with this later today. I'm at
Plumbers so it might have to wait a bit.
Bruno, if you're comfortable adding the patch to the Fedora 18 kernel
via a spec file, you should be able to do a scratch build by passing an
SRPM. If you get tired of waiting for me, give it a shot.
josh
next prev parent reply other threads:[~2012-08-29 17:08 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-24 19:08 Logitech USB headset not working in 3.6-rc3 Josh Boyer
2012-08-24 21:30 ` Daniel Mack
2012-08-24 22:30 ` Josh Boyer
2012-08-24 22:30 ` Daniel Mack
2012-08-25 11:17 ` Bruno Wolff III
2012-08-25 11:54 ` Bruno Wolff III
2012-08-25 12:02 ` Daniel Mack
2012-08-25 12:07 ` Bruno Wolff III
2012-08-25 12:13 ` Josh Boyer
2012-08-25 12:16 ` Bruno Wolff III
2012-08-26 12:46 ` Kernel module build workflow (Re: Logitech USB headset not working in 3.6-rc3) Takashi Iwai
[not found] ` <CAFz=ag47OU1GbS-a8GUug4bJ04JA6kn_uBUZ0LiBjzUCJ0fN_w@mail.gmail.com>
2012-08-27 18:46 ` [alsa-devel] " Takashi Iwai
2012-08-25 15:00 ` Logitech USB headset not working in 3.6-rc3 Bruno Wolff III
2012-08-25 12:13 ` Daniel Mack
2012-08-25 12:17 ` Josh Boyer
2012-08-25 13:45 ` Takashi Iwai
2012-08-29 11:26 ` Daniel Mack
2012-08-29 13:29 ` Takashi Iwai
2012-08-29 13:32 ` Daniel Mack
2012-08-29 14:14 ` Takashi Iwai
2012-08-29 14:25 ` Daniel Mack
2012-08-29 15:01 ` Takashi Iwai
2012-08-29 17:07 ` Josh Boyer [this message]
2012-08-29 17:22 ` Josh Boyer
2012-08-29 18:50 ` Bruno Wolff III
2012-08-30 14:10 ` Takashi Iwai
2012-08-30 15:03 ` Bruno Wolff III
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=20120829170753.GA1895@zod.bos.redhat.com \
--to=jwboyer@redhat.com \
--cc=alsa-devel@alsa-project.org \
--cc=bruno@wolff.to \
--cc=linux-kernel@vger.kernel.org \
--cc=perex@perex.cz \
--cc=tiwai@suse.de \
--cc=zonque@gmail.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;
as well as URLs for NNTP newsgroup(s).