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