Skip to contentSkip to conversation

pci: don't override drv->shutdown unconditionally

PATCH In review Christoph Hellwig 2 participants 6 messages Jun 17, 2005 PCI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 pci: don't override drv->shutdown unconditionally

There are many drivers that have been setting the generic driver model�level shutdown callback, and pci thus must not override it.

Without this patch we can have really bad data loss on various raid controllers.

drivers/pci/pci-driver.c +2 -1
Index: linux-2.6/drivers/pci/pci-driver.c
===================================================================
@@ -393,7 +393,8 @@
 	drv->driver.bus = &pci_bus_type;
 	drv->driver.probe = pci_device_probe;
 	drv->driver.remove = pci_device_remove;
-	drv->driver.shutdown = pci_device_shutdown,
+	if (!drv->driver.shutdown)
+		drv->driver.shutdown = pci_device_shutdown,
 	drv->driver.owner = drv->owner;
 	drv->driver.kobj.ktype = &pci_driver_kobj_type;
 	pci_init_dynids(&drv->dynids);
#1 lore raw
Quoted 5 lines · Christoph Hellwig

There are many drivers that have been setting the generic driver model�level shutdown callback, and pci thus must not override it.

Without this patch we can have really bad data loss on various raid controllers.

Without the kexec patch?

So, why are these drivers setting the shutdown function in the first place if they don't want it to be called? My change finally enabled this call, which is what the driver authors expected in the first place.

Without the change I made, the same data loss would be had as drivers never know that the box is going down.

The fact that a few drivers never tested their shutdown calls is no reason to penalize the whole kernel.

So, no, I do not want this change in, the drivers should be fixed properly.

Care to point me to any drivers that need fixing?

thanks,

greg k-h

#2 lore raw
Quoted 8 lines · Greg KH
Quoted 5 lines · Christoph Hellwig

There are many drivers that have been setting the generic driver model�level shutdown callback, and pci thus must not override it.

Without this patch we can have really bad data loss on various raid controllers.

Without the kexec patch?

On shutdown. I don't know why you're talking about kexec here.

Quoted 2 lines

So, why are these drivers setting the shutdown function in the first place if they don't want it to be called?

They _do_ want it called. They set the driver-model level one because there hasn't been a pci-level one until a few years ago.

#3 lore raw
Quoted 11 lines · Christoph Hellwig
Quoted 8 lines · Greg KH
Quoted 5 lines · Christoph Hellwig

There are many drivers that have been setting the generic driver model�level shutdown callback, and pci thus must not override it.

Without this patch we can have really bad data loss on various raid controllers.

Without the kexec patch?

On shutdown. I don't know why you're talking about kexec here.

Because of the previous kexec comments on the linux-scsi list.

Quoted 5 lines
Quoted 2 lines

So, why are these drivers setting the shutdown function in the first place if they don't want it to be called?

They _do_ want it called. They set the driver-model level one because there hasn't been a pci-level one until a few years ago.

So they are setting two callbacks? Have a pointer to any driver that does this?

thanks,

greg k-h

#4 lore raw
Quoted 6 lines · Greg KH
Quoted 2 lines · Christoph Hellwig

They _do_ want it called. They set the driver-model level one because there hasn't been a pci-level one until a few years ago.

So they are setting two callbacks? Have a pointer to any driver that does this?

Ok, I see the drivers that do this now, they should be fixed. I'll make up a patch now to do that.

thanks,

greg k-h

#5 lore raw
Quoted 10 lines · Greg KH
Quoted 6 lines · Greg KH
Quoted 2 lines · Christoph Hellwig

They _do_ want it called. They set the driver-model level one because there hasn't been a pci-level one until a few years ago.

So they are setting two callbacks? Have a pointer to any driver that does this?

Ok, I see the drivers that do this now, they should be fixed. I'll make up a patch now to do that.

Hm, that's too big of a patch for 2.6.12 this late in the game, I've forwarded your patch on to Linus with an added comment. I'll fix up the drivers for 2.6.13 and then change the pci core back.

Sorry for the confusion, you were right.

thanks,

greg k-h