All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gary Hade <garyhade@us.ibm.com>
To: Alex Chiang <achiang@hp.com>, Gary Hade <garyhade@us.ibm.com>,
	Matthew Wilcox <matthew@wil.cx>,
	gregkh@suse.de, kristen.c.accardi@intel.com, lenb@kernel.org,
	rick.jones2@hp.com, linux
Subject: Re: [PATCH 0/4, v3] Physical PCI slot objects
Date: Mon, 19 Nov 2007 15:32:25 -0800	[thread overview]
Message-ID: <20071119233225.GA6931@us.ibm.com> (raw)
In-Reply-To: <20071117182954.GA25003@ldl.fc.hp.com>

On Sat, Nov 17, 2007 at 11:29:54AM -0700, Alex Chiang wrote:
> Hi all,
> 
> This is v3 of the pci_slot patch series.
> 
> The major change is making the ACPI-PCI slot driver a Kconfig
> option, as per the recommendations of others (Gary, Kenji-san).

Alex, What I was trying to suggest is a boot-time kernel option, 
not a kernel configuration option.  The basic idea is to give
the user (with a single binary kernel) the ability to include
your ACPI-PCI slot driver feature changes only when they are
really needed.  In addition to reducing the number of
system/PCI hotplug driver combinations where your changes
would need to be validated, I believe would also help 
alleviate other worries (e.g. Andi Kleen's memory consumption
concern).  I believe this goal could also be achieved with the
kernel config option by making the pci_slot module runtime
loadable with the PCI hotplug drivers only visiting your new
code when the pci_slot driver is loaded, although I think this
would be more difficult to implement.

Also, I notice that even with your current CONFIG_ACPI_PCI_SLOT
implementation your numerous PCI hotplug driver changes (except
for only two places in pci_hotplug_core.c where there is 
`#ifndef CONFIG_ACPI_PCI_SLOT` and `#ifdef CONFIG_ACPI_PCI_SLOT`)
are _always_ exposed.  So, even with CONFIG_ACPI_PCI_SLOT disabled
there is IMO a need for testing of the affected PCI hotplug drivers
on more than a small number of isolated systems.

The good news is that I was able to test your v3 changes
(w/2.6.24-rc3 source) on our x3850 today with 'acpiphp'
and, except for the above mentioned inability to run-time
include/exclude them, they seemed to work fine.  The previous
boot-time ACPI error messages are gone and I was able to
successfully hot-remove and hot-add both PCI-X and PCIe
adapters.

Thanks,
Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc



WARNING: multiple messages have this Message-ID (diff)
From: Gary Hade <garyhade@us.ibm.com>
To: Alex Chiang <achiang@hp.com>, Gary Hade <garyhade@us.ibm.com>,
	Matthew Wilcox <matthew@wil.cx>,
	gregkh@suse.de, kristen.c.accardi@intel.com, lenb@kernel.org,
	rick.jones2@hp.com, linux-kernel@vger.kernel.org,
	linux-pci@atrey.karlin.mff.cuni.cz,
	kaneshige.kenji@jp.fujitsu.com,
	pcihpd-discuss@lists.sourceforge.net, linux-acpi@vger.kernel.org
Subject: Re: [PATCH 0/4, v3] Physical PCI slot objects
Date: Mon, 19 Nov 2007 15:32:25 -0800	[thread overview]
Message-ID: <20071119233225.GA6931@us.ibm.com> (raw)
In-Reply-To: <20071117182954.GA25003@ldl.fc.hp.com>

On Sat, Nov 17, 2007 at 11:29:54AM -0700, Alex Chiang wrote:
> Hi all,
> 
> This is v3 of the pci_slot patch series.
> 
> The major change is making the ACPI-PCI slot driver a Kconfig
> option, as per the recommendations of others (Gary, Kenji-san).

Alex, What I was trying to suggest is a boot-time kernel option, 
not a kernel configuration option.  The basic idea is to give
the user (with a single binary kernel) the ability to include
your ACPI-PCI slot driver feature changes only when they are
really needed.  In addition to reducing the number of
system/PCI hotplug driver combinations where your changes
would need to be validated, I believe would also help 
alleviate other worries (e.g. Andi Kleen's memory consumption
concern).  I believe this goal could also be achieved with the
kernel config option by making the pci_slot module runtime
loadable with the PCI hotplug drivers only visiting your new
code when the pci_slot driver is loaded, although I think this
would be more difficult to implement.

Also, I notice that even with your current CONFIG_ACPI_PCI_SLOT
implementation your numerous PCI hotplug driver changes (except
for only two places in pci_hotplug_core.c where there is 
`#ifndef CONFIG_ACPI_PCI_SLOT` and `#ifdef CONFIG_ACPI_PCI_SLOT`)
are _always_ exposed.  So, even with CONFIG_ACPI_PCI_SLOT disabled
there is IMO a need for testing of the affected PCI hotplug drivers
on more than a small number of isolated systems.

