Latest bk kernel does not properly free PCI IO & MEM allocations

Hello all,

I noticed this in testing of a hotplug driver I've written for SGI Altix hardware.

If you cat /proc/iomem and check lspci after removing a device via hotplug:

[root@cranberry3 linux-2.6.9-hotplug]# lspci
01:01.0 Co-processor: Silicon Graphics, Inc. IOC4 I/O controller (rev 4f)
01:03.0 SCSI storage controller: QLogic Corp. ISP12160 Dual Channel Ultra3 SCSI Processor (rev 06) 01:04.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5701 Gigabit Ethernet (rev 15) 03:02.0 ATM network controller: FORE Systems Inc ForeRunnerHE ATM Adapter
04:01.0 SCSI storage controller: QLogic Corp. QLA2200 64-bit Fibre Channel Adapter (rev 05) [root@cranberry3 linux-2.6.9-hotplug]# cat /proc/iomem c00004080c200000-c00004080c2fffff : 0000:01:01.0
c00004080f400000-c00004080f4fffff : 0000:03:02.0
c00004080fe00000-c00004080fe00fff : 0000:04:01.0
c00004080fe01000-c00004080fe01fff : 0000:04:02.0
00220000-0023ffff : 0000:04:01.0

There are still allocations listed for device 04:02.0 even though it has been removed from the system.

I tracked down the issue to the following:

When pci_remove_bus_device is called on the device in the slot (should be a safe thing to do) both the pci_driver remove function for a device and the function pci_free_resources are called.

In the case of the QLA2x00 driver, the driver's remove function calls pci_release_regions. This function releases the IO & MEM allocs for the pci device and kfree's them.

As previously mentioned, pci_free_resources is called. This function attempts to use and free IO & MEM allocs for the pci device.

This may (about 5% of the time) causes an oops, ie) this is a critical issue for anyone using hotplug.

This raises the following question -- should a driver be cleaning up resources or should PCI?

P.

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id396&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

The following patch reproduces the oops 100% of the time by doing the following.

Boot machine.
Load Hotplug driver (if not already loaded)
Disable Slot that has MEM and IO allocations
cat /proc/iomem <--- leads to oops.

P.

@@ -495,20 +495,21 @@
 			break;
 		if (res->start <= start && res->end >= end) {
 			if (!(res->flags & IORESOURCE_BUSY)) {
 				p = &res->child;
 				continue;
 			}
 			if (res->start != start || res->end != end)
 				break;
 			*p = res->sibling;
 			write_unlock(&resource_lock);
+			memset(res, 0, sizeof(*res));
 			kfree(res);
 			return;
 		}
 		p = &res->sibling;
 	}
 
 	write_unlock(&resource_lock);
 
 	printk(KERN_WARNING "Trying to free nonexistent resource <%08lx-%08lx>\n", start, end);
 }

Where does it attempt to use it? I see a loop to free the resources, not anything else.

Also, if you enable slab debugging, does this always occur?

And, this should be brought up on the linux-pci mailing list, not the hotplug list.

thanks,

greg k-h

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id396&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Sorry -- I confused you. I meant "use and then free" in that it uses kfree'd memory and then attempts to free the resources.

Haven't tried that ... I'll give it a shot and report back to the list.

I'll defer to your judgement on that :) Given the code paths above it seemed to be a hotplug issue to me :)

Thanks for your reply -- I'll turn on slab debug and see what happens...

