On 8/4/07, Andrew Morton <[EMAIL PROTECTED]> wrote: > On Sat, 4 Aug 2007 11:30:41 +0200 Andi Kleen <[EMAIL PROTECTED]> wrote: > > > On Saturday 04 August 2007 00:50, Andrew Morton wrote: > > > On Fri, 03 Aug 2007 18:10:03 -0400 > > > > > > Chuck Ebbert <[EMAIL PROTECTED]> wrote: > > > > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=250859 > > > > > > > > at line 74: > > > > > > > > [EMAIL PROTECTED]: > > > > [EMAIL PROTECTED]: sd = > > > > bus->sysdata; > > > > [EMAIL PROTECTED]: sd->node = > > > > node; <===== > > > > > > > > bus->sysdata is NULL. > > > > > > > > Last changed by this hunk of > > > > "x86-64: introduce struct pci_sysdata to facilitate sharing of > > > > ->sysdata": > > > > Hmm, will double check. Perhaps Muli's conversion was incomplete. > > hm. > > > > > @@ -67,7 +69,9 @@ fill_mp_bus_to_cpumask(void) > > > > continue; > > > > if (!node_online(node)) > > > > node = 0; > > > > - bus->sysdata = (void *)node; > > > > + > > > > + sd = bus->sysdata; > > > > + sd->node = node; > > > > } > > > > } > > > > } > > > > > > Andy keeps trotting out a patch which will probably fix this, > > > > What patch do you mean? I don't have anything sysdata related > > left over. > > > > "pci device ensure sysdata initialised", now at version 4. > > > > From: Andy Whitcroft <[EMAIL PROTECTED]> > > We have been seeing panic's on NUMA systems in pci_call_probe() in > 2.6.19-rc1-mm1 and later. This is related to the changes introduced in the > commit below: > > [x86, PCI] Switch pci_bus::sysdata from NUMA node integer to a pointer > 0a247a58fc3e2ecfc17654301033e8b8d08df2a2 > > In this change the sysdata has changed from directly representing a value > (the node number in NUMA) to a pointer to a structure. However, it seems > that we do not always initialise this sysdata before we probe the device. > > Prior to the changes above the node was defaulted to 'NULL' allocating the > devices to node 0 unconditionally. This patch adds a default sysdata entry > (pci_default_sysdata), this is then used where 'NULL' was used previously. > pci_default_sysdata defaults the node to unknown (-1). This is a more > accurate assignment, mirroring the value returned where no topology support > is provided and no locality information is available. > > There are only two uses of this value in the affected architectures > (x86, x86_64) and generic code: > > 1) in x86_64, dma_alloc_pages() looks up the node in order to > allocate node local memory. Here if the node is invalid we > will default to the first online node. Behaviour here should > be unchanged. > 2) in generic, pci_call_probe() looks up the node in order to > restrict execution of the probe on the card local node, to > favor node local allocation. Where this is unknown previously > we would force execution (and thereby allocation) to node 0, > this is arguably wrong and using -1 releases this restriction. > > In an ideal world we should be supplying a sysdata for the > appropriate node where it is known. Where it is not known defaulting > to -1 seems a better course, and would help us where node 0 is > short of memory. > > Signed-off-by: Andy Whitcroft <[EMAIL PROTECTED]> > Acked-by: Yinghai Lu <[EMAIL PROTECTED]> > Cc: Andi Kleen <[EMAIL PROTECTED]> > Cc: Jeff Garzik <[EMAIL PROTECTED]> > Cc: Greg KH <[EMAIL PROTECTED]> > Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
Andrew, still need x86_64-get-mp_bus_to_node-as-early-v2.patch in the -mm it fix diff -puN arch/i386/pci/irq.c~x86_64-get-mp_bus_to_node-as-early-v2 arch/i386/pci/irq.c --- a/arch/i386/pci/irq.c~x86_64-get-mp_bus_to_node-as-early-v2 +++ a/arch/i386/pci/irq.c @@ -136,10 +136,26 @@ static void __init pirq_peer_trick(void) busmap[e->bus] = 1; } for(i = 1; i < 256; i++) { + struct pci_bus *bus = NULL; + struct pci_sysdata *sd; if (!busmap[i] || pci_find_bus(0, i)) continue; - if (pci_scan_bus(i, &pci_root_ops, NULL)) + /* Allocate per-root-bus (not per bus) arch-specific data. + * TODO: leak; this memory is never freed. + * It's arguable whether it's worth the trouble to care. + */ + sd = kzalloc(sizeof(*sd), GFP_KERNEL); + if (!sd) { + printk(KERN_ERR "PCI: OOM, not probing PCI bus %02x\n", + i); + continue; + } + sd->node = get_mp_bus_to_node(i); + bus = pci_scan_bus(i, &pci_root_ops, sd); + if (bus) printk(KERN_INFO "PCI: Discovered primary peer bus %02x [IRQ]\n", i); + else + kfree(sd); } pcibios_last_bus = -1; } diff -puN arch/i386/pci/legacy.c~x86_64-get-mp_bus_to_node-as-early-v2 arch/i386/pci/legacy.c --- a/arch/i386/pci/legacy.c~x86_64-get-mp_bus_to_node-as-early-v2 +++ a/arch/i386/pci/legacy.c @@ -12,6 +12,9 @@ static void __devinit pcibios_fixup_peer_bridges(void) { int n, devfn; + struct pci_bus *bus = NULL; + struct pci_sysdata *sd; + long node; if (pcibios_last_bus <= 0 || pcibios_last_bus >= 0xff) return; @@ -21,12 +24,29 @@ static void __devinit pcibios_fixup_peer u32 l; if (pci_find_bus(0, n)) continue; + node = get_mp_bus_to_node(n); for (devfn = 0; devfn < 256; devfn += 8) { if (!raw_pci_ops->read(0, n, devfn, PCI_VENDOR_ID, 2, &l) && l != 0x0000 && l != 0xffff) { DBG("Found device at %02x:%02x [%04x]\n", n, devfn, l); printk(KERN_INFO "PCI: Discovered peer bus %02x\n", n); - pci_scan_bus(n, &pci_root_ops, NULL); + /* Allocate per-root-bus (not per bus) + * arch-specific data. + * TODO: leak; this memory is never freed. + * It's arguable whether it's worth the trouble + * to care. + */ + sd = kzalloc(sizeof(*sd), GFP_KERNEL); + if (!sd) { + printk(KERN_ERR + "PCI: OOM, not probing PCI bus %02x\n", + n); + break; + } + sd->node = node; + bus = pci_scan_bus(n, &pci_root_ops, sd); + if (!bus) + kfree(sd); break; } } YH - 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/