From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org, Gavin Shan <gwshan@linux.vnet.ibm.com>,
Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>,
Andrew Donnellan <andrew.donnellan@au1.ibm.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>
Subject: [PATCH 4.7 01/31] powerpc/pseries: use pci_host_bridge.release_fn() to kfree(phb)
Date: Fri, 14 Oct 2016 14:54:45 +0200 [thread overview]
Message-ID: <20161014122715.295287333@linuxfoundation.org> (raw)
In-Reply-To: <20161014122715.235592611@linuxfoundation.org>
4.7-stable review patch. If anyone has any objections, please let me know.
------------------
From: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
commit 2dd9c11b9d4dfbd6c070eab7b81197f65e82f1a0 upstream.
This patch leverages 'struct pci_host_bridge' from the PCI subsystem
in order to free the pci_controller only after the last reference to
its devices is dropped (avoiding an oops in pcibios_release_device()
if the last reference is dropped after pcibios_free_controller()).
The patch relies on pci_host_bridge.release_fn() (and .release_data),
which is called automatically by the PCI subsystem when the root bus
is released (i.e., the last reference is dropped). Those fields are
set via pci_set_host_bridge_release() (e.g. in the platform-specific
implementation of pcibios_root_bridge_prepare()).
It introduces the 'pcibios_free_controller_deferred()' .release_fn()
and it expects .release_data to hold a pointer to the pci_controller.
The function implictly calls 'pcibios_free_controller()', so an user
must *NOT* explicitly call it if using the new _deferred() callback.
The functionality is enabled for pseries (although it isn't platform
specific, and may be used by cxl).
Details on not-so-elegant design choices:
- Use 'pci_host_bridge.release_data' field as pointer to associated
'struct pci_controller' so *not* to 'pci_bus_to_host(bridge->bus)'
in pcibios_free_controller_deferred().
That's because pci_remove_root_bus() sets 'host_bridge->bus = NULL'
(so, if the last reference is released after pci_remove_root_bus()
runs, which eventually reaches pcibios_free_controller_deferred(),
that would hit a null pointer dereference).
The cxl/vphb.c code calls pci_remove_root_bus(), and the cxl folks
are interested in this fix.
Test-case #1 (hold references)
# ls -ld /sys/block/sd* | grep -m1 0021:01:00.0
<...> /sys/block/sdaa -> ../devices/pci0021:01/0021:01:00.0/<...>
# ls -ld /sys/block/sd* | grep -m1 0021:01:00.1
<...> /sys/block/sdab -> ../devices/pci0021:01/0021:01:00.1/<...>
# cat >/dev/sdaa & pid1=$!
# cat >/dev/sdab & pid2=$!
# drmgr -w 5 -d 1 -c phb -s 'PHB 33' -r
Validating PHB DLPAR capability...yes.
[ 594.306719] pci_hp_remove_devices: PCI: Removing devices on bus 0021:01
[ 594.306738] pci_hp_remove_devices: Removing 0021:01:00.0...
...
[ 598.236381] pci_hp_remove_devices: Removing 0021:01:00.1...
...
[ 611.972077] pci_bus 0021:01: busn_res: [bus 01-ff] is released
[ 611.972140] rpadlpar_io: slot PHB 33 removed
# kill -9 $pid1
# kill -9 $pid2
[ 632.918088] pcibios_free_controller_deferred: domain 33, dynamic 1
Test-case #2 (don't hold references)
# drmgr -w 5 -d 1 -c phb -s 'PHB 33' -r
Validating PHB DLPAR capability...yes.
[ 916.357363] pci_hp_remove_devices: PCI: Removing devices on bus 0021:01
[ 916.357386] pci_hp_remove_devices: Removing 0021:01:00.0...
...
[ 920.566527] pci_hp_remove_devices: Removing 0021:01:00.1...
...
[ 933.955873] pci_bus 0021:01: busn_res: [bus 01-ff] is released
[ 933.955977] pcibios_free_controller_deferred: domain 33, dynamic 1
[ 933.955999] rpadlpar_io: slot PHB 33 removed
Suggested-By: Gavin Shan <gwshan@linux.vnet.ibm.com>
Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Tested-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> # cxl
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/include/asm/pci-bridge.h | 1
arch/powerpc/kernel/pci-common.c | 36 +++++++++++++++++++++++++++++
arch/powerpc/platforms/pseries/pci.c | 4 +++
arch/powerpc/platforms/pseries/pci_dlpar.c | 7 ++++-
4 files changed, 46 insertions(+), 2 deletions(-)
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -299,6 +299,7 @@ extern void pci_process_bridge_OF_ranges
/* Allocate & free a PCI host bridge structure */
extern struct pci_controller *pcibios_alloc_controller(struct device_node *dev);
extern void pcibios_free_controller(struct pci_controller *phb);
+extern void pcibios_free_controller_deferred(struct pci_host_bridge *bridge);
#ifdef CONFIG_PCI
extern int pcibios_vaddr_is_ioport(void __iomem *address);
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -103,6 +103,42 @@ void pcibios_free_controller(struct pci_
EXPORT_SYMBOL_GPL(pcibios_free_controller);
/*
+ * This function is used to call pcibios_free_controller()
+ * in a deferred manner: a callback from the PCI subsystem.
+ *
+ * _*DO NOT*_ call pcibios_free_controller() explicitly if
+ * this is used (or it may access an invalid *phb pointer).
+ *
+ * The callback occurs when all references to the root bus
+ * are dropped (e.g., child buses/devices and their users).
+ *
+ * It's called as .release_fn() of 'struct pci_host_bridge'
+ * which is associated with the 'struct pci_controller.bus'
+ * (root bus) - it expects .release_data to hold a pointer
+ * to 'struct pci_controller'.
+ *
+ * In order to use it, register .release_fn()/release_data
+ * like this:
+ *
+ * pci_set_host_bridge_release(bridge,
+ * pcibios_free_controller_deferred
+ * (void *) phb);
+ *
+ * e.g. in the pcibios_root_bridge_prepare() callback from
+ * pci_create_root_bus().
+ */
+void pcibios_free_controller_deferred(struct pci_host_bridge *bridge)
+{
+ struct pci_controller *phb = (struct pci_controller *)
+ bridge->release_data;
+
+ pr_debug("domain %d, dynamic %d\n", phb->global_number, phb->is_dynamic);
+
+ pcibios_free_controller(phb);
+}
+EXPORT_SYMBOL_GPL(pcibios_free_controller_deferred);
+
+/*
* The function is used to return the minimal alignment
* for memory or I/O windows of the associated P2P bridge.
* By default, 4KiB alignment for I/O windows and 1MiB for
--- a/arch/powerpc/platforms/pseries/pci.c
+++ b/arch/powerpc/platforms/pseries/pci.c
@@ -119,6 +119,10 @@ int pseries_root_bridge_prepare(struct p
bus = bridge->bus;
+ /* Rely on the pcibios_free_controller_deferred() callback. */
+ pci_set_host_bridge_release(bridge, pcibios_free_controller_deferred,
+ (void *) pci_bus_to_host(bus));
+
dn = pcibios_get_phb_of_node(bus);
if (!dn)
return 0;
--- a/arch/powerpc/platforms/pseries/pci_dlpar.c
+++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
@@ -106,8 +106,11 @@ int remove_phb_dynamic(struct pci_contro
release_resource(res);
}
- /* Free pci_controller data structure */
- pcibios_free_controller(phb);
+ /*
+ * The pci_controller data structure is freed by
+ * the pcibios_free_controller_deferred() callback;
+ * see pseries_root_bridge_prepare().
+ */
return 0;
}
next prev parent reply other threads:[~2016-10-14 12:55 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20161014125514uscas1p2cd792639354106580734df022fbaba7a@uscas1p2.samsung.com>
2016-10-14 12:54 ` [PATCH 4.7 00/31] 4.7.8-stable review Greg Kroah-Hartman
2016-10-14 12:54 ` Greg Kroah-Hartman [this message]
2016-10-14 12:54 ` [PATCH 4.7 02/31] cxl: use pcibios_free_controller_deferred() when removing vPHBs Greg Kroah-Hartman
2016-10-14 12:54 ` [PATCH 4.7 03/31] timekeeping: Fix __ktime_get_fast_ns() regression Greg Kroah-Hartman
2016-10-14 12:54 ` [PATCH 4.7 04/31] usb: dwc3: fix Clear Stall EP command failure Greg Kroah-Hartman
2016-10-14 12:54 ` [PATCH 4.7 05/31] ALSA: ali5451: Fix out-of-bound position reporting Greg Kroah-Hartman
2016-10-14 12:54 ` [PATCH 4.7 06/31] ALSA: usb-audio: Extend DragonFly dB scale quirk to cover other variants Greg Kroah-Hartman
2016-10-14 12:54 ` [PATCH 4.7 07/31] ALSA: usb-line6: use the same declaration as definition in header for MIDI manufacturer ID Greg Kroah-Hartman
2016-10-14 12:54 ` [PATCH 4.7 08/31] mfd: rtsx_usb: Avoid setting ucr->current_sg.status Greg Kroah-Hartman
2016-10-14 12:54 ` [PATCH 4.7 09/31] mfd: atmel-hlcdc: Do not sleep in atomic context Greg Kroah-Hartman
2016-10-14 12:54 ` [PATCH 4.7 10/31] mfd: 88pm80x: Double shifting bug in suspend/resume Greg Kroah-Hartman
2016-10-14 12:54 ` [PATCH 4.7 12/31] xen/x86: Update topology map for PV VCPUs Greg Kroah-Hartman
2016-10-14 12:54 ` [PATCH 4.7 13/31] KVM: PPC: Book3s PR: Allow access to unprivileged MMCR2 register Greg Kroah-Hartman
2016-10-14 12:54 ` [PATCH 4.7 15/31] KVM: arm64: Require in-kernel irqchip for PMU support Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 16/31] KVM: arm/arm64: vgic: Dont flush/sync without a working vgic Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 17/31] KVM: PPC: BookE: Fix a sanity check Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 18/31] arm64: fix dump_backtrace/unwind_frame with NULL tsk Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 19/31] x86/boot: Fix kdump, cleanup aborted E820_PRAM max_pfn manipulation Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 20/31] x86/irq: Prevent force migration of irqs which are not in the vector domain Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 21/31] x86/pkeys: Make protection keys an "eager" feature Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 22/31] x86/apic: Get rid of apic_version[] array Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 23/31] arch/x86: Handle non enumerated CPU after physical hotplug Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 24/31] x86/mm/pkeys: Do not skip PKRU register if debug registers are not used Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 25/31] x86/dumpstack: Fix x86_32 kernel_stack_pointer() previous stack access Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 26/31] ARM: dts: mvebu: armada-390: add missing compatibility string and bracket Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 28/31] ARM: cpuidle: Fix error return code Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 29/31] ima: use file_dentry() Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 30/31] tpm: fix a race condition in tpm2_unseal_trusted() Greg Kroah-Hartman
2016-10-14 12:55 ` [PATCH 4.7 31/31] tpm_crb: fix crb_req_canceled behavior Greg Kroah-Hartman
2016-10-14 18:54 ` [PATCH 4.7 00/31] 4.7.8-stable review Shuah Khan
2016-10-14 19:17 ` Guenter Roeck
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=20161014122715.295287333@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=andrew.donnellan@au1.ibm.com \
--cc=benh@kernel.crashing.org \
--cc=gwshan@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mauricfo@linux.vnet.ibm.com \
--cc=stable@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome