Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1641664 > unrolled thread

[RFC/RFT PATCH v2 3/3] PCI/ACPI: Add ACPI pci_bus_find_numa_node() implementation

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC/RFT PATCH v2 3/3] PCI/ACPI: Add ACPI pci_bus_find_numa_node() implementation Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2017-05-15 15:30 +0200
    Re: [RFC/RFT PATCH v2 3/3] PCI/ACPI: Add ACPI  pci_bus_find_numa_node() implementation Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2017-05-16 20:10 +0200
      Re: [RFC/RFT PATCH v2 3/3] PCI/ACPI: Add ACPI  pci_bus_find_numa_node() implementation Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2017-05-17 15:50 +0200
        Re: [RFC/RFT PATCH v2 3/3] PCI/ACPI: Add ACPI  pci_bus_find_numa_node() implementation Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2017-05-17 18:10 +0200

#1641664 — [RFC/RFT PATCH v2 3/3] PCI/ACPI: Add ACPI pci_bus_find_numa_node() implementation

The introduction of pci_bus_find_numa_node(pci_bus) allows at PCI
host bridge registration to detect the NUMA node for a given
struct pci_bus.dev. Implement an ACPI method that, through
the struct pci_bus.bridge ACPI companion, retrieve and return
the NUMA node corresponding to a given struct pci_bus.dev.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci-acpi.c | 20 ++++++++++++++++++++
 drivers/pci/pci.c      |  2 +-
 include/linux/pci.h    |  6 ++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 0018603..915da79 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -10,6 +10,7 @@
 #include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/irqdomain.h>
+#include <linux/numa.h>
 #include <linux/pci.h>
 #include <linux/msi.h>
 #include <linux/pci_hotplug.h>
@@ -853,6 +854,25 @@ struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus)
 	return irq_find_matching_fwnode(fwnode, DOMAIN_BUS_PCI_MSI);
 }
 
+int acpi_pci_bus_find_numa_node(struct pci_bus *bus)
+{
+	int node = NUMA_NO_NODE;
+	struct device *bridge = get_device(bus->bridge);
+
+	if (!bridge)
+		return node;
+
+	if (has_acpi_companion(bridge)) {
+		struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
+
+		node = acpi_get_node(adev->handle);
+	}
+
+	put_device(bridge);
+
+	return node;
+}
+
 static int __init acpi_pci_init(void)
 {
 	int ret;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index e9803c1..451342d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5406,7 +5406,7 @@ int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent)
 
 int pci_bus_find_numa_node(struct pci_bus *bus)
 {
-	return NUMA_NO_NODE;
+	return acpi_disabled ? NUMA_NO_NODE : acpi_pci_bus_find_numa_node(bus);
 }
 
 /**
diff --git a/include/linux/pci.h b/include/linux/pci.h
index ccbeefd..ed89220 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1472,6 +1472,12 @@ static inline int acpi_pci_bus_find_domain_nr(struct pci_bus *bus)
 #endif
 int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent);
 #endif
+#ifdef CONFIG_ACPI
+int acpi_pci_bus_find_numa_node(struct pci_bus *bus);
+#else
+static inline int acpi_pci_bus_find_numa_node(struct pci_bus *bus)
+{ return NUMA_NO_NODE; }
+#endif
 int pci_bus_find_numa_node(struct pci_bus *bus);
 
 /* some architectures require additional setup to direct VGA traffic */
-- 
2.10.0

[toc] | [next] | [standalone]

#1642715 — Re: [RFC/RFT PATCH v2 3/3] PCI/ACPI: Add ACPI pci_bus_find_numa_node() implementation

On Tue, May 16, 2017 at 05:15:29PM +0200, Robert Richter wrote:
> On 15.05.17 14:22:05, Lorenzo Pieralisi wrote:
> > The introduction of pci_bus_find_numa_node(pci_bus) allows at PCI
> > host bridge registration to detect the NUMA node for a given
> > struct pci_bus.dev. Implement an ACPI method that, through
> > the struct pci_bus.bridge ACPI companion, retrieve and return
> > the NUMA node corresponding to a given struct pci_bus.dev.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > ---
> >  drivers/pci/pci-acpi.c | 20 ++++++++++++++++++++
> >  drivers/pci/pci.c      |  2 +-
> >  include/linux/pci.h    |  6 ++++++
> >  3 files changed, 27 insertions(+), 1 deletion(-)
> > 
> 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index e9803c1..451342d 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -5406,7 +5406,7 @@ int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent)
> >  
> >  int pci_bus_find_numa_node(struct pci_bus *bus)
> >  {
> > -	return NUMA_NO_NODE;
> > +	return acpi_disabled ? NUMA_NO_NODE : acpi_pci_bus_find_numa_node(bus);
> 
> I looked into how this works with devicetree.
> 
> With ACPI it is set directly in pci_register_host_bridge() with
> set_dev_node().
> 
> For the DT case the set_dev_node call sets it to NUMA_NO_NODE first.
> Since in DT the bridge is a platform device which has the node id
> assigned already (if there is one), the bus' node id is set later in
> device_add() when deriving it from the parent device which is the
> bridge. So this should work.

Which also means that the node propagation for bus->dev in patch 2 can
be probably removed :), the problem with ACPI is setting the node
for the host bridge which in DT is done by default at platform device
creation, the rest is done by the core already there is not any need
to propagate it again when child busses are created (they take their
node from the parent).

