Hi,<br>I tested it on red hat 5 and it's working.<br><br>Ron<br><br><br><div class="gmail_quote">On Wed, May 14, 2008 at 2:37 PM, Eli Cohen <<a href="mailto:eli@dev.mellanox.co.il">eli@dev.mellanox.co.il</a>> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">IB/ipoib: Fix neigh destructor oops<br>
<br>
For kernels 2.6.20 and older, it may happen that the pointer to<br>
ipoib_neigh_cleanup() is called after IPoIB has been unloades,<br>
causing a kernel oops. This problem has been fixed for 2.6.21 with<br>
the following commit: ecbb416939da77c0d107409976499724baddce7b<br>
<br>
The idea with this patch is to have a helper module which remains<br>
always loaded, and this modules provides the destructor for<br>
neighbours which calls IPoIB's destructor through a function poiner.<br>
When IPoIB is unloaded, the function pointer is cleared so subsequent<br>
calls to a neighbour destructor will be made to valid addresses but<br>
IPoIB's destructor won't get called.<br>
<br>
Signed-off-by: Eli Cohen <<a href="mailto:eli@mellanox.co.il">eli@mellanox.co.il</a>><br>
---<br>
<br>
I know it's not the most elegant way to handle this but since patching<br>
distros' kernels is not relevant this is what I have. I checked this<br>
for 20 hours while originally the failure occurs after half an hour.<br>
Comments?<br>
<br>
<br>
Index: ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib_main.c<br>
===================================================================<br>
--- ofa_1_3_dev_kernel.orig/drivers/infiniband/ulp/ipoib/ipoib_main.c 2008-05-14 12:49:11.000000000 +0300<br>
+++ ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib_main.c 2008-05-14 12:49:32.000000000 +0300<br>
@@ -49,6 +49,7 @@<br>
<br>
#include <net/dst.h><br>
#include <linux/vmalloc.h><br>
+#include <linux/delay.h><br>
<br>
MODULE_AUTHOR("Roland Dreier");<br>
MODULE_DESCRIPTION("IP-over-InfiniBand net driver");<br>
@@ -916,7 +917,7 @@ void ipoib_neigh_free(struct net_device<br>
<br>
static int ipoib_neigh_setup_dev(struct net_device *dev, struct neigh_parms *parms)<br>
{<br>
- parms->neigh_cleanup = ipoib_neigh_cleanup;<br>
+ parms->neigh_cleanup = ipoib_neigh_cleanup_container;<br>
<br>
return 0;<br>
}<br>
@@ -1383,9 +1384,13 @@ static int __init ipoib_init_module(void<br>
ipoib_max_conn_qp = min(ipoib_max_conn_qp, IPOIB_CM_MAX_CONN_QP);<br>
#endif<br>
<br>
+<br>
+ ipoib_set_cleanup_function(ipoib_neigh_cleanup);<br>
ret = ipoib_register_debugfs();<br>
- if (ret)<br>
+ if (ret) {<br>
+ ipoib_set_cleanup_function(NULL);<br>
return ret;<br>
+ }<br>
<br>
/*<br>
* We create our own workqueue mainly because we want to be<br>
@@ -1397,6 +1402,7 @@ static int __init ipoib_init_module(void<br>
*/<br>
ipoib_workqueue = create_singlethread_workqueue("ipoib");<br>
if (!ipoib_workqueue) {<br>
+ ipoib_set_cleanup_function(NULL);<br>
ret = -ENOMEM;<br>
goto err_fs;<br>
}<br>
@@ -1404,8 +1410,10 @@ static int __init ipoib_init_module(void<br>
ib_sa_register_client(&ipoib_sa_client);<br>
<br>
ret = ib_register_client(&ipoib_client);<br>
- if (ret)<br>
+ if (ret) {<br>
+ ipoib_set_cleanup_function(NULL);<br>
goto err_sa;<br>
+ }<br>
<br>
return 0;<br>
<br>
@@ -1421,7 +1429,16 @@ err_fs:<br>
<br>
static void __exit ipoib_cleanup_module(void)<br>
{<br>
+ int ret;<br>
+<br>
ib_unregister_client(&ipoib_client);<br>
+<br>
+ do {<br>
+ ret = ipoib_set_cleanup_function(NULL);<br>
+ if (ret)<br>
+ msleep(10);<br>
+ } while(ret);<br>
+<br>
ib_sa_unregister_client(&ipoib_sa_client);<br>
ipoib_unregister_debugfs();<br>
destroy_workqueue(ipoib_workqueue);<br>
Index: ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/Makefile<br>
===================================================================<br>
--- ofa_1_3_dev_kernel.orig/drivers/infiniband/ulp/ipoib/Makefile 2008-05-14 12:49:11.000000000 +0300<br>
+++ ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/Makefile 2008-05-14 12:49:32.000000000 +0300<br>
@@ -1,4 +1,4 @@<br>
-obj-$(CONFIG_INFINIBAND_IPOIB) += ib_ipoib.o<br>
+obj-$(CONFIG_INFINIBAND_IPOIB) += ib_ipoib.o ipoib_helper.o<br>
<br>
ib_ipoib-y := ipoib_main.o \<br>
ipoib_ib.o \<br>
Index: ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib.h<br>
===================================================================<br>
--- ofa_1_3_dev_kernel.orig/drivers/infiniband/ulp/ipoib/ipoib.h 2008-05-14 12:49:11.000000000 +0300<br>
+++ ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib.h 2008-05-14 12:49:32.000000000 +0300<br>
@@ -554,6 +554,9 @@ int ipoib_mcast_stop_thread(struct net_d<br>
void ipoib_mcast_dev_down(struct net_device *dev);<br>
void ipoib_mcast_dev_flush(struct net_device *dev);<br>
<br>
+int ipoib_set_cleanup_function(void (*func)(struct neighbour *n));<br>
+void ipoib_neigh_cleanup_container(struct neighbour *n);<br>
+<br>
#ifdef CONFIG_INFINIBAND_IPOIB_DEBUG<br>
struct ipoib_mcast_iter *ipoib_mcast_iter_init(struct net_device *dev);<br>
int ipoib_mcast_iter_next(struct ipoib_mcast_iter *iter);<br>
Index: ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib_helper.c<br>
===================================================================<br>
--- /dev/null 1970-01-01 00:00:00.000000000 +0000<br>
+++ ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib_helper.c 2008-05-14 12:49:32.000000000 +0300<br>
@@ -0,0 +1,63 @@<br>
+#include <linux/kernel.h><br>
+#include <linux/module.h><br>
+#include <net/neighbour.h><br>
+<br>
+MODULE_AUTHOR("Eli Cohen");<br>
+MODULE_DESCRIPTION("container for ipoib neighbour destructor");<br>
+MODULE_LICENSE("Dual BSD/GPL");<br>
+<br>
+DEFINE_SPINLOCK(spl);<br>
+static int busy;<br>
+<br>
+static void (*cleanup_func)(struct neighbour *n);<br>
+<br>
+static int ipoib_set_cleanup_function(void (*func)(struct neighbour *n))<br>
+{<br>
+ unsigned long flags;<br>
+<br>
+ spin_lock_irqsave(&spl, flags);<br>
+ if (busy) {<br>
+ spin_unlock_irqrestore(&spl, flags);<br>
+ return -EBUSY;<br>
+ }<br>
+ cleanup_func = func;<br>
+ spin_unlock_irqrestore(&spl, flags);<br>
+<br>
+ return 0;<br>
+}<br>
+<br>
+static void ipoib_neigh_cleanup_container(struct neighbour *n)<br>
+{<br>
+ unsigned long flags;<br>
+<br>
+ spin_lock_irqsave(&spl, flags);<br>
+ busy = 1;<br>
+ spin_unlock_irqrestore(&spl, flags);<br>
+ if (cleanup_func)<br>
+ cleanup_func(n);<br>
+<br>
+ spin_lock_irqsave(&spl, flags);<br>
+ busy = 0;<br>
+ spin_unlock_irqrestore(&spl, flags);<br>
+}<br>
+<br>
+<br>
+EXPORT_SYMBOL(ipoib_set_cleanup_function);<br>
+EXPORT_SYMBOL(ipoib_neigh_cleanup_container);<br>
+<br>
+<br>
+static int __init ipoib_helper_init(void)<br>
+{<br>
+ if (!try_module_get(THIS_MODULE))<br>
+ return -1;<br>
+<br>
+ return 0;<br>
+}<br>
+<br>
+<br>
+static void __exit ipoib_helper_cleanup(void)<br>
+{<br>
+}<br>
+<br>
+module_init(ipoib_helper_init);<br>
+module_exit(ipoib_helper_cleanup);<br>
<br>
<br>
_______________________________________________<br>
ewg mailing list<br>
<a href="mailto:ewg@lists.openfabrics.org">ewg@lists.openfabrics.org</a><br>
<a href="http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg" target="_blank">http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg</a><br>
</blockquote></div><br>