PCI device order problem

DISCUSSION H. J. Lu 4 participants 26 messages Oct 24, 2002 – Oct 28, 2002

Share & tools

H. J. Lu

author

2002-10-24 23:33 UTC #0 lore raw

In arch/i386/kernel/pci-pc.c, there are

/*
* Sort the device list according to PCI BIOS. Nasty hack, but since some
* fool forgot to define the `correct' device order in the PCI BIOS specs
* and we want to be (possibly bug-to-bug ;-]) compatible with older kernels
* which used BIOS ordering, we are bound to do this...
*/

static void __devinit pcibios_sort(void)

The problem is on my MB:

00:00.0 Host bridge: Intel Corp. e7500 [Plumas] DRAM Controller (rev 03) 00:00.1 Class ff00: Intel Corp. e7500 [Plumas] DRAM Controller Error Reporting ( rev 03) 00:03.0 PCI bridge: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F0) (rev 03) 00:03.1 Class ff00: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F1) (rev 03) 00:1d.0 USB Controller: Intel Corp. 82801CA/CAM USB (Hub (rev 02)
00:1d.1 USB Controller: Intel Corp. 82801CA/CAM USB (Hub (rev 02)
00:1e.0 PCI bridge: Intel Corp. 82801BA/CA/DB PCI Bridge (rev 42)
00:1f.0 ISA bridge: Intel Corp. 82801CA ISA Bridge (LPC) (rev 02)
00:1f.1 IDE interface: Intel Corp. 82801CA IDE U100 (rev 02)
00:1f.3 SMBus: Intel Corp. 82801CA/CAM SMBus (rev 02)
01:0c.0 VGA compatible controller: ATI Technologies Inc Rage XL (rev 27) 02:1c.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
02:1d.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03) 02:1e.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
02:1f.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03) 03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01) 03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01) 03:08.0 RAID bus controller: 3ware Inc 3ware 7000-series ATA-RAID (rev 01)

Eth1 becomes:
03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)

and eth0 becomes:
03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)

Is that a good idea to have an option to sort the PCI device by PCI bus and slot numbers?

H.J.

Jeff Garzik

2002-10-24 23:43 UTC #1 lore raw

Quoted 30 lines · H. J. Lu

In arch/i386/kernel/pci-pc.c, there are
/*
* Sort the device list according to PCI BIOS. Nasty hack, but since some
* fool forgot to define the `correct' device order in the PCI BIOS specs
* and we want to be (possibly bug-to-bug ;-]) compatible with older kernels * which used BIOS ordering, we are bound to do this... */
static void __devinit pcibios_sort(void)
The problem is on my MB:
00:00.0 Host bridge: Intel Corp. e7500 [Plumas] DRAM Controller (rev 03) 00:00.1 Class ff00: Intel Corp. e7500 [Plumas] DRAM Controller Error Reporting ( rev 03) 00:03.0 PCI bridge: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F0) (rev 03) 00:03.1 Class ff00: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F1) (rev 03) 00:1d.0 USB Controller: Intel Corp. 82801CA/CAM USB (Hub (rev 02)
00:1d.1 USB Controller: Intel Corp. 82801CA/CAM USB (Hub (rev 02)
00:1e.0 PCI bridge: Intel Corp. 82801BA/CA/DB PCI Bridge (rev 42)
00:1f.0 ISA bridge: Intel Corp. 82801CA ISA Bridge (LPC) (rev 02)
00:1f.1 IDE interface: Intel Corp. 82801CA IDE U100 (rev 02)
00:1f.3 SMBus: Intel Corp. 82801CA/CAM SMBus (rev 02)
01:0c.0 VGA compatible controller: ATI Technologies Inc Rage XL (rev 27) 02:1c.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
02:1d.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03) 02:1e.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
02:1f.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03) 03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01) 03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01) 03:08.0 RAID bus controller: 3ware Inc 3ware 7000-series ATA-RAID (rev 01)

Quoted 10 lines