The good news is that I was able to test your v3 changes
(w/2.6.24-rc3 source) on our x3850 today with 'acpiphp'
and, except for the above mentioned inability to run-time
include/exclude them, they seemed to work fine.  The previous
boot-time ACPI error messages are gone and I was able to
successfully hot-remove and hot-add both PCI-X and PCIe
adapters.

Thanks,
Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc



  parent reply	other threads:[~2007-11-19 23:32 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-17 18:29 [PATCH 0/4, v3] Physical PCI slot objects Alex Chiang
2007-11-17 18:29 ` Alex Chiang
2007-11-17 18:35 ` [PATCH 1/4, v3] PCI Hotplug: Remove path attribute from sgi_hotplug Alex Chiang
2007-11-17 18:35   ` Alex Chiang
2007-11-17 18:35 ` [PATCH 2/4, v3] PCI Hotplug: Construct one fakephp slot per pci slot Alex Chiang
2007-11-17 18:35   ` Alex Chiang
2007-11-17 18:37 ` [PATCH 3/4, v3] PCI, PCI Hotplug: Introduce pci_slot Alex Chiang
2007-11-17 18:37   ` Alex Chiang
2007-11-19 22:03   ` [PATCH 3/4, v4] " Alex Chiang
2007-11-19 22:03     ` Alex Chiang
2007-11-17 18:38 ` [PATCH 4/4, v3] ACPI, PCI: ACPI PCI slot detection driver Alex Chiang
     [not found]   ` <20071119220418.GE32540@ldl.fc.hp.com>
2007-11-19 22:26     ` [PATCH 4/4, v4] " Kristen Carlson Accardi
2007-11-20  3:07       ` Alex Chiang
2007-11-20 16:23         ` stgit (was Re: [PATCH 4/4, v4] ACPI, PCI: ACPI PCI slot detection driver) Henrique de Moraes Holschuh
2007-11-17 18:38 ` [PATCH 4/4, v3] ACPI, PCI: ACPI PCI slot detection driver Alex Chiang
2007-11-19 17:43 ` [PATCH 0/4, v3] Physical PCI slot objects Kristen Carlson Accardi
2007-11-19 17:43   ` Kristen Carlson Accardi
2007-11-19 17:57   ` Alex Chiang
2007-11-19 22:02   ` Alex Chiang
2007-11-19 23:32 ` Gary Hade [this message]
2007-11-19 23:32   ` Gary Hade
2007-11-20  1:33   ` Kenji Kaneshige
2007-11-20  2:04   ` Matthew Garrett
2007-11-20 19:53     ` Gary Hade
2007-11-26 22:22   ` Alex Chiang
2007-11-26 22:26     ` [PATCH 3/4, v5] PCI, PCI Hotplug: Introduce pci_slot Alex Chiang
2007-11-26 22:28     ` [PATCH 4/4, v5] ACPI, PCI: ACPI PCI slot detection driver Alex Chiang
2007-11-27  3:04     ` [PATCH 0/4, v3] Physical PCI slot objects Gary Hade
2007-11-27 19:11       ` Kristen Carlson Accardi
2007-11-27 19:11         ` Kristen Carlson Accardi
2007-11-28 21:31         ` Gary Hade
2007-11-29  0:02           ` Kristen Carlson Accardi
2007-11-29  0:02             ` Kristen Carlson Accardi
2007-11-29  1:09             ` Gary Hade
2007-11-30  1:19           ` Alex Chiang
2007-11-30 19:10             ` Gary Hade
2007-11-29  7:47     ` Kenji Kaneshige
2007-11-29  7:47       ` Kenji Kaneshige
2007-11-30  1:51       ` Alex Chiang
2007-12-03  3:30         ` Kenji Kaneshige
2007-12-03 22:43           ` Alex Chiang
2007-12-04 12:57             ` Kenji Kaneshige
2007-12-04 12:57               ` Kenji Kaneshige
2007-12-10 23:02               ` Alex Chiang
2008-03-11 19:15       ` Kristen Carlson Accardi
2008-03-11 19:15         ` Kristen Carlson Accardi
2008-03-12 12:25         ` Kenji Kaneshige

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20071119233225.GA6931@us.ibm.com \
    --to=garyhade@us.ibm.com \
    --cc=achiang@hp.com \
    --cc=gregkh@suse.de \
    --cc=kristen.c.accardi@intel.com \
    --cc=lenb@kernel.org \
    --cc=matthew@wil.cx \
    --cc=rick.jones2@hp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.