Re: PCI: fix an oops in some pci devices on hotplug remove when their resources are being freed.
On Sat, Apr 02, 2005 at 04:53:30AM +0100, Matthew Wilcox wrote: > > So yes, please revert this patch. There is no way it can possibly > affect anything. Agreed. It's now reverted and is queued for Linus to pull from. Thanks for reviewing this. greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: PCI: fix an oops in some pci devices on hotplug remove when their resources are being freed.
On Fri, Apr 01, 2005 at 07:31:41PM -0800, Greg KH wrote: > I agree. However, SGI seems to have some majorly > hardware and drivers that cause this line to release a already released > resource. See the other part of this patch for the part where this > resource is supposedly freed up. That one's even more stupid: +++ b/kernel/resource.c 2005-04-01 15:37:58 -08:00 @@ -505,6 +505,7 @@ *p = res->sibling; write_unlock(&resource_lock); kfree(res); + res = NULL; return; } p = &res->sibling; This pointer is a local variable! Setting it to null right before return cannot possibly affect anything. > I took the patch as it doesn't hurt anyone, and it gets them off of my > back. But if you so much as think this patch isn't needed, I'll gladly > revert it, as I'm really not trusting any PCI hotplug patches coming out > from them anymore... I think the problem is that they have a majorly hacked tree they're working from as well as some quite inexperienced people working on it. They need to do more internal peer review ... and clearly I need to start reviewing their patches more carefully. So yes, please revert this patch. There is no way it can possibly affect anything. -- "Next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank God for the better sleep he enjoys after this process of grotesque self-deception." -- Mark Twain - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: PCI: fix an oops in some pci devices on hotplug remove when their resources are being freed.
On Sat, Apr 02, 2005 at 02:10:23AM +0100, Matthew Wilcox wrote: > On Fri, Apr 01, 2005 at 03:47:50PM -0800, Greg KH wrote: > > PCI: fix an oops in some pci devices on hotplug remove when their resources > > are being freed. > > > > As reported by Prarit Bhargava <[EMAIL PROTECTED]> > > Signed-off-by: Greg Kroah-Hartman <[EMAIL PROTECTED]> > > > > diff -Nru a/drivers/pci/remove.c b/drivers/pci/remove.c > > for (i = 0; i < PCI_NUM_RESOURCES; i++) { > > struct resource *res = dev->resource + i; > > - if (res->parent) > > + if (res && res->parent) > > release_resource(res); > > I can't believe this fixes anything. How can res possibly be NULL here? > It's a pointer into a pci_dev. I agree. However, SGI seems to have some majorly hardware and drivers that cause this line to release a already released resource. See the other part of this patch for the part where this resource is supposedly freed up. I took the patch as it doesn't hurt anyone, and it gets them off of my back. But if you so much as think this patch isn't needed, I'll gladly revert it, as I'm really not trusting any PCI hotplug patches coming out from them anymore... thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: PCI: fix an oops in some pci devices on hotplug remove when their resources are being freed.
On Fri, Apr 01, 2005 at 03:47:50PM -0800, Greg KH wrote: > PCI: fix an oops in some pci devices on hotplug remove when their resources > are being freed. > > As reported by Prarit Bhargava <[EMAIL PROTECTED]> > Signed-off-by: Greg Kroah-Hartman <[EMAIL PROTECTED]> > > diff -Nru a/drivers/pci/remove.c b/drivers/pci/remove.c > for (i = 0; i < PCI_NUM_RESOURCES; i++) { > struct resource *res = dev->resource + i; > - if (res->parent) > + if (res && res->parent) > release_resource(res); I can't believe this fixes anything. How can res possibly be NULL here? It's a pointer into a pci_dev. -- "Next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank God for the better sleep he enjoys after this process of grotesque self-deception." -- Mark Twain - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
PCI: fix an oops in some pci devices on hotplug remove when their resources are being freed.
ChangeSet 1.2181.16.7, 2005/03/17 10:30:46-08:00, [EMAIL PROTECTED] PCI: fix an oops in some pci devices on hotplug remove when their resources are being freed. As reported by Prarit Bhargava <[EMAIL PROTECTED]> Signed-off-by: Greg Kroah-Hartman <[EMAIL PROTECTED]> drivers/pci/remove.c |2 +- kernel/resource.c|1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff -Nru a/drivers/pci/remove.c b/drivers/pci/remove.c --- a/drivers/pci/remove.c 2005-04-01 15:37:58 -08:00 +++ b/drivers/pci/remove.c 2005-04-01 15:37:58 -08:00 @@ -19,7 +19,7 @@ pci_cleanup_rom(dev); for (i = 0; i < PCI_NUM_RESOURCES; i++) { struct resource *res = dev->resource + i; - if (res->parent) + if (res && res->parent) release_resource(res); } } diff -Nru a/kernel/resource.c b/kernel/resource.c --- a/kernel/resource.c 2005-04-01 15:37:58 -08:00 +++ b/kernel/resource.c 2005-04-01 15:37:58 -08:00 @@ -505,6 +505,7 @@ *p = res->sibling; write_unlock(&resource_lock); kfree(res); + res = NULL; return; } p = &res->sibling; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/