Eth1 becomes:
03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
and eth0 becomes:
03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
Is that a good idea to have an option to sort the PCI device by PCI bus and slot numbers?

Without answering your specific question, but addressing $subject, what problem is caused by the PCI device order you see?

You can use ETHTOOL_GDRVINFO to get the PCI bus info for an ethernet interface, so you know at all times what device is associated with what PCI device. And among other ways you are notified of device addition/removal by the execution of /sbin/hotplug for each ethernet interface.

Jeff

H. J. Lu

author

2002-10-24 23:50 UTC #2 lore raw

Quoted 40 lines · Jeff Garzik

Quoted 38 lines · H. J. Lu
In arch/i386/kernel/pci-pc.c, there are
/*
* Sort the device list according to PCI BIOS. Nasty hack, but since some
* fool forgot to define the `correct' device order in the PCI BIOS specs
* and we want to be (possibly bug-to-bug ;-]) compatible with older kernels
* which used BIOS ordering, we are bound to do this...
*/
static void __devinit pcibios_sort(void)
The problem is on my MB:
00:00.0 Host bridge: Intel Corp. e7500 [Plumas] DRAM Controller (rev 03) 00:00.1 Class ff00: Intel Corp. e7500 [Plumas] DRAM Controller Error Reporting ( rev 03) 00:03.0 PCI bridge: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F0) (rev 03) 00:03.1 Class ff00: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F1) (rev 03) 00:1d.0 USB Controller: Intel Corp. 82801CA/CAM USB (Hub (rev 02)
00:1d.1 USB Controller: Intel Corp. 82801CA/CAM USB (Hub (rev 02)
00:1e.0 PCI bridge: Intel Corp. 82801BA/CA/DB PCI Bridge (rev 42)
00:1f.0 ISA bridge: Intel Corp. 82801CA ISA Bridge (LPC) (rev 02)
00:1f.1 IDE interface: Intel Corp. 82801CA IDE U100 (rev 02)
00:1f.3 SMBus: Intel Corp. 82801CA/CAM SMBus (rev 02)
01:0c.0 VGA compatible controller: ATI Technologies Inc Rage XL (rev 27) 02:1c.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
02:1d.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03) 02:1e.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
02:1f.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03) 03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01) 03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01) 03:08.0 RAID bus controller: 3ware Inc 3ware 7000-series ATA-RAID (rev 01)
Eth1 becomes:
03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
and eth0 becomes:
03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)

Quoted 7 lines

Quoted 4 lines
Is that a good idea to have an option to sort the PCI device by PCI bus and slot numbers?
Without answering your specific question, but addressing $subject, what problem is caused by the PCI device order you see?

It is different from the hardware documentation. The hardware manual says it has 2 NICs, NIC 1 (03:07.0) and NIC2 (03:07.1), which makes senses to me. NIC 1 is a special one which supports IPMI over LAN. Since we only use one NIC now, we'd like to use NIC 1 and call it eth0.

H.J.

Jeff Garzik

2002-10-25 00:08 UTC #3 lore raw

Quoted 49 lines · H. J. Lu

Quoted 46 lines · Jeff Garzik
Quoted 42 lines · H. J. Lu
In arch/i386/kernel/pci-pc.c, there are
/*
* Sort the device list according to PCI BIOS. Nasty hack, but since some
* fool forgot to define the `correct' device order in the PCI BIOS specs
* and we want to be (possibly bug-to-bug ;-]) compatible with older kernels * which used BIOS ordering, we are bound to do this... */
static void __devinit pcibios_sort(void)
The problem is on my MB:
00:00.0 Host bridge: Intel Corp. e7500 [Plumas] DRAM Controller (rev 03) 00:00.1 Class ff00: Intel Corp. e7500 [Plumas] DRAM Controller Error Reporting ( rev 03) 00:03.0 PCI bridge: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F0) (rev 03) 00:03.1 Class ff00: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F1) (rev 03) 00:1d.0 USB Controller: Intel Corp. 82801CA/CAM USB (Hub (rev 02)
00:1d.1 USB Controller: Intel Corp. 82801CA/CAM USB (Hub (rev 02)
00:1e.0 PCI bridge: Intel Corp. 82801BA/CA/DB PCI Bridge (rev 42)
00:1f.0 ISA bridge: Intel Corp. 82801CA ISA Bridge (LPC) (rev 02)
00:1f.1 IDE interface: Intel Corp. 82801CA IDE U100 (rev 02)
00:1f.3 SMBus: Intel Corp. 82801CA/CAM SMBus (rev 02)
01:0c.0 VGA compatible controller: ATI Technologies Inc Rage XL (rev 27) 02:1c.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
02:1d.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03) 02:1e.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
02:1f.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03) 03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01) 03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01) 03:08.0 RAID bus controller: 3ware Inc 3ware 7000-series ATA-RAID (rev 01)
Eth1 becomes:
03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
and eth0 becomes:
03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
Is that a good idea to have an option to sort the PCI device by PCI bus and slot numbers?

