From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757047Ab2IDNGx (ORCPT ); Tue, 4 Sep 2012 09:06:53 -0400 Received: from cantor2.suse.de ([195.135.220.15]:53041 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753037Ab2IDNGw (ORCPT ); Tue, 4 Sep 2012 09:06:52 -0400 Date: Tue, 04 Sep 2012 15:06:47 +0200 Message-ID: From: Takashi Iwai To: Greg Kroah-Hartman Cc: Ming Lei , "Rafael J. Wysocki" , Kay Sievers , linux-kernel@vger.kernel.org Subject: A workaround for request_firmware() stuck in module_init User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.1 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. 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. Do you think this kind of hack is OK? If not, any better (IOW easier) solution? thanks, Takashi === Subject: [PATCH] driver-core: Add a helper to work around the stuck with request_firmware() Since the recent udev loads the module with libkmod, the module loading works no longer properly when a driver calls request_firmware() in module_init. Certainly we can fix all these with request_firmware_nowait(), but it'd need fairly lots of code changes, so it's no preferred for a lazy person like me. This patch adds an easier workaround: use the deferred probing. The driver that may call request_firmware() in module_init should call the new helper function dev_defer_for_fw_load(), and returns immediately with -EPROBE_DEFER if it's true. If it's false, simply continues the rest. That's all. In the driver core side, a new bit flag field is added to the device private data for bookkeeping and triggering the deferred probe explicitly. (Otherwise the trigger won't happen unless any new driver binding occurs.) As an example implementation, the patch contains the fix for sound/pci/rme9652/hdsp.c, too. Signed-off-by: Takashi Iwai --- drivers/base/base.h | 6 ++++++ drivers/base/dd.c | 25 +++++++++++++++++++++++++ include/linux/firmware.h | 6 ++++++ sound/pci/rme9652/hdsp.c | 3 +++ 4 files changed, 40 insertions(+) diff --git a/drivers/base/base.h b/drivers/base/base.h index 6ee17bb..de5a7ca 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -67,6 +67,7 @@ struct driver_private { * list soon. * @device - pointer back to the struct class that this structure is * associated with. + * @flags - extra bit flags, currently used for deferred probing for fw loader * * Nothing outside of the driver core should ever touch these fields. */ @@ -78,6 +79,7 @@ struct device_private { struct list_head deferred_probe; void *driver_data; struct device *device; + unsigned int flags; }; #define to_device_private_parent(obj) \ container_of(obj, struct device_private, knode_parent) @@ -86,6 +88,10 @@ struct device_private { #define to_device_private_bus(obj) \ container_of(obj, struct device_private, knode_bus) +/* bit flags */ +#define DEV_FLAG_IN_DEFERRED_PROBE (1 << 0) +#define DEV_FLAG_NEED_TRIGGER (1 << 1) + extern int device_private_init(struct device *dev); /* initialisation functions */ diff --git a/drivers/base/dd.c b/drivers/base/dd.c index e3bbed8..aaefa7e 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -97,7 +97,9 @@ static void deferred_probe_work_func(struct work_struct *work) device_pm_unlock(); dev_dbg(dev, "Retrying from deferred list\n"); + dev->p->flags |= DEV_FLAG_IN_DEFERRED_PROBE; bus_probe_device(dev); + dev->p->flags &= ~DEV_FLAG_IN_DEFERRED_PROBE; mutex_lock(&deferred_probe_mutex); @@ -301,6 +303,10 @@ probe_failed: /* Driver requested deferred probing */ dev_info(dev, "Driver %s requests probe deferral\n", drv->name); driver_deferred_probe_add(dev); + if (dev->p->flags & DEV_FLAG_NEED_TRIGGER) { + driver_deferred_probe_trigger(); + dev->p->flags &= ~DEV_FLAG_NEED_TRIGGER; + } } else if (ret != -ENODEV && ret != -ENXIO) { /* driver matched but the probe failed */ printk(KERN_WARNING @@ -587,3 +593,22 @@ int dev_set_drvdata(struct device *dev, void *data) return 0; } EXPORT_SYMBOL(dev_set_drvdata); + +#ifdef CONFIG_FW_LOADER +/** + * dev_defer_for_fw_load - check if deferred probe for fw loader is needed + * @dev: device + * + * When a driver may invoke request_firmware() in its module init, call this + * function at the beginning of the probe function. When it's true, the probe + * should return immediately with -EPROBE_DEFER. + */ +bool dev_defer_for_fw_load(struct device *dev) +{ + if (dev->p->flags & DEV_FLAG_IN_DEFERRED_PROBE) + return false; + dev->p->flags |= DEV_FLAG_NEED_TRIGGER; + return true; +} +EXPORT_SYMBOL_GPL(dev_defer_for_fw_load); +#endif diff --git a/include/linux/firmware.h b/include/linux/firmware.h index 1e7c011..7caa96c 100644 --- a/include/linux/firmware.h +++ b/include/linux/firmware.h @@ -44,6 +44,7 @@ int request_firmware_nowait( void (*cont)(const struct firmware *fw, void *context)); void release_firmware(const struct firmware *fw); +bool dev_defer_for_fw_load(struct device *dev); #else static inline int request_firmware(const struct firmware **fw, const char *name, @@ -62,6 +63,11 @@ static inline int request_firmware_nowait( static inline void release_firmware(const struct firmware *fw) { } + +static inline bool dev_defer_for_fw_load(struct device *dev) +{ + return false; +} #endif #endif diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c index 0d6930c..427ac24 100644 --- a/sound/pci/rme9652/hdsp.c +++ b/sound/pci/rme9652/hdsp.c @@ -5594,6 +5594,9 @@ static int __devinit snd_hdsp_probe(struct pci_dev *pci, struct snd_card *card; int err; + if (dev_defer_for_fw_load(&pci->dev)) + return -EPROBE_DEFER; + if (dev >= SNDRV_CARDS) return -ENODEV; if (!enable[dev]) { -- 1.7.11.5