linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Ming Lei <ming.lei@canonical.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rjw@sisk.pl>, Kay Sievers <kay@vrfy.org>,
	linux-kernel@vger.kernel.org
Subject: Re: A workaround for request_firmware() stuck in module_init
Date: Tue, 04 Sep 2012 18:10:03 +0200	[thread overview]
Message-ID: <s5hd32122hw.wl%tiwai@suse.de> (raw)
In-Reply-To: <CACVXFVP-A7M2dG9bOvAnpzR60=iTYSw=WCOEwgo9KeuioxKanQ@mail.gmail.com>

At Tue, 4 Sep 2012 23:52:15 +0800,
Ming Lei wrote:
> 
> On Tue, Sep 4, 2012 at 9:06 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > Hi,
> >
> > as I've got recently a few bug reports regarding the stuck with
> > request_firmware() in module_init of some sound drivers, I started
> > looking at the issue.  Strangely, the problem doesn't happen on
> > openSUSE 12.2 although it has the same udev version with libkmod as
> > Fedora.  So I installed Fedora 17, and indeed I could see a problem
> > there.
> 
> It should be a bug in udev, as discussed in the below link:
> 
>           http://marc.info/?t=134552745100002&r=1&w=2

Yeah, if udev has a fix, I'm fine.  I'll proactively ignore these bug
reports.  But did it really happen...?

> > Obviously a solution would be to rewrite the driver code to use
> > request_firmware_nowait() instead.  But it'd need a lot of code
> > shuffling, and most of such drivers are old stuff I don't want to do a
> > serious surgery.
> >
> > So I tried an easier workaround by using the deferred probing.
> > An experimental patch is below.  As you can see, from the driver side,
> > it's simple: just add two lines at the head of each probe function.
> 
> In fact, the defer probe should only be applied in situations which
> driver is built in kernel and request_firmware is called in .probe().

Is it?  I thought the deferred probe is basically not for this problem
but rather for the dependency problem with other modules.  That's the
reason it's triggered only upon the successful binding.

And IMO, the deferred probe for the built-in kernel is also silly.
Did anyone really make it working for built-in kernel driver and
external firmware files?  (For the resume, it's of course a different
issue.  And I guess it's been solved by your fw cache patch, right?)

> > Do you think this kind of hack is OK?  If not, any better (IOW easier)
> > solution?
> 
> Looks your solution is a bit complicated, and I have wrote a similar
> patch to address the problem, but Linus thought that it may hide the
> problem of drivers.
> 
>         http://marc.info/?t=134278790800004&r=1&w=2
> 
> IMO, driver core needn't to be changed, and the defer probe can be
> supported just by changes in request_firmware() and its caller.

Ideally, yes.  But it won't work in some drivers like sound drivers,
that have its own device number counting changed at each probe call.
For such drivers, the deferred probe check must be done before
entering the normal probe procedure, and returning -EPROBE_DEFER would
be too late (or need more complex fallbacks).


thanks,

Takashi

  reply	other threads:[~2012-09-04 16:10 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-04 13:06 A workaround for request_firmware() stuck in module_init Takashi Iwai
2012-09-04 15:52 ` Ming Lei
2012-09-04 16:10   ` Takashi Iwai [this message]
2012-09-05  1:15     ` Ming Lei
2012-09-05  5:53       ` Takashi Iwai
2012-09-05 11:32         ` Ming Lei
2012-09-05 13:03           ` Alan Cox
2012-09-05 14:01             ` Takashi Iwai
2012-09-05 15:22             ` Ming Lei
2012-09-05 16:30               ` Alan Cox
2012-09-05 21:08                 ` Benjamin Herrenschmidt
2012-09-05 23:18                   ` Alan Cox
2012-09-06  5:06                     ` Benjamin Herrenschmidt
2012-09-06  2:47                 ` Linus Torvalds
2012-09-06  4:12                 ` Ming Lei
2012-09-06 12:59                   ` Alan Cox
2012-09-06 15:38                     ` Ming Lei
2012-09-05 16:59             ` Lucas De Marchi
2012-09-05 17:09               ` Takashi Iwai
2012-09-05 16:51 ` Lucas De Marchi
2012-09-05 17:08   ` Takashi Iwai

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=s5hd32122hw.wl%tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=kay@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.lei@canonical.com \
    --cc=rjw@sisk.pl \
    /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).