Quoted 10 lines

Quoted 3 lines
Without answering your specific question, but addressing $subject, what problem is caused by the PCI device order you see?
It is different from the hardware documentation. The hardware manual says it has 2 NICs, NIC 1 (03:07.0) and NIC2 (03:07.1), which makes senses to me. NIC 1 is a special one which supports IPMI over LAN. Since we only use one NIC now, we'd like to use NIC 1 and call it eth0.

Well, overall, depending on ordering is error-prone (as you see). I would suggest migrating to a less-fragile scheme. nameif and ethtool together should get you exactly the device you need... (though I wonder why simply using eth1 is so awful, if this situation is constant...)

Jeff

H. J. Lu

author

2002-10-25 00:12 UTC #4 lore raw

Quoted 65 lines · Jeff Garzik

Quoted 56 lines · H. J. Lu
Quoted 42 lines · Jeff Garzik
Quoted 38 lines · H. J. Lu
In arch/i386/kernel/pci-pc.c, there are
/*
* Sort the device list according to PCI BIOS. Nasty hack, but since some
* fool forgot to define the `correct' device order in the PCI BIOS specs
* and we want to be (possibly bug-to-bug ;-]) compatible with older kernels
* which used BIOS ordering, we are bound to do this...
*/
static void __devinit pcibios_sort(void)
The problem is on my MB:
00:00.0 Host bridge: Intel Corp. e7500 [Plumas] DRAM Controller (rev 03) 00:00.1 Class ff00: Intel Corp. e7500 [Plumas] DRAM Controller Error Reporting ( rev 03) 00:03.0 PCI bridge: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F0) (rev 03) 00:03.1 Class ff00: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F1) (rev 03) 00:1d.0 USB Controller: Intel Corp. 82801CA/CAM USB (Hub (rev 02)
00:1d.1 USB Controller: Intel Corp. 82801CA/CAM USB (Hub (rev 02)
00:1e.0 PCI bridge: Intel Corp. 82801BA/CA/DB PCI Bridge (rev 42)
00:1f.0 ISA bridge: Intel Corp. 82801CA ISA Bridge (LPC) (rev 02)
00:1f.1 IDE interface: Intel Corp. 82801CA IDE U100 (rev 02)
00:1f.3 SMBus: Intel Corp. 82801CA/CAM SMBus (rev 02)
01:0c.0 VGA compatible controller: ATI Technologies Inc Rage XL (rev 27) 02:1c.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
02:1d.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03) 02:1e.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
02:1f.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03) 03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01) 03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01) 03:08.0 RAID bus controller: 3ware Inc 3ware 7000-series ATA-RAID (rev 01)
Eth1 becomes:
03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
and eth0 becomes:
03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
Quoted 10 lines
Quoted 6 lines
Is that a good idea to have an option to sort the PCI device by PCI bus and slot numbers?
Without answering your specific question, but addressing $subject, what problem is caused by the PCI device order you see?
Quoted 6 lines
It is different from the hardware documentation. The hardware manual says it has 2 NICs, NIC 1 (03:07.0) and NIC2 (03:07.1), which makes senses to me. NIC 1 is a special one which supports IPMI over LAN. Since we only use one NIC now, we'd like to use NIC 1 and call it eth0.