P.

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id396&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Sorry Greg -- still waiting on linux-pci subscription ... :(

Anyway just to continue the discussion -- here's a run with SLAB debug on. The oops is now 100% reproducible.

[root@altix3 ~]# lspci
01:01.0 Co-processor: Silicon Graphics, Inc. IOC4 I/O controller (rev 53) 01:03.0 SCSI storage controller: QLogic Corp. ISP12160 Dual Channel Ultra3 SCSI Processor (rev 06)
01:04.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5701 Gigabit Ethernet (rev 15)
02:01.0 Fibre Channel: QLogic Corp. QLA2312 Fibre Channel Adapter (rev 02) 02:01.1 Fibre Channel: QLogic Corp. QLA2312 Fibre Channel Adapter (rev 02) 03:01.0 SCSI storage controller: LSI Logic / Symbios Logic 53c1030 PCI-X Fusion-MPT Dual Ultra320 SCSI (rev 07)
03:01.1 SCSI storage controller: LSI Logic / Symbios Logic 53c1030 PCI-X Fusion-MPT Dual Ultra320 SCSI (rev 07)
05:01.0 SCSI storage controller: LSI Logic / Symbios Logic 53c1030 PCI-X Fusion-MPT Dual Ultra320 SCSI (rev 07)
05:01.1 SCSI storage controller: LSI Logic / Symbios Logic 53c1030 PCI-X Fusion-MPT Dual Ultra320 SCSI (rev 07)
[root@altix3 ~]# cat /proc/iomem c00000880c200000-c00000880c2fffff : 0000:01:01.0 c00000880f400000-c00000880f41ffff : 0000:03:01.0 c00000880d400000-c00000880d41ffff : 0000:05:01.0 c00001880cc00000-c00001880cc00fff : 0000:02:01.0 c00001880cc01000-c00001880cc01fff : 0000:02:01.1
[root@altix3 ~]# cat /proc/ioports
00000060-0000006f : i8042
130027ad160-130027ad163 : PM1a_EVT_BLK
130027ad170-130027ad171 : PM1a_CNT_BLK
130027ad180-130027ad183 : PM_TMR
130027ad190-130027ad197 : GPE0_BLK c00000880c200100-c00000880c20011f : ide0 c00000880c200140-c00000880c200163 : ide0 c00000880c600000-c00000880c6000ff : 0000:01:03.0 c00001880ca00000-c00001880ca000ff : 0000:02:01.0 c00000880f200000-c00000880f2000ff : 0000:03:01.0 c00000880d200000-c00000880d2000ff : 0000:05:01.0
[root@altix3 ~]# !modprobe
modprobe sgi_hotplug
[root@altix3 ~]# cd /sys/bus/pci/slots/
[root@altix3 slots]# ls

m_001
_b_1_s_2  m_001
_b_3_s_1  m_001
_b_4_s_2  m_001
_b_6_s_1
m_001
_b_2_s_1  m_001
_b_3_s_2  m_001
_b_5_s_1  m_001
_b_6_s_2
m_001
_b_2_s_2  m_001
_b_4_s_1  m_001
_b_5_s_2

[root@altix3 slots]# cd m_001
_b_5_s_1
[root@altix3 m_001
_b_5_s_1]# echo 0 > power
Begin PCI Hot-Plug Message for -> m_001
_b_5_s_1
Remove operation successful
End PCI Hot-Plug Message for -> m_001
_b_5_s_1

[root@altix3 m_001
_b_5_s_1]# lspci
01:01.0 Co-processor: Silicon Graphics, Inc. IOC4 I/O controller (rev 53) 01:03.0 SCSI storage controller: QLogic Corp. ISP12160 Dual Channel Ultra3 SCSI Processor (rev 06)
01:04.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5701 Gigabit Ethernet (rev 15)
02:01.0 Fibre Channel: QLogic Corp. QLA2312 Fibre Channel Adapter (rev 02) 02:01.1 Fibre Channel: QLogic Corp. QLA2312 Fibre Channel Adapter (rev 02) 03:01.0 SCSI storage controller: LSI Logic / Symbios Logic 53c1030 PCI-X Fusion-MPT Dual Ultra320 SCSI (rev 07)
03:01.1 SCSI storage controller: LSI Logic / Symbios Logic 53c1030 PCI-X Fusion-MPT Dual Ultra320 SCSI (rev 07)
[root@altix3 m_001
_b_5_s_1]# cat /proc/iomem
Segmentation fault

Unable to handle kernel paging request at virtual address 6b6b6b6b6b6b6b8b
cat[2871]: Oops 11012296146944 [1]
Modules linked in: sgi_hotplug(U) md5(U) ipv6(U) parport_pc(U) lp(U) 

parport(U) autofs4(U) sunrpc(U) ds(U) yenta_socket(U) pcmcia_core(U) ipt_REJECT(U) ipt_state(U) ip_conntrack(U) iptable_filter(U) ip_tables(U) vfat(U) fat(U) button(U) sgiioc4(U) tg3(U) dm_snapshot(U) dm_zero(U) dm_mirror(U) ext3(U) jbd(U) dm_mod(U) qla2300(U) qla2xxx(U) scsi_transport_fc(U) qla1280(U) mptscsih(U) mptbase(U) sd_mod(U) scsi_mod(U)

Pid: 2871, CPU 3, comm: cat
psr : 0000121008126010 ifs : 800000000000028d ip :
[<a00000010007ddf1>] Not tainted
ip is at r_show+0x51/0x120
unat: 0000000000000000 pfs : 0000000000000894 rsc : 0000000000000003
rnat: 0000101008126010 bsps: a00000010022fe80 pr : 000000000955a959
ldrs: 0000000000000000 ccv : 0000000000000000 fpsr: 0009804c8a70033f csd : 0000000000000000 ssd : 0000000000000000
b0 : a00000010015ee70 b6 : a00000010007db80 b7 : a00000010007dda0 f6 : 0fffafffffffff0000000 f7 : 0ffdec000000000000000 f8 : 10002c000000000000000 f9 : 100038000000000000000 f10 : 0fffebffffffff4000000 f11 : 1003e0000000000000000 r1 : a00000010096a980 r2 : a000000100702a18 r3 : e00001b07be04748 r8 : 6b6b6b6b6b6b6b6b r9 : 6b6b6b6b6b6b6b8b r10 : a000000100634858 r11 : a00000010007dda0 r12 : e00000b0759afe00 r13 : e00000b0759a8000 r14 : 000000000000ffff r15 : ffffffffffffffff r16 : a000000100702a08 r17 : 6b6b6b6b6b6b6b6b r18 : 0000000000000062 r19 : e00000b0759afe20 r20 : a0000001005cfa00 r21 : 0000000000000270 r22 : a0000001005cf790 r23 : a000000100778a18 r24 : a0000001007124d3 r25 : 0000000000000073 r26 : e00000b0759afe10 r27 : ffffffffffffffff r28 : e00000b0759afdd0 r29 : 000000000000000b r30 : 000000000000000c r31 : fffffffffffffffe

Call Trace:
 [<a000000100016a40>] show_stack+0x80/0xa0
                                spà0000b0759af9b0 bspà0000b0759a9110
 [<a000000100017350>] show_regs+0x890/0x8c0
                                spà0000b0759afb80 bspà0000b0759a90c8
 [<a00000010003c970>] die+0x150/0x240
                                spà0000b0759afba0 bspà0000b0759a9088
 [<a00000010005d870>] ia64_do_page_fault+0x9f0/0xba0
                                spà0000b0759afba0 bspà0000b0759a9020
 [<a00000010000f480>] ia64_leave_kernel+0x0/0x260
                                spà0000b0759afc30 bspà0000b0759a9020
 [<a00000010007ddf0>] r_show+0x50/0x120
                                spà0000b0759afe00 bspà0000b0759a8fb0
 [<a00000010015ee70>] seq_read+0x730/0x940
                                spà0000b0759afe10 bspà0000b0759a8f28
 [<a000000100111590>] vfs_read+0x290/0x360
                                spà0000b0759afe20 bspà0000b0759a8ed8
 [<a000000100111bd0>] sys_read+0x70/0xe0
                                spà0000b0759afe20 bspà0000b0759a8e60

[root@altix3 m_001
_b_5_s_1]#

I'm close to what I think an appropriate solution is. I could just remove the erroneous call to pci_free_resources, however, I believe that this may cause some drivers (that are dependent on the pci_free_resources call) to break.

Since pci_free_resources is a legacy (deprecated?) call that we should modify the code such that release_resources sets the appropriate pci_dev->res pointers to null, and pci_free_resources only attempts to free a resource if the pointer is not NULL.

At that time we would output a warning message about the deprecation of the call ... IMO, pci_free_resources should never be called.

Of course, you could argue that the drivers are broken if they depend on pci_free_resources anyway ;)