Thanks for the heads-up and testing,
Lorenzo

> I have tested it both DT and ACPI.
> 
> Thanks,
> 
> -Robert
> 
> >  }
> >  
> >  /**

[toc] | [prev] | [next] | [standalone]

#1643368 — Re: [RFC/RFT PATCH v2 3/3] PCI/ACPI: Add ACPI pci_bus_find_numa_node() implementation

On Tue, May 16, 2017 at 07:02:00PM +0100, Lorenzo Pieralisi wrote:
> On Tue, May 16, 2017 at 05:15:29PM +0200, Robert Richter wrote:
> > On 15.05.17 14:22:05, Lorenzo Pieralisi wrote:
> > > The introduction of pci_bus_find_numa_node(pci_bus) allows at PCI
> > > host bridge registration to detect the NUMA node for a given
> > > struct pci_bus.dev. Implement an ACPI method that, through
> > > the struct pci_bus.bridge ACPI companion, retrieve and return
> > > the NUMA node corresponding to a given struct pci_bus.dev.
> > > 
> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > ---
> > >  drivers/pci/pci-acpi.c | 20 ++++++++++++++++++++
> > >  drivers/pci/pci.c      |  2 +-
> > >  include/linux/pci.h    |  6 ++++++
> > >  3 files changed, 27 insertions(+), 1 deletion(-)
> > > 
> > 
> > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > index e9803c1..451342d 100644
> > > --- a/drivers/pci/pci.c
> > > +++ b/drivers/pci/pci.c
> > > @@ -5406,7 +5406,7 @@ int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent)
> > >  
> > >  int pci_bus_find_numa_node(struct pci_bus *bus)
> > >  {
> > > -	return NUMA_NO_NODE;
> > > +	return acpi_disabled ? NUMA_NO_NODE : acpi_pci_bus_find_numa_node(bus);
> > 
> > I looked into how this works with devicetree.
> > 
> > With ACPI it is set directly in pci_register_host_bridge() with
> > set_dev_node().
> > 
> > For the DT case the set_dev_node call sets it to NUMA_NO_NODE first.
> > Since in DT the bridge is a platform device which has the node id
> > assigned already (if there is one), the bus' node id is set later in
> > device_add() when deriving it from the parent device which is the
> > bridge. So this should work.
> 
> Which also means that the node propagation for bus->dev in patch 2 can
> be probably removed :), the problem with ACPI is setting the node
> for the host bridge which in DT is done by default at platform device
> creation, the rest is done by the core already there is not any need
> to propagate it again when child busses are created (they take their
> node from the parent).

More explicitly, I think the whole series should work also with the diff
below applied on top of it. Side note: for consistency, I do not think
that adding a DT counterpart to pci_bus_find_numa_node() would hurt.

Thanks !
Lorenzo

-- >8 --
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 76c089f..cf0692c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -862,7 +862,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 	 */
 	child->dev.class = &pcibus_class;
 	dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
-	set_dev_node(&child->dev, dev_to_node(&parent->dev));
+
 	/*
 	 * Set up the primary, secondary and subordinate
 	 * bus numbers.

[toc] | [prev] | [next] | [standalone]

#1643487 — Re: [RFC/RFT PATCH v2 3/3] PCI/ACPI: Add ACPI pci_bus_find_numa_node() implementation

On Wed, May 17, 2017 at 04:35:58PM +0200, Robert Richter wrote:
> On 17.05.17 14:46:54, Lorenzo Pieralisi wrote:
> 
> > More explicitly, I think the whole series should work also with the diff
> > below applied on top of it. Side note: for consistency, I do not think
> > that adding a DT counterpart to pci_bus_find_numa_node() would hurt.
> > 
> > Thanks !
> > Lorenzo
> > 
> > -- >8 --
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 76c089f..cf0692c 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -862,7 +862,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
> >  	 */
> >  	child->dev.class = &pcibus_class;
> >  	dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
> > -	set_dev_node(&child->dev, dev_to_node(&parent->dev));
> > +
> 
> Hmm, in device_add() there is already:
> 
> 	/* use parent numa_node */
> 	if (parent && (dev_to_node(dev) == NUMA_NO_NODE))
>         	set_dev_node(dev, dev_to_node(parent));
> 
> So there are cases where the device has a different node than the
> parent. I am not sure if we can assume for pci that it maps always.
> 
> And since device_add() is called later anyway, the above change might
> not necessary at all.

That's why I _removed_ the set_dev_node() in the diff above (that applies
to patch (2)), I do not think it is a) correct and b) necessary to
propagate the NUMA node from bus to a child bus given that device_add()
takes care of that already.

I should post a v3 (with the diff above applied) so that we are all on
the same page and we can test it.

> But at least we must assign the node id to the
> bridge, which is the parent. Maybe just have in
> acpi_pci_root_create():
> 
> 	bridge = get_device(bus->bridge);
> 	adev = to_acpi_device_node(bridge->fwnode);
> 	set_dev_node(bridge, acpi_get_node(adev->handle));

I do not think that's enough, I need to check again but I think that
also the bus->dev should have its NUMA node set for things to work (and
allow the NUMA node to propagate correctly through device_add())
otherwise pcibus_to_node() would fail for devices sitting on the root
bus, right ?

I will check again and post v3 shortly.

Thanks !
Lorenzo

> 
> -Robert
> 
> 
> >  	/*
> >  	 * Set up the primary, secondary and subordinate
> >  	 * bus numbers.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web