Subject: LKM diff for review
To: None <tech-kern@NetBSD.org>
From: Peter Postma <peter@pointless.nl>
List: tech-kern
Date: 10/25/2004 12:01:14
I'm working on cleaning up the LKM code, here's my first diff for review:
changes the static array to a linked list. I've been running with this diff
for a while now and had no problems with LKM's so far.
Most of the code comes from OpenBSD and there's probably room for
improvement..
Index: kern/kern_lkm.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_lkm.c,v
retrieving revision 1.75
diff -u -r1.75 kern_lkm.c
--- kern/kern_lkm.c 22 Oct 2004 09:49:18 -0000 1.75
+++ kern/kern_lkm.c 23 Oct 2004 19:50:12 -0000
@@ -98,13 +98,12 @@
static int lkm_v = 0;
static int lkm_state = LKMS_IDLE;
-#ifndef MAXLKMS
-#define MAXLKMS 20
-#endif
-
-static struct lkm_table lkmods[MAXLKMS]; /* table of loaded modules */
+static TAILQ_HEAD(lkms_head, lkm_table) lkmods; /* table of loaded modules */
static struct lkm_table *curp; /* global for in-progress ops */
+static struct lkm_table *lkmlookup(int, char *, int *);
+static struct lkm_table *lkmalloc(void);
+static void lkmfree(void);
static void lkmunreserve(void);
static int _lkm_syscall(struct lkm_table *, int);
static int _lkm_vfs(struct lkm_table *, int);
@@ -136,6 +135,8 @@
*/
if (lkm_map == NULL)
lkm_map = kernel_map;
+
+ TAILQ_INIT(&lkmods);
}
/*ARGSUSED*/
@@ -173,12 +174,101 @@
}
/*
+ * Look up for a LKM in the list.
+ */
+static struct lkm_table *
+lkmlookup(int i, char *name, int *error)
+{
+ struct lkm_table *p;
+ char istr[MAXLKMNAME];
+
+ /*
+ * p being NULL here implies the list is empty, so any lookup is
+ * invalid (name based or otherwise). Since the list of modules is
+ * kept sorted by id, lowest to highest, the id of the last entry
+ * will be the highest in use.
+ */
+ p = TAILQ_LAST(&lkmods, lkms_head);
+ if (p == NULL || i > p->id) {
+ *error = EINVAL;
+ return (NULL);
+ }
+
+ if (i < 0) { /* unload by name */
+ /*
+ * Copy name and lookup id from all loaded
+ * modules. May fail.
+ */
+ *error = copyinstr(name, istr, MAXLKMNAME - 1, NULL);
+ if (*error)
+ return (NULL);
+ istr[MAXLKMNAME - 1] = '\0';
+
+ TAILQ_FOREACH(p, &lkmods, link) {
+ if (strcmp(istr, p->private.lkm_any->lkm_name) == 0)
+ break;
+ }
+ } else
+ TAILQ_FOREACH(p, &lkmods, link)
+ if (i == p->id)
+ break;
+
+ if (p == NULL)
+ *error = ENOENT;
+
+ return (p);
+}
+
+/*
+ * Allocates memory for a new LKM table entry and inserts in the list.
+ * Returns NULL on failure.
+ */
+static struct lkm_table *
+lkmalloc(void)
+{
+ struct lkm_table *p, *ret;
+ int id = 0;
+
+ ret = malloc(sizeof(struct lkm_table), M_DEVBUF, M_NOWAIT);
+ if (ret == NULL)
+ return (NULL);
+ ret->refcnt = 0;
+ ret->forced = 0;
+
+ /* find the first unused id */
+ TAILQ_FOREACH(p, &lkmods, link) {
+ if (id != p->id)
+ break;
+ id++;
+ }
+ ret->id = id;
+
+ if (p == NULL)
+ TAILQ_INSERT_TAIL(&lkmods, ret, link);
+ else
+ TAILQ_INSERT_BEFORE(p, ret, link);
+
+ return (ret);
+}
+
+/*
+ * Frees the current LKM table entry.
+ */
+static void
+lkmfree(void)
+{
+ TAILQ_REMOVE(&lkmods, curp, link);
+ free(curp, M_DEVBUF);
+ curp = NULL;
+}
+
+/*
* Unreserve the memory associated with the current loaded module; done on
* a coerced close of the lkm device (close on premature exit of modload)
* or explicitly by modload as a result of a link failure.
*/
static void
-lkmunreserve()
+lkmunreserve(void)
{
if (lkm_state == LKMS_IDLE)
@@ -220,13 +310,14 @@
}
/* do this before waking the herd... */
- if (curp && !curp->used) {
+ if (curp != NULL && curp->refcnt == 0) {
/*
* If we close before setting used, we have aborted
* by way of error or by way of close-on-exit from
* a premature exit of "modload".
*/
lkmunreserve(); /* coerce state to LKM_IDLE */
+ lkmfree();
}
lkm_v &= ~LKM_ALLOC;
@@ -244,13 +335,11 @@
int flag;
struct proc *p;
{
- int error = 0;
- int i;
+ int i, error = 0;
struct lmc_resrv *resrvp;
struct lmc_loadbuf *loadbufp;
struct lmc_unload *unloadp;
struct lmc_stat *statp;
- char istr[MAXLKMNAME];
switch(cmd) {
case LMRESERV: /* reserve pages for a module */
@@ -262,30 +351,21 @@
resrvp = (struct lmc_resrv *)data;
- /*
- * Find a free slot.
- */
- for (i = 0; i < MAXLKMS; i++)
- if (!lkmods[i].used)
- break;
- if (i == MAXLKMS) {
- error = ENOMEM; /* no slots available */
+ curp = lkmalloc();
+ if (curp == NULL) {
+ error = ENOMEM;
break;
}
- curp = &lkmods[i];
-
- resrvp->slot = i; /* return slot */
+ resrvp->slot = curp->id; /* return slot */
/*
* Get memory for module
*/
curp->size = resrvp->size;
-
curp->area = LKM_SPACE_ALLOC(curp->size);
-
curp->offset = 0; /* load offset */
- resrvp->addr = curp->area; /* ret kernel addr */
+ resrvp->addr = curp->area; /* ret kernel addr */
if (cmd == LMRESERV && resrvp->sym_size) {
curp->sym_size = resrvp->sym_size;
@@ -387,6 +467,8 @@
return EPERM;
lkmunreserve(); /* coerce state to LKM_IDLE */
+ if (curp != NULL)
+ lkmfree();
#ifdef DEBUG
if (lkmdebug & LKMDB_INFO)
printf("LKM: LMUNRESERV\n");
@@ -445,7 +527,7 @@
*/
lkm_state = LKMS_UNLOADING; /* for lkmunreserve */
lkmunreserve(); /* free memory */
- curp->used = 0; /* free slot */
+ lkmfree(); /* free slot */
#ifdef DEBUG
if (lkmdebug & LKMDB_INFO)
printf("lkm entry point failed with error %d\n",
@@ -453,8 +535,8 @@
#endif /* DEBUG */
break;
}
+ curp->refcnt++;
- curp->used = 1;
#ifdef DEBUG
if (lkmdebug & LKMDB_INFO)
printf("LKM: LMREADY\n");
@@ -471,42 +553,9 @@
unloadp = (struct lmc_unload *)data;
- if ((i = unloadp->id) == -1) { /* unload by name */
- /*
- * Copy name and lookup id from all loaded
- * modules. May fail.
- */
- error = copyinstr(unloadp->name, istr, MAXLKMNAME-1,
- NULL);
- if (error)
- break;
-
- /*
- * look up id...
- */
- for (i = 0; i < MAXLKMS; i++) {
- if (!lkmods[i].used)
- continue;
- if (!strcmp(istr,
- lkmods[i].private.lkm_any->lkm_name))
- break;
- }
- }
-
- /*
- * Range check the value; on failure, return EINVAL
- */
- if (i < 0 || i >= MAXLKMS) {
- error = EINVAL;
+ curp = lkmlookup(unloadp->id, unloadp->name, &error);
+ if (curp == NULL)
break;
- }
-
- curp = &lkmods[i];
-
- if (!curp->used) {
- error = ENOENT;
- break;
- }
/* call entry(unload) */
if ((*(curp->entry))(curp, LKM_E_UNLOAD, LKM_VERSION)) {
@@ -516,7 +565,7 @@
lkm_state = LKMS_UNLOADING; /* non-idle for lkmunreserve */
lkmunreserve(); /* free memory */
- curp->used = 0; /* free slot */
+ lkmfree(); /* free slot */
break;
case LMSTAT: /* stat a module by id/name */
@@ -524,43 +573,8 @@
statp = (struct lmc_stat *)data;
- if ((i = statp->id) == -1) { /* stat by name */
- /*
- * Copy name and lookup id from all loaded
- * modules.
- */
- copystr(statp->name, istr, MAXLKMNAME-1, (size_t *)0);
- /*
- * look up id...
- */
- for (i = 0; i < MAXLKMS; i++) {
- if (!lkmods[i].used)
- continue;
- if (!strcmp(istr,
- lkmods[i].private.lkm_any->lkm_name))
- break;
- }
-
- if (i == MAXLKMS) { /* Not found */
- error = ENOENT;
- break;
- }
- }
-
- /*
- * Range check the value; on failure, return EINVAL
- */
- if (i < 0 || i >= MAXLKMS) {
- error = EINVAL;
- break;
- }
-
- curp = &lkmods[i];
-
- if (!curp->used) { /* Not found */
- error = ENOENT;
+ if ((curp = lkmlookup(statp->id, statp->name, &error)) == NULL)
break;
- }
if ((error = (*curp->entry)(curp, LKM_E_STAT, LKM_VERSION)))
break;
@@ -568,7 +582,7 @@
/*
* Copy out stat information for this module...
*/
- statp->id = i;
+ statp->id = curp->id;
statp->offset = curp->private.lkm_any->lkm_offset;
statp->type = curp->private.lkm_any->lkm_type;
statp->area = curp->area;
@@ -640,20 +654,12 @@
lkmexists(lkmtp)
struct lkm_table *lkmtp;
{
- int i;
+ struct lkm_table *p;
- /*
- * see if name exists...
- */
- for (i = 0; i < MAXLKMS; i++) {
- /*
- * An unused module and the one we are testing are not
- * considered.
- */
- if (!lkmods[i].used || &lkmods[i] == lkmtp)
- continue;
- if (!strcmp(lkmtp->private.lkm_any->lkm_name,
- lkmods[i].private.lkm_any->lkm_name))
+ /* see if name exists... */
+ TAILQ_FOREACH(p, &lkmods, link) {
+ if (strcmp(lkmtp->private.lkm_any->lkm_name,
+ p->private.lkm_any->lkm_name) == 0 && (p->refcnt != 0))
return (1); /* already loaded... */
}
Index: sys/lkm.h
===================================================================
RCS file: /cvsroot/src/sys/sys/lkm.h,v
retrieving revision 1.33
diff -u -r1.33 lkm.h
--- sys/lkm.h 17 Aug 2004 22:38:50 -0000 1.33
+++ sys/lkm.h 23 Oct 2004 19:50:12 -0000
@@ -40,6 +40,8 @@
#ifndef _SYS_LKM_H_
#define _SYS_LKM_H_
+#include <sys/queue.h>
+
/*
* Supported module types
*/
@@ -177,7 +179,8 @@
* Per module information structure
*/
struct lkm_table {
- char used;
+ int id; /* Identifier */
+ char refcnt; /* Reference count */
char forced; /* Forced load, skipping compatibility check */
int (*entry) __P((struct lkm_table *, int, int));/* entry function */
@@ -192,6 +195,8 @@
u_long sym_size; /* size of symbol table (syms+strings) */
u_long sym_offset; /* offset of next symbol chunk */
u_long sym_symsize; /* size of symbol part only */
+
+ TAILQ_ENTRY(lkm_table) link;
};
--
Peter Postma