P.

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id396&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

pci_free_resources is not a legacy call, it's a function internal to the pci core.

Does the following patch fix the issue for you?

thanks,

greg k-h

=== drivers/pci/remove.c 1.12 vs edited ==--- 1.12/drivers/pci/remove.c 2005-01-18 12:22:39 -08:00
+++ edited/drivers/pci/remove.c 2005-03-10 15:22:52 -08:00
@@ -19,7 +19,7 @@ static void pci_free_resources(struct pc
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);
}
}

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id396&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Hi Greg,

Sorry -- I read that back and realize how incorrect that was.

What I meant to say was pci_free_resources calls release_resource, where release_region calls __release_region. __release_region is called a "legacy" function? I assumed this meant that it was to be deprecated? Oh ... Uh ... does the term Legacy really refer to Legacy address space? (Prarit tries not to look sheepish asking that question.)

Nope, because the res pointer still points to an "old" memory address -- then the res value check succeeds and I would oops if res->parent was NULL. (Haven't tried it but I think it will still die if SLAB debug is on ...)

How about the attached patch? (I think the warning is a bit wordy ... if you have a better idea I'm all for it ... If you want to dump the warning I'm fine with that too...)

P.

P.S. Greg, I've sent 3 separate requests to the linux PCI list but haven't heard anything back. Do you know who the maintainer is? ... I'll double check my spam filter ...

===== drivers/pci/remove.c 1.6 vs edited =====

@@ -19,8 +19,12 @@
 	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) {
+			printk(KERN_WARNING "WARNING: PCI RESOURCE 0x%lx:0x%lx for dev %s free'd by PCI subsystem.",
+				pci_resource_start(dev,i), pci_resource_end(dev,i), dev->slot_name);
+			printk(KERN_WARNING " This should be done at the driver level.");
 			release_resource(res);
+		}
 	}
 }
 

===== kernel/resource.c 1.26 vs edited =====

@@ -505,6 +505,7 @@
 			*p = res->sibling;
 			write_unlock(&resource_lock);
 			kfree(res);
+			res = NULL;
 			return;
 		}
 		p = &res->sibling;

