Re: PCI: fix an oops in some pci devices on hotplug remove when their resources are being freed.

2005-04-01 Thread Greg KH
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.

2005-04-01 Thread Matthew Wilcox
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.

2005-04-01 Thread Greg KH
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.

2005-04-01 Thread Matthew Wilcox
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.

2005-04-01 Thread Greg KH
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/