Quoted 4 lines

Well, overall, depending on ordering is error-prone (as you see). I would suggest migrating to a less-fragile scheme. nameif and ethtool together should get you exactly the device you need... (though I wonder why simply using eth1 is so awful, if this situation is constant...)

We can use eth1. It is just very confusing since Linux and hardware manual don't agree which one is the first NIC. Also, when we upgrade the BIOS, the BIOS order may change. As for other schemes, we don't want to change every software which access ethX.

H.J.

Alan Cox

2002-10-25 09:37 UTC #5 lore raw

In reply to H. J. Lu

Quoted 4 lines · H. J. Lu

It is different from the hardware documentation. The hardware manual says it has 2 NICs, NIC 1 (03:07.0) and NIC2 (03:07.1), which makes senses to me. NIC 1 is a special one which supports IPMI over LAN. Since we only use one NIC now, we'd like to use NIC 1 and call it eth0.

SIOCSIFNAME ioctl. You can call them "haddock" and "chips" if you really want, or swap the eth%d names about. RH 8.0 allows you to bind an interface to a mac address too

H. J. Lu

author

2002-10-25 16:04 UTC #6 lore raw

Quoted 10 lines · Alan Cox

Quoted 4 lines · H. J. Lu
It is different from the hardware documentation. The hardware manual says it has 2 NICs, NIC 1 (03:07.0) and NIC2 (03:07.1), which makes senses to me. NIC 1 is a special one which supports IPMI over LAN. Since we only use one NIC now, we'd like to use NIC 1 and call it eth0.
SIOCSIFNAME ioctl. You can call them "haddock" and "chips" if you really want, or swap the eth%d names about. RH 8.0 allows you to bind an interface to a mac address too

It doesn't help RedHat installer over network.

H.J.

H. J. Lu

author

2002-10-26 03:19 UTC #7 lore raw

PATCH: Support PCI device sorting (Re: PCI device order problem)

Here is a patch to add an option to sort the PCI device by bus, slot and function.

H.J.

arch/i386/config.in renamed from arch/i386/config.in.order +1 -0

@@ -257,6 +257,7 @@ else
       if [ "$CONFIG_PCI_GODIRECT" = "y" -o "$CONFIG_PCI_GOANY" = "y" ]; then
          define_bool CONFIG_PCI_DIRECT y
       fi
+      bool '  Sort device by bus, slot, function' CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC
    fi
 fi

arch/i386/kernel/pci-i386.h renamed from arch/i386/kernel/pci-i386.h.order +1 -0

@@ -21,6 +21,7 @@
 #define PCI_ASSIGN_ROMS		0x1000
 #define PCI_BIOS_IRQ_SCAN	0x2000
 #define PCI_ASSIGN_ALL_BUSSES	0x4000
+#define PCI_BUS_SORT		0x8000
 
 extern unsigned int pci_probe;

arch/i386/kernel/pci-pc.c renamed from arch/i386/kernel/pci-pc.c.order +65 -1

92 lines not on this page · raw message

Jeff Garzik

2002-10-26 21:06 UTC #8 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Well, WRT your implementation, the function you add is dead code if your new config variable is not set, which is not desireable at all.

WRT the overall idea, I would prefer to see what Linus and Martin Mares (and Ivan K) thought about it, before merging it. The x86 PCI code is very touchy, and your patch has the potential to change driver probe order for little gain.

Jeff

H. J. Lu

author

2002-10-26 21:20 UTC #9 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Quoted 2 lines · Jeff Garzik

Well, WRT your implementation, the function you add is dead code if your new config variable is not set, which is not desireable at all.

I am not sure if I understand what you were trying to say. If CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is not set, you should be able to pass "pci=bussort" to kernel to sort the PCI device by bus, slot and function. Did I miss something?

Quoted 6 lines

