Name: Request Module Returns Void Author: Mitchell Blank Jr , Rusty Russell Status: Booted on 2.6.0-test5-bk3 request_module returns an error code: you can't rely on that, so don't. The module might have been removed again, or not register what you want: you have to search again. Remove the temptation: make it return void. Also, use a macro if !CONFIG_KMOD, to get rid of the string literal. diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .27081-linux-2.6.0-test5-bk12/drivers/ide/ide-probe.c .27081-linux-2.6.0-test5-bk12.updated/drivers/ide/ide-probe.c --- .27081-linux-2.6.0-test5-bk12/drivers/ide/ide-probe.c 2003-09-22 10:27:58.000000000 +1000 +++ .27081-linux-2.6.0-test5-bk12.updated/drivers/ide/ide-probe.c 2003-09-27 11:29:38.000000000 +1000 @@ -1164,15 +1164,15 @@ struct kobject *ata_probe(dev_t dev, int return NULL; if (drive->driver == &idedefault_driver) { if (drive->media == ide_disk) - (void) request_module("ide-disk"); + request_module("ide-disk"); if (drive->scsi) - (void) request_module("ide-scsi"); + request_module("ide-scsi"); if (drive->media == ide_cdrom || drive->media == ide_optical) - (void) request_module("ide-cd"); + request_module("ide-cd"); if (drive->media == ide_tape) - (void) request_module("ide-tape"); + request_module("ide-tape"); if (drive->media == ide_floppy) - (void) request_module("ide-floppy"); + request_module("ide-floppy"); } if (drive->driver == &idedefault_driver) return NULL; diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .27081-linux-2.6.0-test5-bk12/drivers/ide/ide.c .27081-linux-2.6.0-test5-bk12.updated/drivers/ide/ide.c --- .27081-linux-2.6.0-test5-bk12/drivers/ide/ide.c 2003-09-22 10:27:58.000000000 +1000 +++ .27081-linux-2.6.0-test5-bk12.updated/drivers/ide/ide.c 2003-09-27 11:29:38.000000000 +1000 @@ -448,7 +448,7 @@ void ide_probe_module (void) { if (!ide_probe) { #if defined(CONFIG_KMOD) && defined(CONFIG_BLK_DEV_IDE_MODULE) - (void) request_module("ide-probe-mod"); + request_module("ide-probe-mod"); #endif /* (CONFIG_KMOD) && (CONFIG_BLK_DEV_IDE_MODULE) */ } else { (void)ide_probe(); diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .27081-linux-2.6.0-test5-bk12/drivers/media/video/zoran_card.c .27081-linux-2.6.0-test5-bk12.updated/drivers/media/video/zoran_card.c --- .27081-linux-2.6.0-test5-bk12/drivers/media/video/zoran_card.c 2003-09-22 10:27:59.000000000 +1000 +++ .27081-linux-2.6.0-test5-bk12.updated/drivers/media/video/zoran_card.c 2003-09-27 11:29:38.000000000 +1000 @@ -1349,12 +1349,7 @@ find_zr36057 (void) } if (i2c_dec_name) { - if ((result = request_module(i2c_dec_name)) < 0) { - dprintk(1, - KERN_ERR - "%s: failed to load module %s: %d\n", - ZR_DEVNAME(zr), i2c_dec_name, result); - } + request_module(i2c_dec_name); } /* i2c encoder */ @@ -1369,12 +1364,7 @@ find_zr36057 (void) } if (i2c_enc_name) { - if ((result = request_module(i2c_enc_name)) < 0) { - dprintk(1, - KERN_ERR - "%s: failed to load module %s: %d\n", - ZR_DEVNAME(zr), i2c_enc_name, result); - } + request_module(i2c_enc_name); } if (zoran_register_i2c(zr) < 0) { @@ -1392,22 +1382,12 @@ find_zr36057 (void) if (zr->card.video_codec != 0 && (codec_name = codecid_to_modulename(zr->card.video_codec)) != NULL) { - if ((result = request_module(codec_name)) < 0) { - dprintk(1, - KERN_ERR - "%s: failed to load modules %s: %d\n", - ZR_DEVNAME(zr), codec_name, result); - } + request_module(codec_name); } if (zr->card.video_vfe != 0 && (vfe_name = codecid_to_modulename(zr->card.video_vfe)) != NULL) { - if ((result = request_module(vfe_name)) < 0) { - dprintk(1, - KERN_ERR - "%s: failed to load modules %s: %d\n", - ZR_DEVNAME(zr), vfe_name, result); - } + request_module(vfe_name); } /* reset JPEG codec */ diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .27081-linux-2.6.0-test5-bk12/drivers/mtd/chips/chipreg.c .27081-linux-2.6.0-test5-bk12.updated/drivers/mtd/chips/chipreg.c --- .27081-linux-2.6.0-test5-bk12/drivers/mtd/chips/chipreg.c 2003-09-22 10:09:01.000000000 +1000 +++ .27081-linux-2.6.0-test5-bk12.updated/drivers/mtd/chips/chipreg.c 2003-09-27 11:29:38.000000000 +1000 @@ -63,11 +63,7 @@ struct mtd_info *do_map_probe(const char struct mtd_chip_driver *drv; struct mtd_info *ret; - drv = get_mtd_chip_driver(name); - - if (!drv && !request_module("%s", name)) - drv = get_mtd_chip_driver(name); - + drv = try_then_request_module(get_mtd_chip_driver(name), "%s", name); if (!drv) return NULL; diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .27081-linux-2.6.0-test5-bk12/drivers/mtd/mtdpart.c .27081-linux-2.6.0-test5-bk12.updated/drivers/mtd/mtdpart.c --- .27081-linux-2.6.0-test5-bk12/drivers/mtd/mtdpart.c 2003-09-22 10:22:14.000000000 +1000 +++ .27081-linux-2.6.0-test5-bk12.updated/drivers/mtd/mtdpart.c 2003-09-27 11:29:38.000000000 +1000 @@ -526,11 +526,8 @@ int parse_mtd_partitions(struct mtd_info int ret = 0; for ( ; ret <= 0 && *types; types++) { - parser = get_partition_parser(*types); -#ifdef CONFIG_KMOD - if (!parser && !request_module("%s", *types)) - parser = get_partition_parser(*types); -#endif + parser = try_then_request_module(get_partition_parser(*types), + "%s", *types); if (!parser) { printk(KERN_NOTICE "%s partition parsing not available\n", *types); diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .27081-linux-2.6.0-test5-bk12/fs/dquot.c .27081-linux-2.6.0-test5-bk12.updated/fs/dquot.c --- .27081-linux-2.6.0-test5-bk12/fs/dquot.c 2003-09-22 10:23:11.000000000 +1000 +++ .27081-linux-2.6.0-test5-bk12.updated/fs/dquot.c 2003-09-27 11:29:38.000000000 +1000 @@ -129,10 +129,8 @@ static struct quota_format_type *find_qu int qm; for (qm = 0; module_names[qm].qm_fmt_id && module_names[qm].qm_fmt_id != id; qm++); - if (!module_names[qm].qm_fmt_id || request_module(module_names[qm].qm_mod_name)) { - actqf = NULL; - goto out; - } + if (!module_names[qm].qm_fmt_id) + request_module(module_names[qm].qm_mod_name); for (actqf = quota_formats; actqf && actqf->qf_fmt_id != id; actqf = actqf->qf_next); if (actqf && !try_module_get(actqf->qf_owner)) actqf = NULL; diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .27081-linux-2.6.0-test5-bk12/fs/filesystems.c .27081-linux-2.6.0-test5-bk12.updated/fs/filesystems.c --- .27081-linux-2.6.0-test5-bk12/fs/filesystems.c 2003-09-22 10:09:07.000000000 +1000 +++ .27081-linux-2.6.0-test5-bk12.updated/fs/filesystems.c 2003-09-27 11:29:38.000000000 +1000 @@ -209,21 +209,18 @@ int get_filesystem_list(char * buf) return len; } -struct file_system_type *get_fs_type(const char *name) +static struct file_system_type *__get_fs_type(const char *name) { struct file_system_type *fs; - read_lock(&file_systems_lock); fs = *(find_filesystem(name)); if (fs && !try_module_get(fs->owner)) fs = NULL; read_unlock(&file_systems_lock); - if (!fs && (request_module("%s", name) == 0)) { - read_lock(&file_systems_lock); - fs = *(find_filesystem(name)); - if (fs && !try_module_get(fs->owner)) - fs = NULL; - read_unlock(&file_systems_lock); - } return fs; } + +struct file_system_type *get_fs_type(const char *name) +{ + return try_then_request_module(__get_fs_type(name), "%s", name); +} diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .27081-linux-2.6.0-test5-bk12/fs/nls/nls_base.c .27081-linux-2.6.0-test5-bk12.updated/fs/nls/nls_base.c --- .27081-linux-2.6.0-test5-bk12/fs/nls/nls_base.c 2003-09-26 11:08:06.000000000 +1000 +++ .27081-linux-2.6.0-test5-bk12.updated/fs/nls/nls_base.c 2003-09-27 11:29:38.000000000 +1000 @@ -227,12 +227,12 @@ struct nls_table *load_nls(char *charset return nls; #ifdef CONFIG_KMOD - ret = request_module("nls_%s", charset); - if (ret != 0) { + request_module("nls_%s", charset); + nls = find_nls(charset); + if (!nls) { printk("Unable to load NLS charset %s\n", charset); return NULL; } - nls = find_nls(charset); #endif return nls; } diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .27081-linux-2.6.0-test5-bk12/include/linux/kmod.h .27081-linux-2.6.0-test5-bk12.updated/include/linux/kmod.h --- .27081-linux-2.6.0-test5-bk12/include/linux/kmod.h 2003-09-22 10:28:12.000000000 +1000 +++ .27081-linux-2.6.0-test5-bk12.updated/include/linux/kmod.h 2003-09-27 11:31:50.000000000 +1000 @@ -20,16 +20,19 @@ */ #include +#include +/* FIXME Sep 2003: Too many files rely on this #include indirectly. Remove */ #include -#include #ifdef CONFIG_KMOD -extern int request_module(const char * name, ...) __attribute__ ((format (printf, 1, 2))); +extern void request_module(const char *name, ...) __attribute__ ((format (printf, 1, 2))); +#define try_then_request_module(x, mod...) \ + ({ might_sleep(); (x) ?: ({ request_module(mod); (x); }); }) #else -static inline int request_module(const char * name, ...) { return -ENOSYS; } +#define request_module(name...) +#define try_then_request_module(x, mod...) {( might_sleep(); (x); )} #endif -#define try_then_request_module(x, mod...) ((x) ?: (request_module(mod), (x))) extern int call_usermodehelper(char *path, char *argv[], char *envp[], int wait); #ifdef CONFIG_HOTPLUG diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .27081-linux-2.6.0-test5-bk12/kernel/kmod.c .27081-linux-2.6.0-test5-bk12.updated/kernel/kmod.c --- .27081-linux-2.6.0-test5-bk12/kernel/kmod.c 2003-09-22 10:28:13.000000000 +1000 +++ .27081-linux-2.6.0-test5-bk12.updated/kernel/kmod.c 2003-09-27 11:29:38.000000000 +1000 @@ -49,16 +49,14 @@ char modprobe_path[256] = "/sbin/modprob * request_module - try to load a kernel module * @module_name: Name of module * - * Load a module using the user mode module loader. The function returns - * zero on success or a negative errno code on failure. Note that a - * successful module load does not mean the module did not then unload - * and exit on an error of its own. Callers must check that the service - * they requested is now available not blindly invoke it. + * Load a module using the user mode module loader. Callers must check + * that the service they requested is now available not blindly invoke it: + * see try_then_request_module(). * * If module auto-loading support is disabled then this function * becomes a no-operation. */ -int request_module(const char *fmt, ...) +void request_module(const char *fmt, ...) { va_list args; char module_name[MODULE_NAME_LEN]; @@ -76,8 +74,11 @@ int request_module(const char *fmt, ...) va_start(args, fmt); ret = vsnprintf(module_name, MODULE_NAME_LEN, fmt, args); va_end(args); - if (ret >= MODULE_NAME_LEN) - return -ENAMETOOLONG; + if (ret >= MODULE_NAME_LEN) { + printk(KERN_ERR "request_module for %i chars\n", ret); + return; + } + /* If modprobe needs a service that is in a module, we get a recursive * loop. Limit the number of running kmod threads to max_threads/2 or @@ -100,7 +101,7 @@ int request_module(const char *fmt, ...) "request_module: runaway loop modprobe %s\n", module_name); atomic_dec(&kmod_concurrent); - return -ENOMEM; + return; } ret = call_usermodehelper(modprobe_path, argv, envp, 1); @@ -115,7 +116,6 @@ int request_module(const char *fmt, ...) } } atomic_dec(&kmod_concurrent); - return ret; } #endif /* CONFIG_KMOD */