* [git pull] PCI pull request for 2.6.27
@ 2008-07-16 23:39 Jesse Barnes
2008-07-16 23:50 ` Linus Torvalds
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Jesse Barnes @ 2008-07-16 23:39 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-pci@vger.kernel.org
Please consider pulling my linux-next branch:
git pull git://git.kernel.org/pub/scm/linux/kernel/git/jbarnes/pci-2.6.git linux-next
I'm open to suggestions here about a couple of caveats with this pull:
- it's not merged up to master (though the conflicts are fairly trivial,
you can check out my linux-next-merge branch to see how I handled them)
- it contains a revert for a patch I thought might be ready but have since
chickened out on. I didn't want to rebase since I'm carrying some
changes from the ACPI tree
But if you're ok with that, please pull away. Diffstat & shortlog below.
Note that the diffstat is actually from my linux-next-merge tree since my
master branch is up-to-date but my linux-next branch isn't, but it should
be pretty close to what you end up with after you merge.
Thanks,
Jesse
Adrian Bunk (4):
PCI: make {pciehp,shpchp}_slot_with_bus static
PCI: drivers/pci/pci.c: add prototypes
PCI: remove CVS keywords
PCI: remove unused arch pcibios_update_resource() functions
Alex Chiang (4):
PCIe: fix 'symbol not declared' sparse warnings
PCI: construct one fakephp slot per PCI slot
PCI: introduce pci_slot
PCI: ACPI PCI slot detection driver
Alok Kataria (1):
x86/PCI: ACPI based PCI gap calculation
Bjorn Helgaas (3):
PCI: use dev_printk when possible
PCIE: aer: use dev_printk when possible
PCIE: port driver: use dev_printk when possible
David Brownell (1):
ACPI PM: acpi_pm_device_sleep_state() cleanup
David Howells (2):
PCI: fix pci_setup_device()'s sprinting into a const buffer
Fix acpi_pm_device_sleep_wake() by providing a stub for CONFIG_PM_SLEEP=n
Gary Hade (1):
PCI: boot parameter to avoid expansion ROM memory allocation
Greg Kroah-Hartman (2):
PCI: handle pci_name() being const
PCI: make pci_name use dev_name
Hidetoshi Seto (1):
PCI/MSI: skip calling pci_find_capability from msi_set_mask_bits
Jean Delvare (1):
PCI: Unhide the SMBus on the Compaq Evo D510
Jesse Barnes (6):
PCI: update location of PCI hotplug mailing lists
Merge branch 'suspend' of git://git.kernel.org/.../lenb/linux-acpi-2.6 into linux-next
x86/PCI: fixup early quirk probing
PCI: fixup kdoc blocks for hotplug functions
PCI: correct resource number in debug output
Revert "x86/PCI: ACPI based PCI gap calculation"
Julia Lawall (1):
PCI: eliminate double kfree in intel-iommu initialization
Kenji Kaneshige (19):
pci-acpi: remove duplicate code for _OSC
pci-acpi: use local buffer for _OSC
pci-acpi: add flag to indicate query had been done
pci-acpi: remove unused variable in __pci_osc_support_set
pci-acpi: formatting cleanups for _OSC
pciehp: remove redundant pci_dev initialization
pciehp: evaluate _OSC/OSHP before controller init
pciehp: block signals while waiting for command completion
shpchp: check firmware before taking control
pciehp: fixes typo in dbg_ctrl() in pciehp_hpc.c
pciehp: removes redundant NULL write to slot status register
pciehp: fix typo in hpc_release_ctlr
pciehp: cleanup pcie_poll_cmd
pciehp: change command polling frequency
pciehp: remove inline from command related functions
Remove unnecessary 'tmp' variable from pci_hp_register().
pciehp: fix interrupt initialization
pciehp: remove needless command completed interrupt setting
pciehp: use get_service_data
Krzysztof Helt (1):
PCI: unhide the SMBus on the Compaq Deskpro EN
Matthew Wilcox (1):
x86/PCI: Fix PCI config space for domains > 0
Miklos Vajna (1):
x86/PCI: janitor work in irq.c
Ming Lei (1):
PCI: remove unnecessary volatile in PCIe hotplug struct controller
Rafael J. Wysocki (16):
Suspend/Resume bug in PCI layer wrt quirks
Introduce new top level suspend and hibernation callbacks
PCI: implement new suspend/resume callbacks
Implement new suspend and hibernation callbacks for platform busses
PCI ACPI: Drop the second argument of platform_pci_choose_state
ACPI PM: Remove obsolete Toshiba workaround
Suspend-related patches for 2.6.27
ACPI: Introduce acpi_bus_power_manageable function
PCI: Introduce platform_pci_power_manageable function
PCI: rework pci_set_power_state function to call platform first
ACPI: Introduce acpi_device_sleep_wake function
ACPI: Introduce new device wakeup flag 'prepared'
PCI ACPI: Rework PCI handling of wake-up
PCI PM: Introduce pci_prepare_to_sleep and pci_back_from_sleep
PCI: Simplify PCI device PM code
PCI PM: Fix pci_prepare_to_sleep
Randy Dunlap (1):
PCI: add stub for pci_set_consistent_dma_mask()
Robert P. J. Day (1):
PCI: Replace deprecated __initcall with device_initcall.
Stephen Rothwell (1):
PCI: include linux/pm_wakeup.h for device_set_wakeup_capable
Wang Chen (1):
PCI: Fix comment of pci_dynids
Yinghai Lu (2):
PCI/x86: write_pci_config_byte fix offset
PCI/x86: early dump pci conf space v2
Zhao Yakui (1):
PCI: Disable PME during PCI scan
Documentation/kernel-parameters.txt | 9 +-
MAINTAINERS | 12 +-
arch/frv/mb93090-mb00/pci-frv.c | 30 -
arch/m68knommu/kernel/comempci.c | 9 -
arch/mips/pmc-sierra/yosemite/ht.c | 36 --
arch/sh/drivers/pci/pci.c | 32 --
arch/sparc64/kernel/pci.c | 2 +-
arch/x86/kernel/acpi/sleep.c | 2 +
arch/x86/kernel/apm_32.c | 8 +-
arch/x86/kernel/early-quirks.c | 26 +-
arch/x86/kernel/setup.c | 5 +
arch/x86/pci/common.c | 30 +-
arch/x86/pci/early.c | 60 ++-
arch/x86/pci/irq.c | 258 ++++++---
arch/x86/pci/pci.h | 3 +-
drivers/acpi/Kconfig | 9 +
drivers/acpi/Makefile | 1 +
drivers/acpi/bus.c | 22 +
drivers/acpi/glue.c | 2 +
drivers/acpi/pci_slot.c | 368 ++++++++++++
drivers/acpi/power.c | 138 ++++--
drivers/acpi/scan.c | 42 +--
drivers/acpi/sleep/main.c | 328 +++++++-----
drivers/acpi/sleep/wakeup.c | 13 +-
drivers/base/platform.c | 296 ++++++++++-
drivers/base/power/main.c | 675 ++++++++++++++++++-----
drivers/base/power/power.h | 2 +-
drivers/base/power/sysfs.c | 3 -
drivers/base/power/trace.c | 4 +-
drivers/pci/Makefile | 2 +-
drivers/pci/hotplug/acpi_pcihp.c | 85 +++-
drivers/pci/hotplug/acpiphp.h | 1 -
drivers/pci/hotplug/acpiphp_core.c | 25 +-
drivers/pci/hotplug/acpiphp_glue.c | 23 +-
drivers/pci/hotplug/acpiphp_ibm.c | 6 +-
drivers/pci/hotplug/cpci_hotplug_core.c | 2 +-
drivers/pci/hotplug/cpqphp_core.c | 4 +-
drivers/pci/hotplug/fakephp.c | 86 +--
drivers/pci/hotplug/ibmphp_ebda.c | 3 +-
drivers/pci/hotplug/pci_hotplug_core.c | 284 ++++------
drivers/pci/hotplug/pciehp.h | 16 +-
drivers/pci/hotplug/pciehp_core.c | 127 ++----
drivers/pci/hotplug/pciehp_hpc.c | 318 +++++------
drivers/pci/hotplug/rpadlpar_sysfs.c | 5 +-
drivers/pci/hotplug/rpaphp_slot.c | 44 +--
drivers/pci/hotplug/sgi_hotplug.c | 12 +-
drivers/pci/hotplug/shpchp.h | 14 +-
drivers/pci/hotplug/shpchp_core.c | 37 +-
drivers/pci/hotplug/shpchp_hpc.c | 1 -
drivers/pci/intel-iommu.c | 1 -
drivers/pci/msi.c | 22 +-
drivers/pci/pci-acpi.c | 277 +++++-----
drivers/pci/pci-driver.c | 388 ++++++++++++--
drivers/pci/pci.c | 479 ++++++++++++-----
drivers/pci/pci.h | 48 ++-
drivers/pci/pcie/aer/aerdrv.c | 9 +-
drivers/pci/pcie/aer/aerdrv_acpi.c | 8 +-
drivers/pci/pcie/aer/aerdrv_core.c | 24 +-
drivers/pci/pcie/portdrv_bus.c | 1 +
drivers/pci/pcie/portdrv_core.c | 22 +-
drivers/pci/pcie/portdrv_pci.c | 5 +-
drivers/pci/probe.c | 38 +-
drivers/pci/proc.c | 4 +-
drivers/pci/quirks.c | 134 ++++--
drivers/pci/setup-bus.c | 43 +-
drivers/pci/setup-irq.c | 3 +-
drivers/pci/setup-res.c | 70 ++--
drivers/pci/slot.c | 233 ++++++++
drivers/pnp/pnpacpi/core.c | 4 +-
include/acpi/acpi_bus.h | 12 +-
include/acpi/acpi_drivers.h | 4 +-
include/asm-generic/vmlinux.lds.h | 6 +
include/asm-x86/pci-direct.h | 4 +
include/linux/acpi.h | 3 +
include/linux/device.h | 9 +
include/linux/pci.h | 57 ++-
include/linux/pci_hotplug.h | 14 +-
include/linux/pci_regs.h | 1 +
include/linux/platform_device.h | 1 +
include/linux/pm.h | 314 ++++++++++-
include/linux/pm_wakeup.h | 28 +-
include/linux/suspend.h | 14 +-
kernel/power/disk.c | 50 ++-
kernel/power/main.c | 16 +-
lib/kobject.c | 1 +
85 files changed, 4061 insertions(+), 1806 deletions(-)
create mode 100644 drivers/acpi/pci_slot.c
create mode 100644 drivers/pci/slot.c
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [git pull] PCI pull request for 2.6.27
2008-07-16 23:39 [git pull] PCI pull request for 2.6.27 Jesse Barnes
@ 2008-07-16 23:50 ` Linus Torvalds
2008-07-16 23:54 ` Jesse Barnes
2008-07-17 0:28 ` Linus Torvalds
2008-07-21 13:57 ` [patch] acpi: fix crash in core ACPI code, triggered by CONFIG_ACPI_PCI_SLOT=y Ingo Molnar
2 siblings, 1 reply; 8+ messages in thread
From: Linus Torvalds @ 2008-07-16 23:50 UTC (permalink / raw)
To: Jesse Barnes; +Cc: linux-kernel, linux-pci@vger.kernel.org
On Wed, 16 Jul 2008, Jesse Barnes wrote:
>
> Miklos Vajna (1):
> x86/PCI: janitor work in irq.c
Please don't take patches like this.
If it's janitor work, the end result should be better. But it's not. This
patch is full of stuff like
- for(addr = (u8 *) __va(0xf0000); addr < (u8 *) __va(0x100000); addr += 16) {
+ for (addr = (u8 *) __va(0xf0000); addr < (u8 *) __va(0x100000);
+ addr += 16) {
rt = pirq_check_routing_table(addr);
Which just brings negative value. The code is _harder_ to look at, not
easier.
The 80-character limit is less important than making code look obvious and
indentation being readable. Splitting the for(;;) loop just made the
indentation look like total crap.
I'm fixing it up (since it also caused trivial conflicts), but I'd ask
people to just ignore that sh*t-for-brains that is the long-line warning
when trying to fix it may silence a warning, but results in worse code!
Linus
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [git pull] PCI pull request for 2.6.27
2008-07-16 23:50 ` Linus Torvalds
@ 2008-07-16 23:54 ` Jesse Barnes
2008-07-17 0:23 ` Maciej W. Rozycki
0 siblings, 1 reply; 8+ messages in thread
From: Jesse Barnes @ 2008-07-16 23:54 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-pci@vger.kernel.org
On Wednesday, July 16, 2008 4:50 pm Linus Torvalds wrote:
> On Wed, 16 Jul 2008, Jesse Barnes wrote:
> > Miklos Vajna (1):
> > x86/PCI: janitor work in irq.c
>
> Please don't take patches like this.
>
> If it's janitor work, the end result should be better. But it's not. This
> patch is full of stuff like
>
> - for(addr = (u8 *) __va(0xf0000); addr < (u8 *) __va(0x100000); addr +=
> 16) { + for (addr = (u8 *) __va(0xf0000); addr < (u8 *) __va(0x100000);
> + addr += 16) {
> rt = pirq_check_routing_table(addr);
>
> Which just brings negative value. The code is _harder_ to look at, not
> easier.
>
> The 80-character limit is less important than making code look obvious and
> indentation being readable. Splitting the for(;;) loop just made the
> indentation look like total crap.
>
> I'm fixing it up (since it also caused trivial conflicts), but I'd ask
> people to just ignore that sh*t-for-brains that is the long-line warning
> when trying to fix it may silence a warning, but results in worse code!
Yeah I noticed that too when I did the merge (and in the original patch),
though on my 80 char display it looked like a wash in terms of readability to
me, but even in that case I should have rejected it as noise. Sorry about
that.
Jesse
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [git pull] PCI pull request for 2.6.27
2008-07-16 23:54 ` Jesse Barnes
@ 2008-07-17 0:23 ` Maciej W. Rozycki
2008-07-17 0:38 ` Linus Torvalds
0 siblings, 1 reply; 8+ messages in thread
From: Maciej W. Rozycki @ 2008-07-17 0:23 UTC (permalink / raw)
To: Jesse Barnes; +Cc: Linus Torvalds, linux-kernel, linux-pci@vger.kernel.org
On Wed, 16 Jul 2008, Jesse Barnes wrote:
> Yeah I noticed that too when I did the merge (and in the original patch),
> though on my 80 char display it looked like a wash in terms of readability to
> me, but even in that case I should have rejected it as noise. Sorry about
> that.
Conveniently "for" is short enough for indentation like this:
for (addr = (u8 *) __va(0xf0000);
addr < (u8 *) __va(0x100000);
addr += 16) {
rt = pirq_check_routing_table(addr);
to make the piece of code both well-readable and polite to the right edge
of the terminal. That would be my preference -- I hope you agree.
Of course, in many cases it may be better to make use of auxiliary
variables instead. GCC has to evaluate the expressions used anyway and
any names given to them come for free for any half-decent version of the
compiler.
Maciej
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [git pull] PCI pull request for 2.6.27
2008-07-16 23:39 [git pull] PCI pull request for 2.6.27 Jesse Barnes
2008-07-16 23:50 ` Linus Torvalds
@ 2008-07-17 0:28 ` Linus Torvalds
2008-07-21 13:57 ` [patch] acpi: fix crash in core ACPI code, triggered by CONFIG_ACPI_PCI_SLOT=y Ingo Molnar
2 siblings, 0 replies; 8+ messages in thread
From: Linus Torvalds @ 2008-07-17 0:28 UTC (permalink / raw)
To: Jesse Barnes; +Cc: linux-kernel, linux-pci@vger.kernel.org
On Wed, 16 Jul 2008, Jesse Barnes wrote:
>
> I'm open to suggestions here about a couple of caveats with this pull:
> - it's not merged up to master (though the conflicts are fairly trivial,
> you can check out my linux-next-merge branch to see how I handled them)
Ok, I handled them without checking your branch, just because I actually
like feeling like I know what I'm doing.
That said, when I inevitably fail, just humor me, and send me a patch to
fix it up, mentioning how I missed a really "subtle" thing, and the fact
that it didn't even compile wasn't really my fault.
I'm special. My mom told me so. And I want the re-assurance.
> - it contains a revert for a patch I thought might be ready but have since
> chickened out on. I didn't want to rebase since I'm carrying some
> changes from the ACPI tree
That's fine.
But do double-check the end result. There's lots of small details (for
example, I did try to just undo the damage from the "overlong lines caused
somebody to violate all the _other_ coding style rules" patch), but this
code from setup_64.c:
+#ifdef CONFIG_PCI
+ if (pci_early_dump_regs)
+ early_dump_pci_devices();
+#endif
that your branch had added I put in the new shared 'setup.c', and while I
think I put it in the right place, somebody needs to double-check it.
Anyway, I've committed what I think is the right resolve, and it's
compiled etc, but I want to reboot it before I push out (so if there is
something seriously broken I can just holler for help or perhaps try to
fixure it out myself before publicising it), so it's not there yet.
Linus
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [git pull] PCI pull request for 2.6.27
2008-07-17 0:23 ` Maciej W. Rozycki
@ 2008-07-17 0:38 ` Linus Torvalds
0 siblings, 0 replies; 8+ messages in thread
From: Linus Torvalds @ 2008-07-17 0:38 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Jesse Barnes, linux-kernel, linux-pci@vger.kernel.org
On Thu, 17 Jul 2008, Maciej W. Rozycki wrote:
>
> Conveniently "for" is short enough for indentation like this:
>
> for (addr = (u8 *) __va(0xf0000);
> addr < (u8 *) __va(0x100000);
> addr += 16) {
> rt = pirq_check_routing_table(addr);
I don't actually like that one very much either.
It's perfectly readable when looking at things closely, but it's not very
nice when quickly "scanning" code visually. It looks like two separate
indents.
Btw, that "code scanning" is not necessarily a bad idea. It's actually
pretty interesting to print code out in a 2-point font (or just open a
terminal and do "ctrl -" several times to make the code basically
unreadable). See if the code flow makes sense from 10,000 feet - you can
pick up overlong functions and various other dubious practices really
clearly (#ifdef's in code etc).
(IOW, the whole point of the exercise is to _not_ be able to actually read
the code, but just look at the _shape_ of it).
Btw, that commit also did things like change the coding style to a
non-kernel coding style by changing
static int function(xyz..)
to
static int
function(xyz..)
just to make lines shorter. Again - introducing bigger problems than it
actually fixes.
Linus
^ permalink raw reply [flat|nested] 8+ messages in thread
* [patch] acpi: fix crash in core ACPI code, triggered by CONFIG_ACPI_PCI_SLOT=y
2008-07-16 23:39 [git pull] PCI pull request for 2.6.27 Jesse Barnes
2008-07-16 23:50 ` Linus Torvalds
2008-07-17 0:28 ` Linus Torvalds
@ 2008-07-21 13:57 ` Ingo Molnar
2008-07-21 18:38 ` Andi Kleen
2 siblings, 1 reply; 8+ messages in thread
From: Ingo Molnar @ 2008-07-21 13:57 UTC (permalink / raw)
To: Jesse Barnes
Cc: Linus Torvalds, linux-kernel, linux-pci@vger.kernel.org,
Len Brown, ak, Alex Chiang, Kenji Kaneshige, Andrew Morton
-tip testing found the following boot crash on 32-bit x86 (Core2Duo
laptop) yesterday:
[ 5.606664] scsi4 : ata_piix
[ 5.606664] scsi5 : ata_piix
[ 5.606664] ACPI Error (psargs-0358): [\_SB_.PCI0.LPC_.EC__.BSTA] Namespace lookup failure, AE_NOT_FOUND
[ 5.606664] ACPI Error (psparse-0530): ACPI Error (nsnames-0186): Invalid NS Node (f7c0e960) while traversing path [20080609]
[ 5.606664] BUG: unable to handle kernel NULL pointer dereference at 0000000f
[ 5.606664] IP: [<80339e2f>] acpi_ns_build_external_path+0x1f/0x80
[ 5.609997] *pdpt = 0000000000a03001 *pde = 0000000000000000
[ 5.609997] Oops: 0002 [#1] SMP
[ 5.609997]
[ 5.609997] Pid: 1, comm: swapper Not tainted (2.6.26-tip-03965-gbbfb62e-dirty #3153)
[ 5.609997] EIP: 0060:[<80339e2f>] EFLAGS: 00010286 CPU: 0
[ 5.609997] EIP is at acpi_ns_build_external_path+0x1f/0x80
[ 5.609997] EAX: f7c18c18 EBX: ffffffff ECX: 00000010 EDX: 00000000
[ 5.609997] ESI: f7c18c18 EDI: 00000010 EBP: f7c4dc28 ESP: f7c4dc18
[ 5.609997] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
[ 5.609997] Process swapper (pid: 1, ti=f7c4c000 task=f7c50000 task.ti=f7c4c000)
[ 5.609997] Stack: 00000000 00000000 f7c18c18 f7c4dc48 f7c4dc40 80339ed0 00000000 f7c18c18
[ 5.609997] 8084c1b6 8084c1b6 f7c4dc58 8033a60a 00000000 00000010 00000000 f7c18c18
[ 5.609997] f7c4dc70 8033a68f f7c18c18 00000000 f6de7600 00000005 f7c4dc98 8033c34d
[ 5.609997] Call Trace:
[ 5.609997] [<80339ed0>] ? acpi_ns_handle_to_pathname+0x40/0x72
[ 5.609997] [<8033a60a>] ? acpi_ns_print_node_pathname+0x2c/0x61
[ 5.609997] [<8033a68f>] ? acpi_ns_report_method_error+0x50/0x6d
[ 5.609997] [<8033c34d>] ? acpi_ps_parse_aml+0x149/0x2f9
[ 5.609997] [<8033d6dd>] ? acpi_ps_execute_method+0x132/0x201
[ 5.609997] [<80339d19>] ? acpi_ns_evaluate+0x1ad/0x258
[ 5.609997] [<803406c4>] ? acpi_ut_evaluate_object+0x55/0x18f
[ 5.609997] [<803408b7>] ? acpi_ut_execute_STA+0x22/0x7a
[ 5.609997] [<8033a907>] ? acpi_get_object_info+0x131/0x1be
[ 5.609997] [<80344bb2>] ? do_acpi_find_child+0x22/0x4b
[ 5.609997] [<8033b855>] ? acpi_ns_walk_namespace+0xa5/0x124
[ 5.609997] [<803394f3>] ? acpi_walk_namespace+0x54/0x74
[ 5.609997] [<80344b90>] ? do_acpi_find_child+0x0/0x4b
[ 5.609997] [<80344b85>] ? acpi_get_child+0x38/0x43
[ 5.609997] [<80344b90>] ? do_acpi_find_child+0x0/0x4b
[ 5.609997] [<804d0148>] ? ata_acpi_associate+0xb5/0x1b5
[ 5.609997] [<804c6ecb>] ? ata_scsi_add_hosts+0x8e/0xdc
[ 5.609997] [<804c40c8>] ? ata_host_register+0x9f/0x1d6
[ 5.609997] [<804cbc7f>] ? ata_pci_sff_activate_host+0x179/0x19f
[ 5.609997] [<804cdd45>] ? ata_sff_interrupt+0x0/0x1c7
[ 5.609997] [<8069b033>] ? piix_init_one+0x569/0x5b0
[ 5.609997] [<801bd400>] ? sysfs_ilookup_test+0x0/0x11
[ 5.609997] [<801987d7>] ? ilookup5_nowait+0x29/0x30
[ 5.609997] [<802efc7e>] ? pci_match_device+0x99/0xa3
[ 5.609997] [<802efd3c>] ? pci_device_probe+0x39/0x59
[ 5.609997] [<803bc4af>] ? driver_probe_device+0xa0/0x11b
[ 5.609997] [<803bc564>] ? __driver_attach+0x3a/0x59
[ 5.609997] [<803bbde3>] ? bus_for_each_dev+0x36/0x58
[ 5.609997] [<803bc354>] ? driver_attach+0x14/0x16
[ 5.609997] [<803bc52a>] ? __driver_attach+0x0/0x59
[ 5.609997] [<803bc161>] ? bus_add_driver+0x93/0x196
[ 5.609997] [<803bc773>] ? driver_register+0x71/0xcd
[ 5.609997] [<802eff05>] ? __pci_register_driver+0x3f/0x6e
[ 5.609997] [<809af7ff>] ? piix_init+0x14/0x24
[ 5.609997] [<80984568>] ? kernel_init+0x128/0x269
[ 5.609997] [<809af7eb>] ? piix_init+0x0/0x24
[ 5.609997] [<802e2758>] ? trace_hardirqs_on_thunk+0xc/0x10
[ 5.609997] [<80116aef>] ? restore_nocheck_notrace+0x0/0xe
[ 5.609997] [<80984440>] ? kernel_init+0x0/0x269
[ 5.609997] [<80984440>] ? kernel_init+0x0/0x269
[ 5.609997] [<80117d87>] ? kernel_thread_helper+0x7/0x10
[ 5.609997] =======================
[ 5.609997] Code: 75 02 b3 01 8d 43 01 8b 5d fc c9 c3 55 89 e5 57 89 cf 56 53 89 d3 4b 83 ec 04 83 fb 03 89 55 f0 77 09 c6 01 5c c6 41 01 00 eb 59 <c6> 04 19 00 8b 55 f0 8d 34 11 89 c2 eb 19 8b 42 08 83 eb 05 89
[ 5.609997] EIP: [<80339e2f>] acpi_ns_build_external_path+0x1f/0x80 SS:ESP 0068:f7c4dc18
[ 5.613331] Kernel panic - not syncing: Fatal exception
[ 5.613331] Rebooting in 1 seconds..[ 4.646664] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
I have bisected it down to:
# bad: [5b664cbe] Merge branch 'upstream-linus' of git://git.kernel.
# good: [bce7f795] Linux 2.6.26
# good: [e18425ab] Merge branch 'tracing/for-linus' of git://git.kern
# good: [cadc7236] Merge branch 'bkl-removal' into next
# good: [4515889a] Merge branch 'merge' of git://git.kernel.org/pub/s
# good: [42fdd14e] Merge git://git.kernel.org/pub/scm/linux/kernel/gi
# good: [8a0ca91f] Merge branch 'for-linus' of git://git.kernel.org/p
# bad: [0af4b8cb] ACPI: Introduce new device wakeup flag 'prepared'
# good: [fe997407] PCI: construct one fakephp slot per PCI slot
# bad: [531f254a] PCIE: aer: use dev_printk when possible
# bad: [15650a20] x86/PCI: fixup early quirk probing
# good: [0e6859d9] ACPI PM: Remove obsolete Toshiba workaround
# bad: [8344b566] PCI: ACPI PCI slot detection driver
# good: [f46753c9] PCI: introduce pci_slot
| 8344b568f5bdc7ee1bba909de3294c6348c36056 is first bad commit
| commit 8344b568f5bdc7ee1bba909de3294c6348c36056
| Author: Alex Chiang <achiang@hp.com>
| Date: Tue Jun 10 15:30:42 2008 -0600
|
| PCI: ACPI PCI slot detection driver
|
| Detect all physical PCI slots as described by ACPI, and create entries in
| /sys/bus/pci/slots/.
I.e. the new CONFIG_ACPI_PCI_SLOT=y option was causing this crash.
But the bug is not mainly in this new PCI code - that code was just
hitting the ACPI code in a new way which made ACPI break.
The crash signature shows that we are crashing on this instruction:
movb $0x0, (%ecx, %ebx, 1)
ECX and EBX are 0x10 and -1. It's this line in
drivers/acpi/namespace/nsnames.c's acpi_ns_build_external_path():
name_buffer[index] = 0;
I.e. name_buffer is 0x10 and index is -1.
index -1 corresponds to size 0, and name_buffer 0x10 is slab's
ZERO_SIZE_PTR special-case for zero-sized allocations.
I.e. when we called acpi_ns_handle_to_pathname(), we got required_size
of 0 due to an error condition, but this is passed to the ACPI allocator
unconditionally:
required_size = acpi_ns_get_pathname_length(node);
/* Validate/Allocate/Clear caller buffer */
status = acpi_ut_initialize_buffer(buffer, required_size);
if (ACPI_FAILURE(status)) {
return_ACPI_STATUS(status);
}
Where acpi_ut_initialize_buffer(), through many (unnecessary) layers,
ends up calling kzalloc(0). Which returns 0x10 and that then causes the
crash later on.
So fix both callers of acpi_ns_get_pathname_length(), which can return 0
in case of an invalid node.
Also add a WARN_ON() against zero sized allocations in
acpi_ut_initialize_buffer() to make it easier to find similar instances
of this bug.
I have tested this patch for the past 24 hours and the crash has not
reappeared.
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
drivers/acpi/namespace/nsnames.c | 8 ++++++++
drivers/acpi/utilities/utalloc.c | 4 ++++
2 files changed, 12 insertions(+)
Index: tip/drivers/acpi/namespace/nsnames.c
===================================================================
--- tip.orig/drivers/acpi/namespace/nsnames.c
+++ tip/drivers/acpi/namespace/nsnames.c
@@ -137,6 +137,10 @@ char *acpi_ns_get_external_pathname(stru
/* Calculate required buffer size based on depth below root */
size = acpi_ns_get_pathname_length(node);
+ if (!size) {
+ ACPI_ERROR((AE_INFO, "Invalid node failure"));
+ return_PTR(NULL);
+ }
/* Allocate a buffer to be returned to caller */
@@ -229,6 +233,10 @@ acpi_ns_handle_to_pathname(acpi_handle t
/* Determine size required for the caller buffer */
required_size = acpi_ns_get_pathname_length(node);
+ if (!required_size) {
+ ACPI_ERROR((AE_INFO, "Invalid node failure"));
+ return_ACPI_STATUS(AE_ERROR);
+ }
/* Validate/Allocate/Clear caller buffer */
Index: tip/drivers/acpi/utilities/utalloc.c
===================================================================
--- tip.orig/drivers/acpi/utilities/utalloc.c
+++ tip/drivers/acpi/utilities/utalloc.c
@@ -242,6 +242,10 @@ acpi_ut_initialize_buffer(struct acpi_bu
{
acpi_status status = AE_OK;
+ if (!required_length) {
+ WARN_ON(1);
+ return AE_ERROR;
+ }
switch (buffer->length) {
case ACPI_NO_BUFFER:
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch] acpi: fix crash in core ACPI code, triggered by CONFIG_ACPI_PCI_SLOT=y
2008-07-21 13:57 ` [patch] acpi: fix crash in core ACPI code, triggered by CONFIG_ACPI_PCI_SLOT=y Ingo Molnar
@ 2008-07-21 18:38 ` Andi Kleen
0 siblings, 0 replies; 8+ messages in thread
From: Andi Kleen @ 2008-07-21 18:38 UTC (permalink / raw)
To: Ingo Molnar
Cc: Jesse Barnes, Linus Torvalds, linux-kernel,
linux-pci@vger.kernel.org, Len Brown, robert.moore, Alex Chiang,
Kenji Kaneshige, Andrew Morton
Ingo Molnar wrote:
[For acpica issues please always put robert.moore@intel.com into Cc. Thanks]
[Bob, will need to resync this into ACPICA]
> I.e. when we called acpi_ns_handle_to_pathname(), we got required_size
> of 0 due to an error condition, but this is passed to the ACPI allocator
> unconditionally:
>
> required_size = acpi_ns_get_pathname_length(node);
>
> /* Validate/Allocate/Clear caller buffer */
>
> status = acpi_ut_initialize_buffer(buffer, required_size);
> if (ACPI_FAILURE(status)) {
> return_ACPI_STATUS(status);
> }
>
> Where acpi_ut_initialize_buffer(), through many (unnecessary) layers,
> ends up calling kzalloc(0). Which returns 0x10 and that then causes the
> crash later on.
>
> So fix both callers of acpi_ns_get_pathname_length(), which can return 0
> in case of an invalid node.
>
> Also add a WARN_ON() against zero sized allocations in
> acpi_ut_initialize_buffer() to make it easier to find similar instances
> of this bug.
>
> I have tested this patch for the past 24 hours and the crash has not
> reappeared.
Thanks for the detailed analysis and the patch. Patch looks mostly good,
except that the error message could be a little clearer ("Invalid zero
length node string" perhaps?). It'll need to be backmerged into
the ACPICA code base, but that can be done in parallel without
delaying fixing this bug.
I can add it to my next batch your or you can add it one of yours
if you prefer.
-Andi
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-07-21 18:38 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-16 23:39 [git pull] PCI pull request for 2.6.27 Jesse Barnes
2008-07-16 23:50 ` Linus Torvalds
2008-07-16 23:54 ` Jesse Barnes
2008-07-17 0:23 ` Maciej W. Rozycki
2008-07-17 0:38 ` Linus Torvalds
2008-07-17 0:28 ` Linus Torvalds
2008-07-21 13:57 ` [patch] acpi: fix crash in core ACPI code, triggered by CONFIG_ACPI_PCI_SLOT=y Ingo Molnar
2008-07-21 18:38 ` Andi Kleen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.