Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 5 Jul 2006 14:10:13 GMT
From:      Paolo Pisati <piso@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 100629 for review
Message-ID:  <200607051410.k65EADwj039327@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help 
http://perforce.freebsd.org/chv.cgi?CH=100629

Change 100629 by piso@piso_newluxor on 2006/07/05 14:09:30

	First simplify dll handling:
	-mutex operations where #ifdef out in userland, that means
	 dll_chain manipulation has never been thread safe, so
	 make it more explicit axing all the empy LIBALIAS_LOCK()
	 operations -> this is not a problem cause all the
	 programs using libalias in userland (ppp and natd) aren't
	 multithreaded, anyway it has to be fixed later.
	-put some comments to mark the thread unsafetyness

Affected files ...

.. //depot/projects/soc2005/libalias/sys/netinet/libalias/alias_mod.c#9 edit
.. //depot/projects/soc2005/libalias/sys/netinet/libalias/alias_mod.h#9 edit

Differences ...

==== //depot/projects/soc2005/libalias/sys/netinet/libalias/alias_mod.c#9 (text+ko) ====

@@ -60,7 +60,8 @@
 #include <sys/types.h>
 
 /* protocol and userland module handlers chains */
-struct chain handler_chain, dll_chain;
+struct chain handler_chain;
+struct dll *dll_chain;
 
 #ifdef _KERNEL
 
@@ -254,13 +255,12 @@
 	return (handler_chain.chain);	
 }
 
+/* dll manipulation code - this code is not thread safe... */
+
 static int
-_attach_dll(struct chain *c, struct dll *p) {
-	struct dll **b;
+_attach_dll(struct dll **b, struct dll *p) {
 	int i = 0;
 
-	LIBALIAS_WLOCK_ASSERT(c);
-	b = (struct dll **)&c->chain;
 	p->next = NULL; /* i'm paranoid... */
 	for(; *b != NULL; b = &((*b)->next), i++)
 		if (!strncmp((*b)->name, p->name, DLL_LEN))
@@ -271,12 +271,9 @@
 }
 
 static void *
-_detach_dll(struct chain *c, char *p) {
-	struct dll **b;
+_detach_dll(struct dll **b, char *p) {
 	void *err = NULL;
 
-	LIBALIAS_WLOCK_ASSERT(c);
-	b = (struct dll **)&c->chain;
 	for(; *b != NULL; b = &((*b)->next))
 		if (!strncmp((*b)->name, p, DLL_LEN)) {
 			err = *b;
@@ -290,9 +287,7 @@
 attach_dll(struct dll *p) {
 	int i;
 
-	LIBALIAS_WLOCK(&dll_chain);
 	i = _attach_dll(&dll_chain, p);
-	LIBALIAS_WUNLOCK(&dll_chain);
 	return (i);
 }
 
@@ -300,15 +295,13 @@
 detach_dll(char *p) {
 	void *i;
 
-	LIBALIAS_WLOCK(&dll_chain);
 	i = _detach_dll(&dll_chain, p);
-	LIBALIAS_WUNLOCK(&dll_chain);
 	return (i);
 }
 
 struct dll *
 walk_dll_chain(void) {
-	struct dll *t, **b = (struct dll **)&dll_chain.chain;
+	struct dll *t, **b = &dll_chain;
 
 	for(t = *b; *b != NULL;) {       
 		*b = (*b)->next; 		

==== //depot/projects/soc2005/libalias/sys/netinet/libalias/alias_mod.h#9 (text+ko) ====

@@ -83,9 +83,13 @@
 // XXX - convert it to use queue(3)
 struct chain {
 	void            *chain;	
-#ifdef _KERNEL
 	struct rwlock   rw;
-#endif
+};
+
+struct dll_chain {
+	void            *chain;	
+	// XXX - what if a process with 2 threads manipulate the
+	// libalias dll list? we still need a lock here...
 };
 
 /*

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200607051410.k65EADwj039327>