WRT the overall idea, I would prefer to see what Linus and Martin Mares (and Ivan K) thought about it, before merging it. The x86 PCI code is very touchy, and your patch has the potential to change driver probe order for little gain.

The whole purpose of my patch is to change PCI driver probe order in such a way that is BIOS independent.

H.J.

Jeff Garzik

2002-10-26 21:29 UTC #10 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Quoted 12 lines · H. J. Lu

Quoted 3 lines · Jeff Garzik
Well, WRT your implementation, the function you add is dead code if your new config variable is not set, which is not desireable at all.
I am not sure if I understand what you were trying to say. If
CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is not set, you should be able to pass "pci=bussort" to kernel to sort the PCI device by bus, slot and function. Did I miss something?

The sorting function you add should be covered by the ifdef you add.

Quoted 9 lines

Quoted 4 lines
WRT the overall idea, I would prefer to see what Linus and Martin Mares (and Ivan K) thought about it, before merging it. The x86 PCI code is very touchy, and your patch has the potential to change driver probe order for little gain.
The whole purpose of my patch is to change PCI driver probe order in such a way that is BIOS independent.

The purpose is irrelevant to the effect on existing drivers and systems -- which is unknown. Making the probe order independent of BIOS ordering may very well break drivers and systems that are dependent on BIOS ordering. IOW what looks nice on your system could _likely_ suck for others. That's what I meant by "x86 PCI code is very touchy."

Jeff

H. J. Lu

author

2002-10-26 21:38 UTC #11 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Quoted 10 lines · Jeff Garzik

Quoted 8 lines · H. J. Lu
Quoted 4 lines · Jeff Garzik
Well, WRT your implementation, the function you add is dead code if your new config variable is not set, which is not desireable at all.

Quoted 7 lines

Quoted 6 lines
I am not sure if I understand what you were trying to say. If CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is not set, you should be able to pass "pci=bussort" to kernel to sort the PCI device by bus, slot and function. Did I miss something?
The sorting function you add should be covered by the ifdef you add.

Have you tried? If I did that, the kernel wouldn't even compile. As I said, when CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC isn't defined, my sorting function is still available, just not called by default.

Quoted 14 lines

Quoted 9 lines
Quoted 8 lines
WRT the overall idea, I would prefer to see what Linus and Martin Mares (and Ivan K) thought about it, before merging it. The x86 PCI code is very touchy, and your patch has the potential to change driver probe order for little gain.
Quoted 4 lines
The whole purpose of my patch is to change PCI driver probe order in such a way that is BIOS independent.

Quoted 5 lines

The purpose is irrelevant to the effect on existing drivers and systems -- which is unknown. Making the probe order independent of BIOS ordering may very well break drivers and systems that are dependent on BIOS ordering. IOW what looks nice on your system could _likely_ suck for others. That's what I meant by "x86 PCI code is very touchy."

That is why CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is off by default and even if it is on, you can still override it by passing "pci=nosort" or "pci=nobussort" to kernel.

H.J.

Jeff Garzik

2002-10-26 21:58 UTC #12 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Quoted 24 lines · H. J. Lu

Quoted 21 lines · Jeff Garzik
Quoted 17 lines · H. J. Lu
Quoted 5 lines · Jeff Garzik
Well, WRT your implementation, the function you add is dead code if your new config variable is not set, which is not desireable at all.
I am not sure if I understand what you were trying to say. If
CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is not set, you should be able to pass "pci=bussort" to kernel to sort the PCI device by bus, slot and function. Did I miss something?

Quoted 8 lines

Quoted 3 lines
The sorting function you add should be covered by the ifdef you add.
Have you tried? If I did that, the kernel wouldn't even compile. As I said, when CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC isn't defined, my sorting function is still available, just not called by default.

Which is the entire problem. The kernel compiles and builds just fine right now, without your function.

If it is "just not called by default" then it clearly can be removed at compile time when a certain CONFIG_xxx is not defined.

