Re: [PATCH 8/9] PCI: hotplug: Embed hotplug_slot
On Sun, 19 Aug 2018, Lukas Wunner wrote:
> When the PCI hotplug core and its first user, cpqphp, were introduced in
> February 2002 with historic commit a8a2069f432c, cpqphp allocated a slot
> struct for its internal use plus a hotplug_slot struct to be registered
> with the hotplug core and linked the two with pointers:
> https://git.kernel.org/tglx/history/c/a8a2069f432c
>
> Nowadays, the predominant pattern in the tree is to embed ("subclass")
> such structures in one another and cast to the containing struct with
> container_of(). But it wasn't until July 2002 that container_of() was
> introduced with historic commit ec4f214232cf:
> https://git.kernel.org/tglx/history/c/ec4f214232cf
>
> pnv_php, introduced in 2016, did the right thing and embedded struct
> hotplug_slot in its internal struct pnv_php_slot, but all other drivers
> cargo-culted cpqphp's design and linked separate structs with pointers.
>
> Embedding structs is preferrable to linking them with pointers because
> it requires fewer allocations, thereby reducing overhead and simplifying
> error paths. Casting an embedded struct to the containing struct
> becomes a cheap subtraction rather than a dereference. And having fewer
> pointers reduces the risk of them pointing nowhere either accidentally
> or due to an attack.
>
> Convert all drivers to embed struct hotplug_slot in their internal slot
> struct. The "private" pointer in struct hotplug_slot thereby becomes
> unused, so drop it.
>
> Signed-off-by: Lukas Wunner
> Cc: Rafael J. Wysocki
> Cc: Len Brown
> Cc: Scott Murray
> Cc: Benjamin Herrenschmidt
> Cc: Paul Mackerras
> Cc: Michael Ellerman
> Cc: Gavin Shan
> Cc: Sebastian Ott
> Cc: Gerald Schaefer
> Cc: Corentin Chary
> Cc: Darren Hart
> Cc: Andy Shevchenko
for s390_pci_hpc.c:
Acked-by: Sebastian Ott
Re: [PATCH 8/9] PCI: hotplug: Embed hotplug_slot
On 08/19/2018 07:29 AM, Lukas Wunner wrote:
> When the PCI hotplug core and its first user, cpqphp, were introduced in
> February 2002 with historic commit a8a2069f432c, cpqphp allocated a slot
> struct for its internal use plus a hotplug_slot struct to be registered
> with the hotplug core and linked the two with pointers:
> https://git.kernel.org/tglx/history/c/a8a2069f432c
>
> Nowadays, the predominant pattern in the tree is to embed ("subclass")
> such structures in one another and cast to the containing struct with
> container_of(). But it wasn't until July 2002 that container_of() was
> introduced with historic commit ec4f214232cf:
> https://git.kernel.org/tglx/history/c/ec4f214232cf
>
> pnv_php, introduced in 2016, did the right thing and embedded struct
> hotplug_slot in its internal struct pnv_php_slot, but all other drivers
> cargo-culted cpqphp's design and linked separate structs with pointers.
>
> Embedding structs is preferrable to linking them with pointers because
> it requires fewer allocations, thereby reducing overhead and simplifying
> error paths. Casting an embedded struct to the containing struct
> becomes a cheap subtraction rather than a dereference. And having fewer
> pointers reduces the risk of them pointing nowhere either accidentally
> or due to an attack.
>
> Convert all drivers to embed struct hotplug_slot in their internal slot
> struct. The "private" pointer in struct hotplug_slot thereby becomes
> unused, so drop it.
>
> Signed-off-by: Lukas Wunner
> Cc: Rafael J. Wysocki
> Cc: Len Brown
> Cc: Scott Murray
> Cc: Benjamin Herrenschmidt
> Cc: Paul Mackerras
> Cc: Michael Ellerman
> Cc: Gavin Shan
> Cc: Sebastian Ott
> Cc: Gerald Schaefer
> Cc: Corentin Chary
> Cc: Darren Hart
Re: [PATCH 8/9] PCI: hotplug: Embed hotplug_slot
On Sun, Aug 19, 2018 at 4:46 PM Lukas Wunner wrote:
>
> When the PCI hotplug core and its first user, cpqphp, were introduced in
> February 2002 with historic commit a8a2069f432c, cpqphp allocated a slot
> struct for its internal use plus a hotplug_slot struct to be registered
> with the hotplug core and linked the two with pointers:
> https://git.kernel.org/tglx/history/c/a8a2069f432c
>
> Nowadays, the predominant pattern in the tree is to embed ("subclass")
> such structures in one another and cast to the containing struct with
> container_of(). But it wasn't until July 2002 that container_of() was
> introduced with historic commit ec4f214232cf:
> https://git.kernel.org/tglx/history/c/ec4f214232cf
>
> pnv_php, introduced in 2016, did the right thing and embedded struct
> hotplug_slot in its internal struct pnv_php_slot, but all other drivers
> cargo-culted cpqphp's design and linked separate structs with pointers.
>
> Embedding structs is preferrable to linking them with pointers because
> it requires fewer allocations, thereby reducing overhead and simplifying
> error paths. Casting an embedded struct to the containing struct
> becomes a cheap subtraction rather than a dereference. And having fewer
> pointers reduces the risk of them pointing nowhere either accidentally
> or due to an attack.
>
> Convert all drivers to embed struct hotplug_slot in their internal slot
> struct. The "private" pointer in struct hotplug_slot thereby becomes
> unused, so drop it.
>
> Signed-off-by: Lukas Wunner
> Cc: Rafael J. Wysocki
> Cc: Len Brown
> Cc: Scott Murray
> Cc: Benjamin Herrenschmidt
> Cc: Paul Mackerras
> Cc: Michael Ellerman
> Cc: Gavin Shan
> Cc: Sebastian Ott
> Cc: Gerald Schaefer
> Cc: Corentin Chary
> Cc: Darren Hart
> Cc: Andy Shevchenko
> ---
> drivers/pci/hotplug/acpiphp.h | 9 ++-
> drivers/pci/hotplug/acpiphp_core.c | 28 +++-
> drivers/pci/hotplug/acpiphp_ibm.c | 2 +-
> drivers/pci/hotplug/cpci_hotplug.h | 9 ++-
> drivers/pci/hotplug/cpci_hotplug_core.c | 37 --
> drivers/pci/hotplug/cpci_hotplug_pci.c | 6 +-
> drivers/pci/hotplug/cpqphp.h| 9 ++-
> drivers/pci/hotplug/cpqphp_core.c | 37 --
> drivers/pci/hotplug/cpqphp_ctrl.c | 2 -
> drivers/pci/hotplug/ibmphp.h| 7 +-
> drivers/pci/hotplug/ibmphp_core.c | 92 +++--
> drivers/pci/hotplug/ibmphp_ebda.c | 37 +++---
> drivers/pci/hotplug/pciehp.h| 11 ++-
> drivers/pci/hotplug/pciehp_core.c | 37 --
> drivers/pci/hotplug/pciehp_ctrl.c | 4 +-
> drivers/pci/hotplug/pciehp_hpc.c| 8 +--
> drivers/pci/hotplug/pnv_php.c | 9 ++-
> drivers/pci/hotplug/rpaphp.h| 7 +-
> drivers/pci/hotplug/rpaphp_core.c | 14 ++--
> drivers/pci/hotplug/rpaphp_slot.c | 15 ++--
> drivers/pci/hotplug/s390_pci_hpc.c | 30
> drivers/pci/hotplug/sgi_hotplug.c | 52 ++
> drivers/pci/hotplug/shpchp.h| 6 +-
> drivers/pci/hotplug/shpchp_core.c | 17 ++---
> drivers/platform/x86/asus-wmi.c | 26 +++
> drivers/platform/x86/eeepc-laptop.c | 30
> include/linux/pci_hotplug.h | 3 -
> 27 files changed, 223 insertions(+), 321 deletions(-)
Good cleanup.
Reviewed-by: Rafael J. Wysocki
[PATCH 8/9] PCI: hotplug: Embed hotplug_slot
When the PCI hotplug core and its first user, cpqphp, were introduced in
February 2002 with historic commit a8a2069f432c, cpqphp allocated a slot
struct for its internal use plus a hotplug_slot struct to be registered
with the hotplug core and linked the two with pointers:
https://git.kernel.org/tglx/history/c/a8a2069f432c
Nowadays, the predominant pattern in the tree is to embed ("subclass")
such structures in one another and cast to the containing struct with
container_of(). But it wasn't until July 2002 that container_of() was
introduced with historic commit ec4f214232cf:
https://git.kernel.org/tglx/history/c/ec4f214232cf
pnv_php, introduced in 2016, did the right thing and embedded struct
hotplug_slot in its internal struct pnv_php_slot, but all other drivers
cargo-culted cpqphp's design and linked separate structs with pointers.
Embedding structs is preferrable to linking them with pointers because
it requires fewer allocations, thereby reducing overhead and simplifying
error paths. Casting an embedded struct to the containing struct
becomes a cheap subtraction rather than a dereference. And having fewer
pointers reduces the risk of them pointing nowhere either accidentally
or due to an attack.
Convert all drivers to embed struct hotplug_slot in their internal slot
struct. The "private" pointer in struct hotplug_slot thereby becomes
unused, so drop it.
Signed-off-by: Lukas Wunner
Cc: Rafael J. Wysocki
Cc: Len Brown
Cc: Scott Murray
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Cc: Gavin Shan
Cc: Sebastian Ott
Cc: Gerald Schaefer
Cc: Corentin Chary
Cc: Darren Hart
Cc: Andy Shevchenko
---
drivers/pci/hotplug/acpiphp.h | 9 ++-
drivers/pci/hotplug/acpiphp_core.c | 28 +++-
drivers/pci/hotplug/acpiphp_ibm.c | 2 +-
drivers/pci/hotplug/cpci_hotplug.h | 9 ++-
drivers/pci/hotplug/cpci_hotplug_core.c | 37 --
drivers/pci/hotplug/cpci_hotplug_pci.c | 6 +-
drivers/pci/hotplug/cpqphp.h| 9 ++-
drivers/pci/hotplug/cpqphp_core.c | 37 --
drivers/pci/hotplug/cpqphp_ctrl.c | 2 -
drivers/pci/hotplug/ibmphp.h| 7 +-
drivers/pci/hotplug/ibmphp_core.c | 92 +++--
drivers/pci/hotplug/ibmphp_ebda.c | 37 +++---
drivers/pci/hotplug/pciehp.h| 11 ++-
drivers/pci/hotplug/pciehp_core.c | 37 --
drivers/pci/hotplug/pciehp_ctrl.c | 4 +-
drivers/pci/hotplug/pciehp_hpc.c| 8 +--
drivers/pci/hotplug/pnv_php.c | 9 ++-
drivers/pci/hotplug/rpaphp.h| 7 +-
drivers/pci/hotplug/rpaphp_core.c | 14 ++--
drivers/pci/hotplug/rpaphp_slot.c | 15 ++--
drivers/pci/hotplug/s390_pci_hpc.c | 30
drivers/pci/hotplug/sgi_hotplug.c | 52 ++
drivers/pci/hotplug/shpchp.h| 6 +-
drivers/pci/hotplug/shpchp_core.c | 17 ++---
drivers/platform/x86/asus-wmi.c | 26 +++
drivers/platform/x86/eeepc-laptop.c | 30
include/linux/pci_hotplug.h | 3 -
27 files changed, 223 insertions(+), 321 deletions(-)
diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
index 8377e736ea69..cf3058404f41 100644
--- a/drivers/pci/hotplug/acpiphp.h
+++ b/drivers/pci/hotplug/acpiphp.h
@@ -33,14 +33,19 @@ struct acpiphp_slot;
* struct slot - slot information for each *physical* slot
*/
struct slot {
- struct hotplug_slot *hotplug_slot;
+ struct hotplug_slot hotplug_slot;
struct acpiphp_slot *acpi_slot;
unsigned int sun; /* ACPI _SUN (Slot User Number) value */
};
static inline const char *slot_name(struct slot *slot)
{
- return hotplug_slot_name(slot->hotplug_slot);
+ return hotplug_slot_name(&slot->hotplug_slot);
+}
+
+static inline struct slot *to_slot(struct hotplug_slot *hotplug_slot)
+{
+ return container_of(hotplug_slot, struct slot, hotplug_slot);
}
/*
diff --git a/drivers/pci/hotplug/acpiphp_core.c
b/drivers/pci/hotplug/acpiphp_core.c
index abd4f8d7e16a..c9e2bd40c038 100644
--- a/drivers/pci/hotplug/acpiphp_core.c
+++ b/drivers/pci/hotplug/acpiphp_core.c
@@ -118,7 +118,7 @@ EXPORT_SYMBOL_GPL(acpiphp_unregister_attention);
*/
static int enable_slot(struct hotplug_slot *hotplug_slot)
{
- struct slot *slot = hotplug_slot->private;
+ struct slot *slot = to_slot(hotplug_slot);
pr_debug("%s - physical_slot = %s\n", __func__, slot_name(slot));
@@ -135,7 +135,7 @@ static int enable_slot(struct hotplug_slot *hotplug_slot)
*/
static int disable_slot(struct hotplug_slot *hotplug_slot)
{
- struct slot *slot = hotplug_slot->private;
+ struct slot *slot = to_slot(hotplug_slot);
pr_debug("%s - physical_slot = %s\n", __func__, slot_name(slot));
@@ -179,7 +179,7 @@ static int set_attention_status(struct hotplug_slot
*hotplug_slot, u8 status)
*/
static int get_power_status(struct hotplug_slot *hot
