[openfabrics-ewg] Backport and fix patches for ipath driver

[Michael S. Tsirkin]

> Quoting  Bryan O'Sullivan <bos at pathscale.com>:
> Subject: Backport and fix patches for ipath driver
> 
> Hi, Vlad and Tziporet -
> 
> Here's a round of fix and backport patches for the ipath driver, for 
> dropping into the OFED 1.2 tree.  The way in which they're organised 
> should, I hope, be clear.

Looks good, fixes look much cleaner than what we had for OFED 1.1.
I think fixes can be applied already.
However, I'm not sure the backports are ready to be applied as is yet.

Just taking a look at random:

./backport/2.6.18/ipath-50-mad-kmem_cache-2.6.19.patch
BACKPORT - kmem_cache_t disappeared after 2.6.19
diff -r a290ff6e9ae7 drivers/infiniband/core/mad.c
--- a/drivers/infiniband/core/mad.c     Wed Jan 31 14:47:02 2007 -0800
+++ b/drivers/infiniband/core/mad.c     Wed Jan 31 14:48:00 2007 -0800
@@ -46,7 +46,7 @@ MODULE_AUTHOR("Hal Rosenstock");
 MODULE_AUTHOR("Hal Rosenstock");
 MODULE_AUTHOR("Sean Hefty");

-static struct kmem_cache *ib_mad_cache;
+static kmem_cache_t *ib_mad_cache;

 static struct list_head ib_mad_port_list;
 static u32 ib_mad_client_id = 0;

This changes a core file, and does not seem to be related to ipath at all.
What problem does this solve?  I note that mad.c already seems to build fine on 2.6.18
for us - this is part of daily build.

Another example that looks strange:

BACKPORT - workqueues changed in 2.6.20

diff -r 8b94fcef1edd drivers/infiniband/hw/ipath/ipath_driver.c
--- a/drivers/infiniband/hw/ipath/ipath_driver.c        Thu Feb 01 08:54:29 2007 -0800
+++ b/drivers/infiniband/hw/ipath/ipath_driver.c        Thu Feb 01 08:57:19 2007 -0800
@@ -241,7 +241,7 @@ static struct ipath_devdata *ipath_alloc
        dd->pcidev = pdev;
        pci_set_drvdata(pdev, dd);

-       INIT_DELAYED_WORK(&dd->link_work, check_link_status);
+       INIT_WORK(&dd->link_work, check_link_status);

        list_add(&dd->ipath_list, &ipath_dev_list);


INIT_DELAYED_WORK is implemented in kernel_addons, so this should
not be necessary.

@@ -725,6 +725,7 @@ static void __devexit ipath_remove_one(s
         */
        ipath_shutdown_device(dd);

+#undef cancel_delayed_work
        cancel_delayed_work(&dd->link_work);
        flush_scheduled_work();

This undef looks quite ugly. What does it do?

Please go over the backport patches and check whether they are really necessary.
I think you will mostly discover that the kernel_addons mechanism makes
the backport patches unnecessary. If not, you should try adding
things under kernel_addons as first choice so that everyone benefits.


-- 
MST