Quoted 15 lines

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
WRT the overall idea, I would prefer to see what Linus and Martin Mares (and Ivan K) thought about it, before merging it. The x86 PCI code is very touchy, and your patch has the potential to change driver probe order for little gain.
Quoted 10 lines
Quoted 5 lines
The whole purpose of my patch is to change PCI driver probe order in such a way that is BIOS independent.
The purpose is irrelevant to the effect on existing drivers and systems
-- which is unknown. Making the probe order independent of BIOS
ordering may very well break drivers and systems that are dependent on BIOS ordering. IOW what looks nice on your system could _likely_ suck for others. That's what I meant by "x86 PCI code is very touchy."

Quoted 5 lines

That is why CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is off by default and even if it is on, you can still override it by passing "pci=nosort" or "pci=nobussort" to kernel.

Sigh. Repeating, the kernel is still bloated by your sorting function if CONFIG_PCI_SORT_BY_BUS_SLOT is not defined. The function should go away if CONFIG_PCI_SORT_BY_BUS_SLOT is not defined.

Jeff

H. J. Lu

author

2002-10-26 22:14 UTC #13 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Quoted 3 lines · Jeff Garzik

Which is the entire problem. The kernel compiles and builds just fine right now, without your function.

Without my patch or my function? My patched file has

        if ((pci_probe & PCI_BUS_SORT) && !(pci_probe & PCI_NO_SORT))
                pci_sort_by_bus_slot_func();
#ifdef CONFIG_PCI_BIOS
        else if ((pci_probe & PCI_BIOS_SORT) && !(pci_probe & PCI_NO_SORT))
                pcibios_sort();

#endif

That is pci_sort_by_bus_slot_func () will be called if PCI_BUS_SORT is set. It is independent of whether CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is set or not, which sets PCI_BUS_SORT. If pci_sort_by_bus_slot_func is not defined when CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is not set. How can kernel compile?

Quoted 2 lines

If it is "just not called by default" then it clearly can be removed at compile time when a certain CONFIG_xxx is not defined.

It is controlled by PCI_BUS_SORT, not CONFIG_xxx.

Quoted 28 lines

Quoted 22 lines
Quoted 14 lines
Quoted 9 lines
Quoted 9 lines
WRT the overall idea, I would prefer to see what Linus and Martin Mares (and Ivan K) thought about it, before merging it. The x86 PCI code is very touchy, and your patch has the potential to change driver probe order for little gain.
Quoted 5 lines
The whole purpose of my patch is to change PCI driver probe order in such a way that is BIOS independent.
Quoted 7 lines
The purpose is irrelevant to the effect on existing drivers and systems -- which is unknown. Making the probe order independent of BIOS ordering may very well break drivers and systems that are dependent on BIOS ordering. IOW what looks nice on your system could _likely_ suck for others. That's what I meant by "x86 PCI code is very touchy."
Quoted 5 lines
That is why CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is off by default and even if it is on, you can still override it by passing "pci=nosort" or "pci=nobussort" to kernel.

Quoted 3 lines

Sigh. Repeating, the kernel is still bloated by your sorting function if CONFIG_PCI_SORT_BY_BUS_SLOT is not defined. The function should go away if CONFIG_PCI_SORT_BY_BUS_SLOT is not defined.

I added pci=nobussort since it might not be safe for all MBs. Then I added "pci=bussort". I have no problem taking out "pci=bussort". If you don't want "pci=bussort", please say so. I can provide a new patch which won't define pci_sort_by_bus_slot_func if CONFIG_PCI_SORT_BY_BUS_SLOT is not set and won't have "pci=bussort" either.

H.J.

Jeff Garzik

2002-10-26 22:23 UTC #14 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Quoted 15 lines · H. J. Lu

Quoted 4 lines · Jeff Garzik
Which is the entire problem. The kernel compiles and builds just fine right now, without your function.
Without my patch or my function? My patched file has
       if ((pci_probe & PCI_BUS_SORT) && !(pci_probe & PCI_NO_SORT))
               pci_sort_by_bus_slot_func();
#ifdef CONFIG_PCI_BIOS
       else if ((pci_probe & PCI_BIOS_SORT) && !(pci_probe & PCI_NO_SORT))
               pcibios_sort();

