avoid deprecated module functions in core code

PATCH In review Christoph Hellwig 1 participant 2 messages Dec 29, 2002 – Dec 30, 2002

Subsystems (9)

VFS Block layer MM core Crypto API +5 more subsystems

lore

Share & tools

lore

Christoph Hellwig

author

2002-12-29 21:51 UTC #0 lore raw

PATCH 1/1 avoid deprecated module functions in core code

A first start at removing them from kernel/*.c and fs/*.c.

Note that module_put is fine for a NULL argument.

fs/block_dev.c +5 -6

@@ -623,8 +623,7 @@
 		}
 	} else {
 		put_disk(disk);
-		if (owner)
-			__MOD_DEC_USE_COUNT(owner);
+		module_put(owner);
 		if (bdev->bd_contains == bdev) {
 			if (bdev->bd_disk->fops->open) {
 				ret = bdev->bd_disk->fops->open(inode, file);

@@ -651,8 +650,7 @@
 		blkdev_put(bdev->bd_contains, BDEV_RAW);
 	bdev->bd_contains = NULL;
 	put_disk(disk);
-	if (owner)
-		__MOD_DEC_USE_COUNT(owner);
+	module_put(owner);
 out:
 	up(&bdev->bd_sem);
 	unlock_kernel();

@@ -723,9 +721,10 @@
 	}
 	if (!bdev->bd_openers) {
 		struct module *owner = disk->fops->owner;
+
 		put_disk(disk);
-		if (owner)
-			__MOD_DEC_USE_COUNT(owner);
+		module_put(owner);
+
 		bdev->bd_disk = NULL;
 		bdev->bd_inode->i_data.backing_dev_info = &default_backing_dev_info;
 		if (bdev != bdev->bd_contains) {

===== fs/dquot.c 1.52 vs edited =====

fs/dquot.c +1 -2

@@ -111,8 +111,7 @@
 
 static void put_quota_format(struct quota_format_type *fmt)
 {
-	if (fmt->qf_owner)
-		__MOD_DEC_USE_COUNT(fmt->qf_owner);
+	module_put(fmt->qf_owner);
 }
 
 /*

===== fs/exec.c 1.58 vs edited =====

fs/exec.c +4 -5

@@ -102,8 +102,7 @@
 
 static inline void put_binfmt(struct linux_binfmt * fmt)
 {
-	if (fmt->module)
-		__MOD_DEC_USE_COUNT(fmt->module);
+	module_put(fmt->module);
 }
 
 /*

@@ -1111,11 +1110,11 @@
 void set_binfmt(struct linux_binfmt *new)
 {
 	struct linux_binfmt *old = current->binfmt;
-	if (new && new->module)
+	if (new)
 		__MOD_INC_USE_COUNT(new->module);
 	current->binfmt = new;
-	if (old && old->module)
-		__MOD_DEC_USE_COUNT(old->module);
+	if (old)
+		module_put(old->module);
 }
 
 #define CORENAME_MAX_SIZE 64

kernel/exit.c +2 -2

@@ -665,8 +665,8 @@
 		disassociate_ctty(1);
 
 	put_exec_domain(tsk->thread_info->exec_domain);
-	if (tsk->binfmt && tsk->binfmt->module)
-		__MOD_DEC_USE_COUNT(tsk->binfmt->module);
+	if (tsk->binfmt)
+		module_put(tsk->binfmt);
 
 	tsk->exit_code = code;
 	exit_notify();

===== kernel/fork.c 1.93 vs edited =====

kernel/fork.c +5 -4

@@ -745,8 +745,8 @@
 	
 	get_exec_domain(p->thread_info->exec_domain);
 
-	if (p->binfmt && p->binfmt->module)
-		__MOD_INC_USE_COUNT(p->binfmt->module);
+	if (p->binfmt && !try_module_get(p->binfmt))
+		goto bad_fork_cleanup_put;
 
 #ifdef CONFIG_PREEMPT
 	/*

@@ -958,9 +958,10 @@
 bad_fork_cleanup:
 	if (p->pid > 0)
 		free_pidmap(p->pid);
+	if (p->binfmt)
+		module_put(p->binfmt);
+bad_fork_cleanup_put:
 	put_exec_domain(p->thread_info->exec_domain);
-	if (p->binfmt && p->binfmt->module)
-		__MOD_DEC_USE_COUNT(p->binfmt->module);
 bad_fork_cleanup_count:
 	atomic_dec(&p->user->processes);
 	free_uid(p->user);

===== kernel/intermodule.c 1.1 vs edited =====

kernel/intermodule.c +1 -1

@@ -166,7 +166,7 @@
 		ime = list_entry(tmp, struct inter_module_entry, list);
 		if (strcmp(ime->im_name, im_name) == 0) {
 			if (ime->owner)
-				__MOD_DEC_USE_COUNT(ime->owner);
+				module_put(ime->owner);
 			spin_unlock(&ime_lock);
 			return;
 		}

Christoph Hellwig

author

2002-12-30 00:13 UTC #1 lore raw

Quoted 3 lines · Christoph Hellwig

A first start at removing them from kernel/*.c and fs/*.c.
Note that module_put is fine for a NULL argument.

Two persons told me independantly that this patch is utter crap and doesn't even compile. And they're right..

Here's a version that does not even compile but even boot and cover more uses.

Note that the return value of set_binfmt isn't yet checked. That's another patch.

crypto/api.c +1 -2

@@ -29,8 +29,7 @@
 
 static inline void crypto_alg_put(struct crypto_alg *alg)
 {
-	if (alg->cra_module)
-		__MOD_DEC_USE_COUNT(alg->cra_module);
+	module_put(alg->cra_module);
 }
 
 struct crypto_alg *crypto_alg_lookup(const char *name)

drivers/block/genhd.c +2 -4

@@ -168,15 +168,13 @@
 		best = p->range;
 		*part = dev - p->dev;
 		if (p->lock && p->lock(dev, data) < 0) {
-			if (owner)
-				__MOD_DEC_USE_COUNT(owner);
+			module_put(owner);
 			continue;
 		}
 		read_unlock(&gendisk_lock);
 		disk = probe(dev, part, data);
 		/* Currently ->owner protects _only_ ->probe() itself. */
-		if (owner)
-			__MOD_DEC_USE_COUNT(owner);
+		module_put(owner);
 		if (disk)
 			return disk;
 		goto retry;

fs/block_dev.c +5 -6

@@ -623,8 +623,7 @@
 		}
 	} else {
 		put_disk(disk);
-		if (owner)
-			__MOD_DEC_USE_COUNT(owner);
+		module_put(owner);
 		if (bdev->bd_contains == bdev) {
 			if (bdev->bd_disk->fops->open) {
 				ret = bdev->bd_disk->fops->open(inode, file);

@@ -651,8 +650,7 @@
 		blkdev_put(bdev->bd_contains, BDEV_RAW);
 	bdev->bd_contains = NULL;
 	put_disk(disk);
-	if (owner)
-		__MOD_DEC_USE_COUNT(owner);
+	module_put(owner);
 out:
 	up(&bdev->bd_sem);
 	unlock_kernel();

@@ -723,9 +721,10 @@
 	}
 	if (!bdev->bd_openers) {
 		struct module *owner = disk->fops->owner;
+
 		put_disk(disk);
-		if (owner)
-			__MOD_DEC_USE_COUNT(owner);
+		module_put(owner);
+
 		bdev->bd_disk = NULL;
 		bdev->bd_inode->i_data.backing_dev_info = &default_backing_dev_info;
 		if (bdev != bdev->bd_contains) {

===== fs/dquot.c 1.52 vs edited =====

fs/dquot.c +1 -2

@@ -111,8 +111,7 @@
 
 static void put_quota_format(struct quota_format_type *fmt)
 {
-	if (fmt->qf_owner)
-		__MOD_DEC_USE_COUNT(fmt->qf_owner);
+	module_put(fmt->qf_owner);
 }
 
 /*

===== fs/exec.c 1.58 vs edited =====

fs/exec.c +10 -7

@@ -102,8 +102,7 @@
 
 static inline void put_binfmt(struct linux_binfmt * fmt)
 {
-	if (fmt->module)
-		__MOD_DEC_USE_COUNT(fmt->module);
+	module_put(fmt->module);
 }
 
 /*

@@ -1108,14 +1107,18 @@
 	return retval;
 }
 
-void set_binfmt(struct linux_binfmt *new)
+int set_binfmt(struct linux_binfmt *new)
 {
 	struct linux_binfmt *old = current->binfmt;
-	if (new && new->module)
-		__MOD_INC_USE_COUNT(new->module);
+
+	if (new) {
+		if (!try_module_get(new->module))
+			return -1;
+	}
 	current->binfmt = new;
-	if (old && old->module)
-		__MOD_DEC_USE_COUNT(old->module);
+	if (old)
+		module_put(old->module);
+	return 0;
 }
 
 #define CORENAME_MAX_SIZE 64

===== fs/nls/nls_base.c 1.7 vs edited =====

fs/nls/nls_base.c +1 -2

@@ -245,8 +245,7 @@
 
 void unload_nls(struct nls_table *nls)
 {
-	if (nls->owner)
-		__MOD_DEC_USE_COUNT(nls->owner);
+	module_put(nls->owner);
 }
 
 wchar_t charset2uni[256] = {

===== fs/partitions/check.c 1.90 vs edited =====

fs/partitions/check.c +1 -2

@@ -565,8 +565,7 @@
 	dname->name = NULL;
 	if (hd) {
 		dname->name = disk_name(hd, part, dname->namebuf);
-		if (hd->fops->owner)
-			__MOD_DEC_USE_COUNT(hd->fops->owner);
+		module_put(hd->fops->owner);
 		put_disk(hd);
 	}
 	if (!dname->name) {

===== include/linux/binfmts.h 1.5 vs edited =====

include/linux/binfmts.h +1 -7

@@ -58,13 +58,7 @@
 extern int copy_strings_kernel(int argc,char ** argv,struct linux_binprm *bprm);
 extern void compute_creds(struct linux_binprm *binprm);
 extern int do_coredump(long signr, int exit_code, struct pt_regs * regs);
-extern void set_binfmt(struct linux_binfmt *new);
-
-
-#if 0
-/* this went away now */
-#define change_ldt(a,b) setup_arg_pages(a,b)
-#endif
+extern int set_binfmt(struct linux_binfmt *new);
 
 #endif /* __KERNEL__ */
 #endif /* _LINUX_BINFMTS_H */

===== include/linux/compiler.h 1.7 vs edited =====

include/linux/compiler.h +2 -2

@@ -21,9 +21,9 @@
  * and then gcc will emit a warning for each usage of the function.
  */
 #if __GNUC__ >= 3
-#define deprecated	__attribute__((deprecated))
+#define __deprecated	__attribute__((deprecated))
 #else
-#define deprecated
+#define __deprecated
 #endif
 
 /* This macro obfuscates arithmetic on a variable address so that gcc

===== include/linux/fs.h 1.204 vs edited =====

include/linux/fs.h +2 -2

@@ -983,13 +983,13 @@
 /* Alas, no aliases. Too much hassle with bringing module.h everywhere */
 #define fops_get(fops) \
 	(((fops) && (fops)->owner)	\
-		? ( try_inc_mod_count((fops)->owner) ? (fops) : NULL ) \
+		? (try_inc_mod_count((fops)->owner) ? (fops) : NULL) \
 		: (fops))
 
 #define fops_put(fops) \
 do {	\
 	if ((fops) && (fops)->owner) \
-		__MOD_DEC_USE_COUNT((fops)->owner);	\
+		module_put((fops)->owner);	\
 } while(0)
 
 extern int register_filesystem(struct file_system_type *);

===== include/linux/ioport.h 1.5 vs edited =====

include/linux/ioport.h +1 -1

@@ -108,7 +108,7 @@
 #define check_mem_region(start,n)	__check_region(&iomem_resource, (start), (n))
 #define release_mem_region(start,n)	__release_region(&iomem_resource, (start), (n))
 
-extern int deprecated __check_region(struct resource *, unsigned long, unsigned long);
+extern int __deprecated __check_region(struct resource *, unsigned long, unsigned long);
 extern void __release_region(struct resource *, unsigned long, unsigned long);
 
 #define get_ioport_list(buf)	get_resource_list(&ioport_resource, buf, PAGE_SIZE)

===== include/linux/module.h 1.30 vs edited =====

include/linux/module.h +26 -9

@@ -296,9 +296,20 @@
 #define symbol_request(x) try_then_request_module(symbol_get(x), "symbol:" #x)
 
 /* BELOW HERE ALL THESE ARE OBSOLETE AND WILL VANISH */
-#define __MOD_INC_USE_COUNT(mod) \
-	do { __unsafe(mod); (void)try_module_get(mod); } while(0)
-#define __MOD_DEC_USE_COUNT(mod) module_put(mod)
+static inline void __deprecated __MOD_INC_USE_COUNT(struct module *module)
+{
+	__unsafe(module);
+	/*
+	 * Yes, we ignore the retval here, that's why it's deprecated.
+	 */
+	try_module_get(module);
+}
+
+static inline void __deprecated __MOD_DEC_USE_COUNT(struct module *module)
+{
+	module_put(module);
+}
+
 #define SET_MODULE_OWNER(dev) ((dev)->owner = THIS_MODULE)
 
 struct obsolete_modparm {

@@ -319,14 +330,21 @@
 /* People do this inside their init routines, when the module isn't
    "live" yet.  They should no longer be doing that, but
    meanwhile... */
+static inline void __deprecated _MOD_INC_USE_COUNT(struct module *module)
+{
+	__unsafe(module);
+
 #if defined(CONFIG_MODULE_UNLOAD) && defined(MODULE)
-#define MOD_INC_USE_COUNT	\
-	do { __unsafe(THIS_MODULE); local_inc(&THIS_MODULE->ref[get_cpu()].count); put_cpu(); } while (0)
+	local_inc(&module->ref[get_cpu()].count);
+	put_cpu();
 #else
-#define MOD_INC_USE_COUNT \
-	do { __unsafe(THIS_MODULE); (void)try_module_get(THIS_MODULE); } while (0)
+	try_module_get(module);
 #endif
-#define MOD_DEC_USE_COUNT module_put(THIS_MODULE)
+}
+#define MOD_INC_USE_COUNT \
+	_MOD_INC_USE_COUNT(THIS_MODULE)
+#define MOD_DEC_USE_COUNT \
+	__MOD_DEC_USE_COUNT(THIS_MODULE)
 #define try_inc_mod_count(mod) try_module_get(mod)
 #define EXPORT_NO_SYMBOLS
 extern int module_dummy_usage;

@@ -364,5 +382,4 @@
 extern const void *inter_module_get(const char *);
 extern const void *inter_module_get_request(const char *, const char *);
 extern void inter_module_put(const char *);
-
 #endif /* _LINUX_MODULE_H */

===== include/linux/personality.h 1.4 vs edited =====

include/linux/personality.h +0 ~~-18~~

@@ -107,22 +107,4 @@
 #define set_personality(pers) \
 	((current->personality == pers) ? 0 : __set_personality(pers))
 
-/*
- * Load an execution domain.
- */
-#define get_exec_domain(ep)				\
-do {							\
-	if (ep != NULL && ep->module != NULL)		\
-		__MOD_INC_USE_COUNT(ep->module);	\
-} while (0)
-
-/*
- * Unload an execution domain.
- */
-#define put_exec_domain(ep)				\
-do {							\
-	if (ep != NULL && ep->module != NULL)		\
-		__MOD_DEC_USE_COUNT(ep->module);	\
-} while (0)
-
 #endif /* _LINUX_PERSONALITY_H */

===== include/linux/wait.h 1.12 vs edited =====

include/linux/wait.h +5 -4

@@ -11,6 +11,7 @@
 #ifdef __KERNEL__
 
 #include <linux/config.h>
+#include <linux/compiler.h>
 #include <linux/list.h>
 #include <linux/stddef.h>
 #include <linux/spinlock.h>

@@ -244,11 +245,11 @@
  * They are racy.  DO NOT use them, use the wait_event* interfaces above.  
  * We plan to remove these interfaces during 2.7.
  */
-extern void FASTCALL(sleep_on(wait_queue_head_t *q));
-extern long FASTCALL(sleep_on_timeout(wait_queue_head_t *q,
+extern void __deprecated FASTCALL(sleep_on(wait_queue_head_t *q));
+extern long __deprecated FASTCALL(sleep_on_timeout(wait_queue_head_t *q,
 				      signed long timeout));
-extern void FASTCALL(interruptible_sleep_on(wait_queue_head_t *q));
-extern long FASTCALL(interruptible_sleep_on_timeout(wait_queue_head_t *q,
+extern void __deprecated FASTCALL(interruptible_sleep_on(wait_queue_head_t *q));
+extern long __deprecated FASTCALL(interruptible_sleep_on_timeout(wait_queue_head_t *q,
 						    signed long timeout));
 
 /*

kernel/exec_domain.c +2 -5

@@ -172,7 +172,7 @@
 
 		fsp = copy_fs_struct(current->fs);
 		if (fsp == NULL) {
-			put_exec_domain(ep);
+			module_put(ep->module);
 			return -ENOMEM;;
 		}

@@ -194,10 +194,7 @@
 	current_thread_info()->exec_domain = ep;
 	set_fs_altroot();
 
-	put_exec_domain(oep);
-
-	printk(KERN_DEBUG "[%s:%d]: set personality to %lx\n",
-			current->comm, current->pid, personality);
+	module_put(oep->module);
 	return 0;
 }

===== kernel/exit.c 1.76 vs edited =====

kernel/exit.c +3 -3

@@ -664,9 +664,9 @@
 	if (current->leader)
 		disassociate_ctty(1);
 
-	put_exec_domain(tsk->thread_info->exec_domain);
-	if (tsk->binfmt && tsk->binfmt->module)
-		__MOD_DEC_USE_COUNT(tsk->binfmt->module);
+	module_put(tsk->thread_info->exec_domain->module);
+	if (tsk->binfmt)
+		module_put(tsk->binfmt->module);
 
 	tsk->exit_code = code;
 	exit_notify();

===== kernel/fork.c 1.93 vs edited =====

kernel/fork.c +8 -6

@@ -743,10 +743,11 @@
 	if (nr_threads >= max_threads)
 		goto bad_fork_cleanup_count;
 	
-	get_exec_domain(p->thread_info->exec_domain);
+	if (!try_module_get(p->thread_info->exec_domain->module))
+		goto bad_fork_cleanup_count;
 
-	if (p->binfmt && p->binfmt->module)
-		__MOD_INC_USE_COUNT(p->binfmt->module);
+	if (p->binfmt && !try_module_get(p->binfmt->module))
+		goto bad_fork_cleanup_put_domain;
 
 #ifdef CONFIG_PREEMPT
 	/*

@@ -958,9 +959,10 @@
 bad_fork_cleanup:
 	if (p->pid > 0)
 		free_pidmap(p->pid);
-	put_exec_domain(p->thread_info->exec_domain);
-	if (p->binfmt && p->binfmt->module)
-		__MOD_DEC_USE_COUNT(p->binfmt->module);
+	if (p->binfmt)
+		module_put(p->binfmt->module);
+bad_fork_cleanup_put_domain:
+	module_put(p->thread_info->exec_domain->module);
 bad_fork_cleanup_count:
 	atomic_dec(&p->user->processes);
 	free_uid(p->user);

===== kernel/intermodule.c 1.1 vs edited =====

kernel/intermodule.c +1 -1

@@ -166,7 +166,7 @@
 		ime = list_entry(tmp, struct inter_module_entry, list);
 		if (strcmp(ime->im_name, im_name) == 0) {
 			if (ime->owner)
-				__MOD_DEC_USE_COUNT(ime->owner);
+				module_put(ime->owner);
 			spin_unlock(&ime_lock);
 			return;
 		}

===== kernel/resource.c 1.7 vs edited =====

kernel/resource.c +1 -1

@@ -237,7 +237,7 @@
 	return res;
 }
 
-int deprecated __check_region(struct resource *parent, unsigned long start, unsigned long n)
+int __deprecated __check_region(struct resource *parent, unsigned long start, unsigned long n)
 {
 	struct resource * res;

===== kernel/sched.c 1.145 vs edited =====

kernel/sched.c +4 -4

@@ -1238,7 +1238,7 @@
 	__remove_wait_queue(q, &wait);				\
 	spin_unlock_irqrestore(&q->lock, flags);
 
-void interruptible_sleep_on(wait_queue_head_t *q)
+void __deprecated interruptible_sleep_on(wait_queue_head_t *q)
 {
 	SLEEP_ON_VAR

@@ -1249,7 +1249,7 @@
 	SLEEP_ON_TAIL
 }
 
-long interruptible_sleep_on_timeout(wait_queue_head_t *q, long timeout)
+long __deprecated interruptible_sleep_on_timeout(wait_queue_head_t *q, long timeout)
 {
 	SLEEP_ON_VAR

@@ -1262,7 +1262,7 @@
 	return timeout;
 }
 
-void sleep_on(wait_queue_head_t *q)
+void __deprecated sleep_on(wait_queue_head_t *q)
 {
 	SLEEP_ON_VAR

@@ -1273,7 +1273,7 @@
 	SLEEP_ON_TAIL
 }
 
-long sleep_on_timeout(wait_queue_head_t *q, long timeout)
+long __deprecated sleep_on_timeout(wait_queue_head_t *q, long timeout)
 {
 	SLEEP_ON_VAR