[ofa-general] RFC: modify upstream code to make backporting easier

[Michael S. Tsirkin]

Roland, Ralph, all,
I'd like to get your opinion on the following matter:
OFED is backporting upstream rdma code to older kernels.
While doing so, I really take pains to keep the ported
code as close as possible to upstream original,
mostly by using preprocessor to implement, as closely
as possible, the APIs from recent kernels on top of
older ones.

As an example where this works well, see my backport of the
new workqueue API to 2.6.19:
http://www.openfabrics.org/git/?p=ofed_1_3/linux-2.6.git;a=blob;f=kernel_addons/backport/2.6.19/include/linux/workqueue.h;hb=HEAD

However, sometimes I am forced to patch the upstream code. Here's an
example of the patch needed to make ipath build on
2.6.22:


diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c b/drivers/infiniband/hw/ipath/ipath_driver.c
index 09c5fd8..94edb5d 100644
--- a/drivers/infiniband/hw/ipath/ipath_driver.c
+++ b/drivers/infiniband/hw/ipath/ipath_driver.c
@@ -287,6 +287,7 @@ static int __devinit ipath_init_one(struct pci_dev *pdev,
 	struct ipath_devdata *dd;
 	unsigned long long addr;
 	u32 bar0 = 0, bar1 = 0;
+	u8 rev;
 
 	dd = ipath_alloc_devdata(pdev);
 	if (IS_ERR(dd)) {
@@ -448,7 +449,13 @@ static int __devinit ipath_init_one(struct pci_dev *pdev,
 	dd->ipath_deviceid = ent->device;	/* save for later use */
 	dd->ipath_vendorid = ent->vendor;
 
-	dd->ipath_pcirev = pdev->revision;
+	ret = pci_read_config_byte(pdev, PCI_REVISION_ID, &rev);
+	if (ret) {
+		ipath_dev_err(dd, "Failed to read PCI revision ID unit "
+			      "%u: err %d\n", dd->ipath_unit, -ret);
+		goto bail_regions;	/* shouldn't ever happen */
+	}
+	dd->ipath_pcirev = rev;
 
 #if defined(__powerpc__)
 	/* There isn't a generic way to specify writethrough mappings */


As you can see, there's nothing I can do with macros outside the code
to make it work without code changes.
However, the patching mechanism is pretty fragile with respect
to code reorgs etc.
I wonder whether it's acceptable in cases such as this to add
a wrapper in upstream code. For example, upstream could have:

#ifndef pci_get_revision
#define pci_get_revision(dev) ((dev)->revision)
#endif

and then all a 2.6.22 backport needs to do is define it's own
pci_get_revision macro.

Upstream maintainers, can you pls comment ASAP on whether such approach would be
acceptable e.g. for 2.6.24?  If I could get rid of backport
patches, it might make sense to start thinking about converting fixes
patches to git commits, post 1.3, as well.

Thanks,

-- 
MST