Quoted 8 lines

#endif
That is pci_sort_by_bus_slot_func () will be called if PCI_BUS_SORT is set. It is independent of whether CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is set or not, which sets PCI_BUS_SORT. If pci_sort_by_bus_slot_func is not defined when CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is not set. How can kernel compile?

I am obviously not being understood / not communicating myself clearly.

Your patch unconditionally adds a new function. Don't do that. Wrap it in an ifdef, like you propose below.

Quoted 7 lines

I added pci=nobussort since it might not be safe for all MBs. Then I added "pci=bussort". I have no problem taking out "pci=bussort". If you don't want "pci=bussort", please say so. I can provide a new patch which won't define pci_sort_by_bus_slot_func if CONFIG_PCI_SORT_BY_BUS_SLOT is not set and won't have "pci=bussort" either.

You're still missing the point here too.

Your patch has the potential to suddenly make systems unbootable, to suddenly reverse people's ethX interface numbering, etc. No command line options in the world will save lkml from being deluged by tons of "my system doesn't network anymore" bug reports.

The basic point is "let's proceed with caution, and test test test before applying this patch."

Jeff

H. J. Lu

author

2002-10-26 22:47 UTC #15 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Quoted 9 lines · Jeff Garzik

Quoted 8 lines
I added pci=nobussort since it might not be safe for all MBs. Then I added "pci=bussort". I have no problem taking out "pci=bussort". If you don't want "pci=bussort", please say so. I can provide a new patch which won't define pci_sort_by_bus_slot_func if CONFIG_PCI_SORT_BY_BUS_SLOT is not set and won't have "pci=bussort" either.

Quoted 4 lines

You're still missing the point here too.
Your patch has the potential to suddenly make systems unbootable, to suddenly reverse people's ethX interface numbering, etc. No command

That is the whole purpose of my patch. But you will only get it when you set CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC.

Quoted 2 lines

line options in the world will save lkml from being deluged by tons of "my system doesn't network anymore" bug reports.

You will only get it when you set CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC on purpose by hand.

Quoted 3 lines

The basic point is "let's proceed with caution, and test test test before applying this patch."

Please state clearly what you have in mind. First you were saying you didn't like pci_sort_by_bus_slot_func defined when CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC wass not set. Now you were saying my patch was dangerous. Please make up your mind.

H.J.

Jeff Garzik

2002-10-26 22:52 UTC #16 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Quoted 10 lines · H. J. Lu

Quoted 3 lines
The basic point is "let's proceed with caution, and test test test before applying this patch."
Please state clearly what you have in mind. First you were saying you didn't like pci_sort_by_bus_slot_func defined when CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC wass not set. Now you were saying my patch was dangerous. Please make up your mind.

In my first reply, I clearly separated implementation issues from commentary on the overall idea. Aside from that, I don't see much value in further repeating what I've already said.

Jeff

Alan Cox

2002-10-26 23:20 UTC #17 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Quoted 3 lines · Jeff Garzik

In my first reply, I clearly separated implementation issues from commentary on the overall idea. Aside from that, I don't see much value in further repeating what I've already said.

Its pretty pointless code but since its done at boot time, just marking it __init ought to be quite sufficient.

H. J. Lu

author

2002-10-26 23:47 UTC #18 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

In reply to Jeff Garzik

Quoted 14 lines · Jeff Garzik

Quoted 11 lines · H. J. Lu
Quoted 4 lines
The basic point is "let's proceed with caution, and test test test before applying this patch."
Please state clearly what you have in mind. First you were saying you didn't like pci_sort_by_bus_slot_func defined when CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC wass not set. Now you were saying my patch was dangerous. Please make up your mind.

Quoted 4 lines

In my first reply, I clearly separated implementation issues from commentary on the overall idea. Aside from that, I don't see much value in further repeating what I've already said.

Here is an alternative patch, which won't define pci_sort_by_bus_slot_func if CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is not set.

H.J.