What documentation calls it that?

I don't like the wording at all, no one will notice it (trust me I've tried stuff like this before...)

But what's really curious, is why no one has hit this before. Nothing has changed recently in this area of the kernel. Did this used to work before? Does it work just fine without the patch for other drivers?

thanks,

greg k-h

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id396&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Greg,

Thanks for the reply. I'm at home today and don't have access to my SGI account. If you want to fwd this to the mailing list please feel free. If I don't see it Monday morning, I'll fwd it back to the list then.

Hrmm ... I distinctly remember seeing that somewhere. I'll find it again. I also recall the term "compatibility cruft" used ... in ioport.h?

Yeah ... I know. I've tried it before too. I've been working in this space for a long time and it's clear to me that it needs a compile time #define DEBUG printk option. Issues within the resource allocation code are impossible to debug without sticking a large # of printk's throughout the code.

I haven't gone back through kernels to determine where this "breakage" occurred, but I do know that it is in the 2.6.9 kernel as I have been developing on a RHEL4 platform.

This is how I stumbled across this: As you know I'm building and am testing an SGI Altix Hotplug Driver.

While testing I started up a few memory stress and IO stress tests *that did not involve the card I was targeting for the test* and approximately 5% of the time I hit a NULL pointer oops.

At first glance, I thought the issue was within the sysfs/proc filesystems or pci_free_resources as that's where the oops' were -- obviously with more inspection I realized that didn't make sense and it wasn't the case.

A few things have to happen (in the precise order) for someone to hit this issue. Note that I have been running on 16, 32, and 64 cpu systems so it is very like #4 below is due to another CPU.

1.  HP slot is disabled via sysfs.
2.  PCI driver must call release_regions
3.  release_regions kfree's resource structures
4.  Context switch/Another CPU: resource area is alloc'd by 

something else.
5. pci_free_resources is called

Possible oops right here.

I've also stumbled across the case where the pci_free_resources case tried to free non-existant regions -- dumping the memory address indicated in one case that I was looking at char data ... I recall that it looked like I was looking at the word "qla". I dumped /proc/iomem and /proc/ioports and incurred an oops in /proc .

Suppose #4 above doesn't happen. Step #5 occurs and no one (user or system) is the wiser. The memory is still intact -- no oops.

Additionally, I've seen people using PCI Hotplug in the field. Typically when removing a card sysadmins tend to quiesce the system before removal. That makes #4 just that much less likely to occur.

:) :) Want the long explanation? :) :)

P.

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id396&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Gee .. I haven't actually tried it :). That's probably a good test to run ... I'll try that first thing on Monday.

In some sense, yes. I would argue that it's not a PCI HotPlug issue, but a driver issue -- most of the drivers out there don't pay a lot of attention to the stability of the driver remove function :(

I've seen all sorts of things in removes that are done wrong -- from not terminating timers, to not quiecing queues, to not properly kfree'ing memory.

Qlogic drivers are especially bad :) ... That's why I hesitate to test with them in the system but in this case it was what was available to me.

I'll go back to work and pull out the oops' etc.

Thanks Greg :) I've tested with that patch and it seems better. IMO though, we still have a glitch in PCI. I'm not convinced that it should be calling pci_free_resources at all.

Thanks,

P.

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id396&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Note that ia64 arch code calls pci_claim_resources() at bus scan time (in pcibios_fixup_bus()). This adds the resources in pci_dev structure to the resource list, even before the corresponding driver is loaded. It is fairly common for a device driver to then call pci_request_regions() when it is loaded. This causes a new resource descriptor to be allocated and added to the resource list. For example, on my tiger4 ia64 system, /proc/iomem shows:

<snip>
f9000000-fbffffff : PCI Bus 0000:00
f9ff0000-f9ff03ff : 0000:00:1d.7

    f9ff0000-f9ff03ff : ehci_hcd
  fa000000-faffffff : 0000:01:01.0
  fbfa0000-fbfbffff : 0000:01:01.0
  fbfd0000-fbfd0fff : 0000:01:01.0
  fbfe0000-fbffffff : 0000:01:00.0
    fbfe0000-fbffffff : e1000

<snip>

Since we have 2 resource descriptors for the same resource in some cases, they both do need to be removed at device remove time. However, I saw only 1 resource in your /proc/iomem (which was presumably after the driver had loaded). I wonder if this is the problem in this case.

In any case, claiming and freeing pci resources needs wider cleanup. See

http://marc.theaimsgroup.com/?l=linux-kernel&m0841094228134&w=2
http://marc.theaimsgroup.com/?l=linux-kernel&m0842019012889&w=2

for related discussions.

Rajesh

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id396&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

I've noticed this as well ...

UGH. This gets worse and worse, doesn't it :)

Thanks Rajesh -- I appreciate the pointers to earlier discussions.

P.

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id396&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel