kaos@ocs.com.au said:
> struct module *address_to_module(unsigned long addr) {
> struct module *mod;
> for (mod = module_list; mod; mod = mod->next) {
> if (MOD_CAN_QUERY(mod) &&
> addr >= (unsigned long)mod &&
> addr < (unsigned long)mod + mod->size)
> return(mod);
> }
> return(NULL); }
Yeah - I actually thought about using that for put_module_symbol but wasn't
sure enough that it would work across all architectures.
kaos@ocs.com.au said:
> Two problems with having get_module_symbol() increase the use count:
> (1) The caller has to do put_module_symbol() for every symbol they
> looked up. Easy to miss one, and it makes it awkward to do things
> like this
> #define NS8390_KSYSMS_PRESENT ( \
> get_module_symbol(NULL, "ethdev_init") != 0 && \
> get_module_symbol(NULL, "NS8390_init") != 0 && \
> get_module_symbol(NULL, "ei_open") != 0 && \
> get_module_symbol(NULL, "ei_close") != 0 && \
> get_module_symbol(NULL, "ei_interrupt") != 0)
Don't do that then.
> If the fifth test fails, you have to do put_module_symbol() on the
> four that worked to release the module. Try coding that as a
> single statement and handling all the possible cases - messy.
It's not that bad.
Index: drivers/net/8390.h
===================================================================
RCS file: /cvs/linux/drivers/net/8390.h,v
retrieving revision 1.1.1.5.2.1
diff -u -w -r1.1.1.5.2.1 8390.h
--- drivers/net/8390.h 1999/10/07 14:51:58 1.1.1.5.2.1
+++ drivers/net/8390.h 2000/05/09 13:27:33
@@ -53,24 +53,23 @@
#if defined(LOAD_8390_BY_KMOD) && defined(MODULE) && !defined(NS8390_CORE)
/* Function pointers to be mapped onto the 8390 core support */
-static int (*S_ethdev_init)(struct net_device *dev);
-static void (*S_NS8390_init)(struct net_device *dev, int startp);
-static int (*S_ei_open)(struct net_device *dev);
-static int (*S_ei_close)(struct net_device *dev);
-static void (*S_ei_interrupt)(int irq, void *dev_id, struct pt_regs *regs);
+static int (*S_ethdev_init)(struct net_device *dev) = NULL;
+static void (*S_NS8390_init)(struct net_device *dev, int startp) = NULL;
+static int (*S_ei_open)(struct net_device *dev) = NULL;
+static int (*S_ei_close)(struct net_device *dev) = NULL;
+static void (*S_ei_interrupt)(int irq, void *dev_id, struct pt_regs *regs) = NULL;
-
-#define NS8390_KSYSMS_PRESENT ( \
- get_module_symbol(NULL, "ethdev_init") != 0 && \
- get_module_symbol(NULL, "NS8390_init") != 0 && \
- get_module_symbol(NULL, "ei_open") != 0 && \
- get_module_symbol(NULL, "ei_close") != 0 && \
- get_module_symbol(NULL, "ei_interrupt") != 0)
-
extern __inline__ int load_8390_module(const char *driver)
{
+ /* Do we need to handle this case? */
+ if (S_ethdev_init) {
+ printk(KERN_DEBUG "%s: load_8390_module called when pointers already present\n");
+ return 0;
+ }
+
+ S_ethdev_init = get_module_symbol(NULL, "ethdev_init");
- if (! NS8390_KSYSMS_PRESENT) {
+ if (!S_ethdev_init) {
int (*request_mod)(const char *module_name);
if (get_module_symbol("", "request_module") == 0) {
@@ -84,48 +83,45 @@
if (request_mod("8390")) {
printk("%s: request to load the 8390 module failed.\n", driver);
return -ENOSYS;
- }
-
- /* Check if module really loaded and is valid */
- if (! NS8390_KSYSMS_PRESENT) {
- printk("%s: 8390.o not found/invalid or failed to load.\n", driver);
- return -ENOSYS;
}
-
- printk(KERN_INFO "%s: auto-loaded 8390 module.\n", driver);
+ S_ethdev_init = get_module_symbol(NULL, "ethdev_init");
}
/* Map the functions into place */
- S_ethdev_init = (void*)get_module_symbol(0, "ethdev_init");
S_NS8390_init = (void*)get_module_symbol(0, "NS8390_init");
S_ei_open = (void*)get_module_symbol(0, "ei_open");
S_ei_close = (void*)get_module_symbol(0, "ei_close");
S_ei_interrupt = (void*)get_module_symbol(0, "ei_interrupt");
- return 0;
+ /* Check if module really loaded and is valid */
+ if (!S_ethdev_init || !S_NS8390_init || !S_ei_open || !S_ei_close
+ || !S_ei_interrupt) {
+ if (S_ethdev_init) {
+ put_module_symbol(S_ethdev_init);
+ S_ethdev_init = NULL;
+ }
+ if (S_NS8390_init) {
+ put_module_symbol(S_NS8390_init);
+ S_NS8390_init = NULL;
+ }
+ if (S_ei_open) {
+ put_module_symbol(S_ei_open);
+ S_ei_open = NULL;
+ }
+ if (S_ei_close) {
+ put_module_symbol(S_ei_close);
+ S_ei_close = NULL;
+ }
+ if (S_ei_interrupt) {
+ put_module_symbol(S_ei_interrupt);
+ S_ei_interrupt = NULL;
}
-
-/*
- * Since a kmod aware driver won't explicitly show a dependence on the
- * exported 8390 functions (due to the mapping above), the 8390 module
- * (if present, and not in-kernel) needs to be protected from garbage
- * collection. NS8390_module is only defined for a modular 8390 core.
- */
-
-extern __inline__ void lock_8390_module(void)
-{
- struct module **mod = (struct module**)get_module_symbol(0, "NS8390_module");
-
- if (mod != NULL && *mod != NULL)
- __MOD_INC_USE_COUNT(*mod);
+ printk("%s: 8390.o not found/invalid or failed to load.\n", driver);
+ return -ENOSYS;
}
-extern __inline__ void unlock_8390_module(void)
-{
- struct module **mod = (struct module**)get_module_symbol(0, "NS8390_module");
-
- if (mod != NULL && *mod != NULL)
- __MOD_DEC_USE_COUNT(*mod);
+ printk(KERN_INFO "%s: auto-loaded 8390 module.\n", driver);
+ return 0;}
}
/*
> (2) More importantly, it differs from most of the other code that
> increases module use counts. Most uses of MOD_INC_USE are in the
> modules proper, not in kernel/module.c.
The current {un,}lock_8390_module also differs from that. Not a problem.
-- dwmw2- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Mon May 15 2000 - 21:00:13 EST