arch/i386/config.in renamed from arch/i386/config.in.order +1 -0

@@ -257,6 +257,7 @@ else
       if [ "$CONFIG_PCI_GODIRECT" = "y" -o "$CONFIG_PCI_GOANY" = "y" ]; then
          define_bool CONFIG_PCI_DIRECT y
       fi
+      bool '  Sort device by bus, slot, function' CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC
    fi
 fi

arch/i386/kernel/pci-i386.h renamed from arch/i386/kernel/pci-i386.h.order +1 -0

@@ -21,6 +21,7 @@
 #define PCI_ASSIGN_ROMS		0x1000
 #define PCI_BIOS_IRQ_SCAN	0x2000
 #define PCI_ASSIGN_ALL_BUSSES	0x4000
+#define PCI_BUS_SORT		0x8000
 
 extern unsigned int pci_probe;

arch/i386/kernel/pci-pc.c renamed from arch/i386/kernel/pci-pc.c.order +64 -0

89 lines not on this page · raw message

Jeff Garzik

2002-10-26 23:51 UTC #19 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

s/__devinit/__init/ and the implementation looks ok to me

Jeff Garzik

2002-10-26 23:59 UTC #20 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Quoted 1 line · Jeff Garzik

s/__devinit/__init/ and the implementation looks ok to me

...except if your patch can be called in hotplug paths...

Alan Cox

2002-10-27 00:06 UTC #21 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Quoted 7 lines · Jeff Garzik

Quoted 1 line · Jeff Garzik
s/__devinit/__init/ and the implementation looks ok to me
...except if your patch can be called in hotplug paths...

We can't go around re-sorting the PCI devices. The pci lists are not locked sanely as it is

H. J. Lu

author

2002-10-27 00:19 UTC #22 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

In reply to Jeff Garzik

Quoted 7 lines · Jeff Garzik

Quoted 1 line · Jeff Garzik
s/__devinit/__init/ and the implementation looks ok to me
...except if your patch can be called in hotplug paths...

There are plenty of __devini in arch/i386/kernel/pci-pc.c. I will leave mine alone.

H.J.

Greg KH

2002-10-27 17:38 UTC #23 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Quoted 11 lines · H. J. Lu

Quoted 7 lines · Jeff Garzik
Quoted 1 line · Jeff Garzik
s/__devinit/__init/ and the implementation looks ok to me
...except if your patch can be called in hotplug paths...
There are plenty of __devini in arch/i386/kernel/pci-pc.c. I will leave mine alone.

That is because those functions can be called in PCI hotplug paths, since yours is only called during init, it should be marked as such.

thanks,

greg k-h

H. J. Lu

author

2002-10-27 20:36 UTC #24 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Quoted 8 lines · Greg KH

Quoted 7 lines · H. J. Lu
Quoted 6 lines · Jeff Garzik
Quoted 1 line · Jeff Garzik
s/__devinit/__init/ and the implementation looks ok to me

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
...except if your patch can be called in hotplug paths...
There are plenty of __devini in arch/i386/kernel/pci-pc.c. I will leave mine alone.
That is because those functions can be called in PCI hotplug paths, since yours is only called during init, it should be marked as such.

Are you telling me that pcibios_sort will be called by PCI hotplug?

H.J.

Jeff Garzik

2002-10-28 00:20 UTC #25 lore raw

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

Quoted 24 lines · H. J. Lu

Quoted 21 lines · Greg KH
Quoted 18 lines · H. J. Lu
Quoted 11 lines · Jeff Garzik
Quoted 3 lines · Jeff Garzik
s/__devinit/__init/ and the implementation looks ok to me
...except if your patch can be called in hotplug paths...
There are plenty of __devini in arch/i386/kernel/pci-pc.c. I will leave mine alone.

Quoted 9 lines

Quoted 5 lines
That is because those functions can be called in PCI hotplug paths, since yours is only called during init, it should be marked as such.
Are you telling me that pcibios_sort will be called by PCI hotplug?

no, he's saying __init is correct, as Alan and I said.

Jeff