* [dpdk-stable] [PATCH] bus/pci: fix selection of default device NUMA node
@ 2021-10-26 9:06 Houssem Bouhlel
2021-10-26 9:17 ` Olivier Matz
0 siblings, 1 reply; 6+ messages in thread
From: Houssem Bouhlel @ 2021-10-26 9:06 UTC (permalink / raw)
To: dev; +Cc: Houssem Bouhlel, stable, Olivier Matz
There can be dev binding issue when no hugepages
are allocated for socket 0.
To avoid this, set device numa node value based on
the first lcore instead of 0.
Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
Cc: stable@dpdk.org
Signed-off-by: Houssem Bouhlel <houssem.bouhlel@6wind.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/bus/pci/pci_common.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index f8fff2c98ebf..c70ab2373c79 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -166,6 +166,7 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
struct rte_pci_device *dev)
{
int ret;
+ unsigned int socket_id;
bool already_probed;
struct rte_pci_addr *loc;
@@ -194,7 +195,8 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
if (rte_socket_count() > 1)
RTE_LOG(INFO, EAL, "Device %s is not NUMA-aware, defaulting socket to 0\n",
dev->name);
- dev->device.numa_node = 0;
+ socket_id = rte_lcore_to_socket_id(rte_get_next_lcore(-1, 0, 0));
+ dev->device.numa_node = socket_id;
}
already_probed = rte_dev_is_probed(&dev->device);
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [PATCH] bus/pci: fix selection of default device NUMA node
2021-10-26 9:06 [dpdk-stable] [PATCH] bus/pci: fix selection of default device NUMA node Houssem Bouhlel
@ 2021-10-26 9:17 ` Olivier Matz
2021-10-29 8:44 ` Olivier Matz
0 siblings, 1 reply; 6+ messages in thread
From: Olivier Matz @ 2021-10-26 9:17 UTC (permalink / raw)
To: Houssem Bouhlel; +Cc: dev, stable
On Tue, Oct 26, 2021 at 11:06:10AM +0200, Houssem Bouhlel wrote:
> There can be dev binding issue when no hugepages
> are allocated for socket 0.
> To avoid this, set device numa node value based on
> the first lcore instead of 0.
>
> Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
Sorry, the Fixes line is wrong. This is the correct one:
Fixes: 8a04cb612589 ("pci: set default numa node for broken systems")
> Cc: stable@dpdk.org
>
> Signed-off-by: Houssem Bouhlel <houssem.bouhlel@6wind.com>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
> drivers/bus/pci/pci_common.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> index f8fff2c98ebf..c70ab2373c79 100644
> --- a/drivers/bus/pci/pci_common.c
> +++ b/drivers/bus/pci/pci_common.c
> @@ -166,6 +166,7 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
> struct rte_pci_device *dev)
> {
> int ret;
> + unsigned int socket_id;
> bool already_probed;
> struct rte_pci_addr *loc;
>
> @@ -194,7 +195,8 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
> if (rte_socket_count() > 1)
> RTE_LOG(INFO, EAL, "Device %s is not NUMA-aware, defaulting socket to 0\n",
> dev->name);
One more comment (sorry, I should have done it before you send the mail):
We should move this log below, and use the socket_id instead of 0.
> - dev->device.numa_node = 0;
> + socket_id = rte_lcore_to_socket_id(rte_get_next_lcore(-1, 0, 0));
> + dev->device.numa_node = socket_id;
> }
>
> already_probed = rte_dev_is_probed(&dev->device);
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [PATCH] bus/pci: fix selection of default device NUMA node
2021-10-26 9:17 ` Olivier Matz
@ 2021-10-29 8:44 ` Olivier Matz
2021-11-03 20:36 ` David Marchand
0 siblings, 1 reply; 6+ messages in thread
From: Olivier Matz @ 2021-10-29 8:44 UTC (permalink / raw)
To: Houssem Bouhlel; +Cc: dev, stable, David Marchand
+CC David
On Tue, Oct 26, 2021 at 11:17:08AM +0200, Olivier Matz wrote:
> On Tue, Oct 26, 2021 at 11:06:10AM +0200, Houssem Bouhlel wrote:
> > There can be dev binding issue when no hugepages
> > are allocated for socket 0.
> > To avoid this, set device numa node value based on
> > the first lcore instead of 0.
> >
> > Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
>
> Sorry, the Fixes line is wrong. This is the correct one:
> Fixes: 8a04cb612589 ("pci: set default numa node for broken systems")
>
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Houssem Bouhlel <houssem.bouhlel@6wind.com>
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> > drivers/bus/pci/pci_common.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> > index f8fff2c98ebf..c70ab2373c79 100644
> > --- a/drivers/bus/pci/pci_common.c
> > +++ b/drivers/bus/pci/pci_common.c
> > @@ -166,6 +166,7 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
> > struct rte_pci_device *dev)
> > {
> > int ret;
> > + unsigned int socket_id;
> > bool already_probed;
> > struct rte_pci_addr *loc;
> >
> > @@ -194,7 +195,8 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
> > if (rte_socket_count() > 1)
> > RTE_LOG(INFO, EAL, "Device %s is not NUMA-aware, defaulting socket to 0\n",
> > dev->name);
>
> One more comment (sorry, I should have done it before you send the mail):
> We should move this log below, and use the socket_id instead of 0.
>
> > - dev->device.numa_node = 0;
> > + socket_id = rte_lcore_to_socket_id(rte_get_next_lcore(-1, 0, 0));
> > + dev->device.numa_node = socket_id;
After some offline discussions with David, some additional comments:
- a similar change may be needed in other bus drivers
- instead of setting the numa node to an existing socket, it can make
more sense to keep its value to unknown (-1). This would however be a
behavior change for pci bus, which returns 0 since 2015 for unknown
cases. See:
81f8d2317df2 ("eal/linux: fix socket value for undetermined numa node")
8a04cb612589 ("pci: set default numa node for broken systems")
I'll tend to be in favor of using -1. Any other opinion?
Should we announce a behavior change in this case?
> > }
> >
> > already_probed = rte_dev_is_probed(&dev->device);
> > --
> > 2.30.2
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [PATCH] bus/pci: fix selection of default device NUMA node
2021-10-29 8:44 ` Olivier Matz
@ 2021-11-03 20:36 ` David Marchand
2021-11-04 8:57 ` Olivier Matz
2022-07-14 13:46 ` Thomas Monjalon
0 siblings, 2 replies; 6+ messages in thread
From: David Marchand @ 2021-11-03 20:36 UTC (permalink / raw)
To: Olivier Matz
Cc: Houssem Bouhlel, dev, dpdk stable, Thomas Monjalon, Yigit,
Ferruh, Burakov, Anatoly, Bruce Richardson
On Fri, Oct 29, 2021 at 10:45 AM Olivier Matz <olivier.matz@6wind.com> wrote:
>
> +CC David
>
> On Tue, Oct 26, 2021 at 11:17:08AM +0200, Olivier Matz wrote:
> > On Tue, Oct 26, 2021 at 11:06:10AM +0200, Houssem Bouhlel wrote:
> > > There can be dev binding issue when no hugepages
> > > are allocated for socket 0.
> > > To avoid this, set device numa node value based on
> > > the first lcore instead of 0.
> > >
> > > Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
> >
> > Sorry, the Fixes line is wrong. This is the correct one:
> > Fixes: 8a04cb612589 ("pci: set default numa node for broken systems")
> >
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Houssem Bouhlel <houssem.bouhlel@6wind.com>
> > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > ---
> > > drivers/bus/pci/pci_common.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> > > index f8fff2c98ebf..c70ab2373c79 100644
> > > --- a/drivers/bus/pci/pci_common.c
> > > +++ b/drivers/bus/pci/pci_common.c
> > > @@ -166,6 +166,7 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
> > > struct rte_pci_device *dev)
> > > {
> > > int ret;
> > > + unsigned int socket_id;
> > > bool already_probed;
> > > struct rte_pci_addr *loc;
> > >
> > > @@ -194,7 +195,8 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
> > > if (rte_socket_count() > 1)
> > > RTE_LOG(INFO, EAL, "Device %s is not NUMA-aware, defaulting socket to 0\n",
> > > dev->name);
> >
> > One more comment (sorry, I should have done it before you send the mail):
> > We should move this log below, and use the socket_id instead of 0.
> >
> > > - dev->device.numa_node = 0;
> > > + socket_id = rte_lcore_to_socket_id(rte_get_next_lcore(-1, 0, 0));
> > > + dev->device.numa_node = socket_id;
>
> After some offline discussions with David, some additional comments:
>
> - a similar change may be needed in other bus drivers
>
> - instead of setting the numa node to an existing socket, it can make
> more sense to keep its value to unknown (-1). This would however be a
> behavior change for pci bus, which returns 0 since 2015 for unknown
> cases. See:
> 81f8d2317df2 ("eal/linux: fix socket value for undetermined numa node")
> 8a04cb612589 ("pci: set default numa node for broken systems")
>
> I'll tend to be in favor of using -1. Any other opinion?
> Should we announce a behavior change in this case?
Good summary.
I copied some more people.
I am for -1 too (as a way to indicate "I don't know what this PCI
device affinity is").
It is dangerous to change now, and I think it is late for 21.11.
--
David Marchand
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [PATCH] bus/pci: fix selection of default device NUMA node
2021-11-03 20:36 ` David Marchand
@ 2021-11-04 8:57 ` Olivier Matz
2022-07-14 13:46 ` Thomas Monjalon
1 sibling, 0 replies; 6+ messages in thread
From: Olivier Matz @ 2021-11-04 8:57 UTC (permalink / raw)
To: David Marchand
Cc: Houssem Bouhlel, dev, dpdk stable, Thomas Monjalon, Yigit,
Ferruh, Burakov, Anatoly, Bruce Richardson
On Wed, Nov 03, 2021 at 09:36:49PM +0100, David Marchand wrote:
> On Fri, Oct 29, 2021 at 10:45 AM Olivier Matz <olivier.matz@6wind.com> wrote:
> >
> > +CC David
> >
> > On Tue, Oct 26, 2021 at 11:17:08AM +0200, Olivier Matz wrote:
> > > On Tue, Oct 26, 2021 at 11:06:10AM +0200, Houssem Bouhlel wrote:
> > > > There can be dev binding issue when no hugepages
> > > > are allocated for socket 0.
> > > > To avoid this, set device numa node value based on
> > > > the first lcore instead of 0.
> > > >
> > > > Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
> > >
> > > Sorry, the Fixes line is wrong. This is the correct one:
> > > Fixes: 8a04cb612589 ("pci: set default numa node for broken systems")
> > >
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Houssem Bouhlel <houssem.bouhlel@6wind.com>
> > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > > ---
> > > > drivers/bus/pci/pci_common.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> > > > index f8fff2c98ebf..c70ab2373c79 100644
> > > > --- a/drivers/bus/pci/pci_common.c
> > > > +++ b/drivers/bus/pci/pci_common.c
> > > > @@ -166,6 +166,7 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
> > > > struct rte_pci_device *dev)
> > > > {
> > > > int ret;
> > > > + unsigned int socket_id;
> > > > bool already_probed;
> > > > struct rte_pci_addr *loc;
> > > >
> > > > @@ -194,7 +195,8 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
> > > > if (rte_socket_count() > 1)
> > > > RTE_LOG(INFO, EAL, "Device %s is not NUMA-aware, defaulting socket to 0\n",
> > > > dev->name);
> > >
> > > One more comment (sorry, I should have done it before you send the mail):
> > > We should move this log below, and use the socket_id instead of 0.
> > >
> > > > - dev->device.numa_node = 0;
> > > > + socket_id = rte_lcore_to_socket_id(rte_get_next_lcore(-1, 0, 0));
> > > > + dev->device.numa_node = socket_id;
> >
> > After some offline discussions with David, some additional comments:
> >
> > - a similar change may be needed in other bus drivers
> >
> > - instead of setting the numa node to an existing socket, it can make
> > more sense to keep its value to unknown (-1). This would however be a
> > behavior change for pci bus, which returns 0 since 2015 for unknown
> > cases. See:
> > 81f8d2317df2 ("eal/linux: fix socket value for undetermined numa node")
> > 8a04cb612589 ("pci: set default numa node for broken systems")
> >
> > I'll tend to be in favor of using -1. Any other opinion?
> > Should we announce a behavior change in this case?
>
> Good summary.
> I copied some more people.
>
> I am for -1 too (as a way to indicate "I don't know what this PCI
> device affinity is").
>
> It is dangerous to change now, and I think it is late for 21.11.
+1, we can make an announce and change this for next version.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [PATCH] bus/pci: fix selection of default device NUMA node
2021-11-03 20:36 ` David Marchand
2021-11-04 8:57 ` Olivier Matz
@ 2022-07-14 13:46 ` Thomas Monjalon
1 sibling, 0 replies; 6+ messages in thread
From: Thomas Monjalon @ 2022-07-14 13:46 UTC (permalink / raw)
To: Olivier Matz, Houssem Bouhlel, David Marchand
Cc: stable, dev, Burakov, Anatoly, Bruce Richardson
03/11/2021 21:36, David Marchand:
> On Fri, Oct 29, 2021 at 10:45 AM Olivier Matz <olivier.matz@6wind.com> wrote:
> >
> > +CC David
> >
> > On Tue, Oct 26, 2021 at 11:17:08AM +0200, Olivier Matz wrote:
> > > On Tue, Oct 26, 2021 at 11:06:10AM +0200, Houssem Bouhlel wrote:
> > > > There can be dev binding issue when no hugepages
> > > > are allocated for socket 0.
> > > > To avoid this, set device numa node value based on
> > > > the first lcore instead of 0.
> > > >
> > > > Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
> > >
> > > Sorry, the Fixes line is wrong. This is the correct one:
> > > Fixes: 8a04cb612589 ("pci: set default numa node for broken systems")
> > >
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Houssem Bouhlel <houssem.bouhlel@6wind.com>
> > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > > ---
> > > > drivers/bus/pci/pci_common.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> > > > index f8fff2c98ebf..c70ab2373c79 100644
> > > > --- a/drivers/bus/pci/pci_common.c
> > > > +++ b/drivers/bus/pci/pci_common.c
> > > > @@ -166,6 +166,7 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
> > > > struct rte_pci_device *dev)
> > > > {
> > > > int ret;
> > > > + unsigned int socket_id;
> > > > bool already_probed;
> > > > struct rte_pci_addr *loc;
> > > >
> > > > @@ -194,7 +195,8 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
> > > > if (rte_socket_count() > 1)
> > > > RTE_LOG(INFO, EAL, "Device %s is not NUMA-aware, defaulting socket to 0\n",
> > > > dev->name);
> > >
> > > One more comment (sorry, I should have done it before you send the mail):
> > > We should move this log below, and use the socket_id instead of 0.
> > >
> > > > - dev->device.numa_node = 0;
> > > > + socket_id = rte_lcore_to_socket_id(rte_get_next_lcore(-1, 0, 0));
> > > > + dev->device.numa_node = socket_id;
> >
> > After some offline discussions with David, some additional comments:
> >
> > - a similar change may be needed in other bus drivers
Yes we need to be consistent.
You need to check what is done in all OSes as well.
Example of a place to look at:
3c6e58102510 ("bus/pci: fix unknown NUMA node value on Windows")
> > - instead of setting the numa node to an existing socket, it can make
> > more sense to keep its value to unknown (-1). This would however be a
> > behavior change for pci bus, which returns 0 since 2015 for unknown
> > cases. See:
> > 81f8d2317df2 ("eal/linux: fix socket value for undetermined numa node")
> > 8a04cb612589 ("pci: set default numa node for broken systems")
> >
> > I'll tend to be in favor of using -1. Any other opinion?
> > Should we announce a behavior change in this case?
>
> Good summary.
> I copied some more people.
>
> I am for -1 too (as a way to indicate "I don't know what this PCI
> device affinity is").
-1 is SOCKET_ID_ANY
I suppose it is OK to use SOCKET_ID_ANY when we have no other info.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-14 13:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-26 9:06 [dpdk-stable] [PATCH] bus/pci: fix selection of default device NUMA node Houssem Bouhlel
2021-10-26 9:17 ` Olivier Matz
2021-10-29 8:44 ` Olivier Matz
2021-11-03 20:36 ` David Marchand
2021-11-04 8:57 ` Olivier Matz
2022-07-14 13:46 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).