All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.