* [dpdk-stable] [PATCH] test/eal: do not scan PCI devices for memory tests
@ 2019-08-01 12:27 David Marchand
2019-08-01 12:29 ` David Marchand
2019-08-02 20:57 ` Thomas Monjalon
0 siblings, 2 replies; 9+ messages in thread
From: David Marchand @ 2019-08-01 12:27 UTC (permalink / raw)
To: dev; +Cc: aconole, msantana, stable
The memory tests currently check that, for normal mode (not legacy mode),
there is no memory left behind when exiting.
The problem is that if a ethdev port is allocated when scanning pci
devices (even if the driver probe fails like when you have a virtio
management interface attached to the kernel), on exit, dpdk won't free
the associated memory since ethdev never frees the ethdev memzone.
Workaround this by disabling pci scan.
Fixes: 651cc78f83b5 ("test: fix hugepage file handling in EAL flags autotest")
Fixes: 690fd3577e90 ("test/eal: add cases for in-memory and single-file-segments")
Cc: stable@dpdk.org
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
app/test/test_eal_flags.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 5b2c0f5..6aaa4a3 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -1044,7 +1044,7 @@ test_file_prefix(void)
DEFAULT_MEM_SIZE, "--file-prefix=" memtest };
/* primary process with memtest1 and default mem mode */
- const char *argv1[] = {prgname, "-m",
+ const char *argv1[] = {prgname, "--no-pci", "-m",
DEFAULT_MEM_SIZE, "--file-prefix=" memtest1 };
/* primary process with memtest1 and legacy mem mode */
@@ -1058,7 +1058,7 @@ test_file_prefix(void)
"--legacy-mem" };
/* primary process with memtest2 and default mem mode */
- const char *argv4[] = {prgname, "-m",
+ const char *argv4[] = {prgname, "--no-pci", "-m",
DEFAULT_MEM_SIZE, "--file-prefix=" memtest2 };
/* primary process with --in-memory mode */
@@ -1075,7 +1075,7 @@ test_file_prefix(void)
DEFAULT_MEM_SIZE, "--in-memory", "--file-prefix", prefix };
/* primary process with memtest1 and --single-file-segments mode */
- const char * const argv8[] = {prgname, "-m",
+ const char * const argv8[] = {prgname, "--no-pci", "-m",
DEFAULT_MEM_SIZE, "--single-file-segments",
"--file-prefix=" memtest1 };
--
1.8.3.1
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [dpdk-stable] [PATCH] test/eal: do not scan PCI devices for memory tests
2019-08-01 12:27 [dpdk-stable] [PATCH] test/eal: do not scan PCI devices for memory tests David Marchand
@ 2019-08-01 12:29 ` David Marchand
2019-08-02 11:13 ` David Marchand
2019-08-02 13:37 ` Aaron Conole
2019-08-02 20:57 ` Thomas Monjalon
1 sibling, 2 replies; 9+ messages in thread
From: David Marchand @ 2019-08-01 12:29 UTC (permalink / raw)
To: dev; +Cc: Aaron Conole, Michael Santana, dpdk stable
On Thu, Aug 1, 2019 at 2:28 PM David Marchand <david.marchand@redhat.com> wrote:
>
> The memory tests currently check that, for normal mode (not legacy mode),
> there is no memory left behind when exiting.
>
> The problem is that if a ethdev port is allocated when scanning pci
> devices (even if the driver probe fails like when you have a virtio
> management interface attached to the kernel), on exit, dpdk won't free
> the associated memory since ethdev never frees the ethdev memzone.
>
> Workaround this by disabling pci scan.
Not entirely happy with this patch.
I am open to suggestions :-)
>
> Fixes: 651cc78f83b5 ("test: fix hugepage file handling in EAL flags autotest")
> Fixes: 690fd3577e90 ("test/eal: add cases for in-memory and single-file-segments")
> Cc: stable@dpdk.org
And we might want to drop stable.
--
David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [dpdk-stable] [PATCH] test/eal: do not scan PCI devices for memory tests
2019-08-01 12:29 ` David Marchand
@ 2019-08-02 11:13 ` David Marchand
2019-08-02 13:37 ` Aaron Conole
1 sibling, 0 replies; 9+ messages in thread
From: David Marchand @ 2019-08-02 11:13 UTC (permalink / raw)
To: dev; +Cc: Aaron Conole, Michael Santana, dpdk stable
On Thu, Aug 1, 2019 at 2:29 PM David Marchand <david.marchand@redhat.com> wrote:
>
> On Thu, Aug 1, 2019 at 2:28 PM David Marchand <david.marchand@redhat.com> wrote:
> >
> > The memory tests currently check that, for normal mode (not legacy mode),
> > there is no memory left behind when exiting.
> >
> > The problem is that if a ethdev port is allocated when scanning pci
> > devices (even if the driver probe fails like when you have a virtio
> > management interface attached to the kernel), on exit, dpdk won't free
> > the associated memory since ethdev never frees the ethdev memzone.
> >
> > Workaround this by disabling pci scan.
>
> Not entirely happy with this patch.
> I am open to suggestions :-)
Note: this problem won't be seen in shared builds as long as the pci
drivers for net devices are not loaded.
--
David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-stable] [PATCH] test/eal: do not scan PCI devices for memory tests
2019-08-01 12:29 ` David Marchand
2019-08-02 11:13 ` David Marchand
@ 2019-08-02 13:37 ` Aaron Conole
2019-08-02 13:45 ` David Marchand
1 sibling, 1 reply; 9+ messages in thread
From: Aaron Conole @ 2019-08-02 13:37 UTC (permalink / raw)
To: David Marchand; +Cc: dev, Michael Santana, dpdk stable
David Marchand <david.marchand@redhat.com> writes:
> On Thu, Aug 1, 2019 at 2:28 PM David Marchand <david.marchand@redhat.com> wrote:
>>
>> The memory tests currently check that, for normal mode (not legacy mode),
>> there is no memory left behind when exiting.
>>
>> The problem is that if a ethdev port is allocated when scanning pci
>> devices (even if the driver probe fails like when you have a virtio
>> management interface attached to the kernel), on exit, dpdk won't free
>> the associated memory since ethdev never frees the ethdev memzone.
>>
>> Workaround this by disabling pci scan.
>
> Not entirely happy with this patch.
> I am open to suggestions :-)
Seems like an order of allocation / free issue. Is it possible to
change the order to be consistent? IE: we only allocate something after
we know there's good reason to do so and then we can be sure to always
free? I don't know the code in this area well enough yet to comment any
more than that.
>>
>> Fixes: 651cc78f83b5 ("test: fix hugepage file handling in EAL flags autotest")
>> Fixes: 690fd3577e90 ("test/eal: add cases for in-memory and single-file-segments")
>> Cc: stable@dpdk.org
>
> And we might want to drop stable.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [dpdk-stable] [PATCH] test/eal: do not scan PCI devices for memory tests
2019-08-02 13:37 ` Aaron Conole
@ 2019-08-02 13:45 ` David Marchand
0 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2019-08-02 13:45 UTC (permalink / raw)
To: Aaron Conole; +Cc: dev, Michael Santana, dpdk stable, Burakov, Anatoly
On Fri, Aug 2, 2019 at 3:37 PM Aaron Conole <aconole@redhat.com> wrote:
>
> David Marchand <david.marchand@redhat.com> writes:
>
> > On Thu, Aug 1, 2019 at 2:28 PM David Marchand <david.marchand@redhat.com> wrote:
> >>
> >> The memory tests currently check that, for normal mode (not legacy mode),
> >> there is no memory left behind when exiting.
> >>
> >> The problem is that if a ethdev port is allocated when scanning pci
> >> devices (even if the driver probe fails like when you have a virtio
> >> management interface attached to the kernel), on exit, dpdk won't free
> >> the associated memory since ethdev never frees the ethdev memzone.
> >>
> >> Workaround this by disabling pci scan.
> >
> > Not entirely happy with this patch.
> > I am open to suggestions :-)
>
> Seems like an order of allocation / free issue. Is it possible to
> change the order to be consistent? IE: we only allocate something after
> we know there's good reason to do so and then we can be sure to always
> free? I don't know the code in this area well enough yet to comment any
> more than that.
Err, this looks hard to achieve.
The test is quite basic, in that it expects all hugepage files to be
removed on dpdk exit (meaning all allocations freed).
--
David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-stable] [PATCH] test/eal: do not scan PCI devices for memory tests
2019-08-01 12:27 [dpdk-stable] [PATCH] test/eal: do not scan PCI devices for memory tests David Marchand
2019-08-01 12:29 ` David Marchand
@ 2019-08-02 20:57 ` Thomas Monjalon
2019-08-03 9:51 ` David Marchand
1 sibling, 1 reply; 9+ messages in thread
From: Thomas Monjalon @ 2019-08-02 20:57 UTC (permalink / raw)
To: David Marchand; +Cc: stable, dev, aconole, msantana
01/08/2019 14:27, David Marchand:
> The memory tests currently check that, for normal mode (not legacy mode),
> there is no memory left behind when exiting.
I think this is the real bug:
we are checking a behaviour that we cannot achieve currently.
> The problem is that if a ethdev port is allocated when scanning pci
> devices (even if the driver probe fails like when you have a virtio
> management interface attached to the kernel), on exit, dpdk won't free
> the associated memory since ethdev never frees the ethdev memzone.
As you said in this thread, we could think about how to free it properly
in a future release.
For 19.08, I would suggest to disable the test with a comment
explaining the reason.
> Workaround this by disabling pci scan.
If you choose the --no-pci workaround, please add a comment
about the reason.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-stable] [PATCH] test/eal: do not scan PCI devices for memory tests
2019-08-02 20:57 ` Thomas Monjalon
@ 2019-08-03 9:51 ` David Marchand
2019-08-05 10:19 ` Thomas Monjalon
0 siblings, 1 reply; 9+ messages in thread
From: David Marchand @ 2019-08-03 9:51 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dpdk stable, dev, Aaron Conole, Michael Santana
On Fri, Aug 2, 2019 at 10:57 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 01/08/2019 14:27, David Marchand:
> > The memory tests currently check that, for normal mode (not legacy mode),
> > there is no memory left behind when exiting.
>
> I think this is the real bug:
> we are checking a behaviour that we cannot achieve currently.
>
> > The problem is that if a ethdev port is allocated when scanning pci
> > devices (even if the driver probe fails like when you have a virtio
> > management interface attached to the kernel), on exit, dpdk won't free
> > the associated memory since ethdev never frees the ethdev memzone.
>
> As you said in this thread, we could think about how to free it properly
> in a future release.
> For 19.08, I would suggest to disable the test with a comment
> explaining the reason.
For 19.08, as long as we test shared builds in the CI, then it just
"works", because the net drivers are not loaded.
No net driver, no ethdev leak ;-)
--
David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-stable] [PATCH] test/eal: do not scan PCI devices for memory tests
2019-08-03 9:51 ` David Marchand
@ 2019-08-05 10:19 ` Thomas Monjalon
2019-08-08 11:23 ` David Marchand
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Monjalon @ 2019-08-05 10:19 UTC (permalink / raw)
To: David Marchand; +Cc: stable, dev, Aaron Conole, Michael Santana
03/08/2019 11:51, David Marchand:
> On Fri, Aug 2, 2019 at 10:57 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > 01/08/2019 14:27, David Marchand:
> > > The memory tests currently check that, for normal mode (not legacy mode),
> > > there is no memory left behind when exiting.
> >
> > I think this is the real bug:
> > we are checking a behaviour that we cannot achieve currently.
> >
> > > The problem is that if a ethdev port is allocated when scanning pci
> > > devices (even if the driver probe fails like when you have a virtio
> > > management interface attached to the kernel), on exit, dpdk won't free
> > > the associated memory since ethdev never frees the ethdev memzone.
> >
> > As you said in this thread, we could think about how to free it properly
> > in a future release.
> > For 19.08, I would suggest to disable the test with a comment
> > explaining the reason.
>
> For 19.08, as long as we test shared builds in the CI, then it just
> "works", because the net drivers are not loaded.
> No net driver, no ethdev leak ;-)
So we keep the bug with the unit test not running with a static build
for 19.08, and we'll try to fix it in 19.11?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-stable] [PATCH] test/eal: do not scan PCI devices for memory tests
2019-08-05 10:19 ` Thomas Monjalon
@ 2019-08-08 11:23 ` David Marchand
0 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2019-08-08 11:23 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dpdk stable, dev, Aaron Conole, Michael Santana
On Mon, Aug 5, 2019 at 12:20 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> 03/08/2019 11:51, David Marchand:
> > On Fri, Aug 2, 2019 at 10:57 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > 01/08/2019 14:27, David Marchand:
> > > > The memory tests currently check that, for normal mode (not legacy mode),
> > > > there is no memory left behind when exiting.
> > >
> > > I think this is the real bug:
> > > we are checking a behaviour that we cannot achieve currently.
> > >
> > > > The problem is that if a ethdev port is allocated when scanning pci
> > > > devices (even if the driver probe fails like when you have a virtio
> > > > management interface attached to the kernel), on exit, dpdk won't free
> > > > the associated memory since ethdev never frees the ethdev memzone.
> > >
> > > As you said in this thread, we could think about how to free it properly
> > > in a future release.
> > > For 19.08, I would suggest to disable the test with a comment
> > > explaining the reason.
> >
> > For 19.08, as long as we test shared builds in the CI, then it just
> > "works", because the net drivers are not loaded.
> > No net driver, no ethdev leak ;-)
>
> So we keep the bug with the unit test not running with a static build
> for 19.08, and we'll try to fix it in 19.11?
Seems the more pragmatic yes.
--
David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-08-08 11:23 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-01 12:27 [dpdk-stable] [PATCH] test/eal: do not scan PCI devices for memory tests David Marchand
2019-08-01 12:29 ` David Marchand
2019-08-02 11:13 ` David Marchand
2019-08-02 13:37 ` Aaron Conole
2019-08-02 13:45 ` David Marchand
2019-08-02 20:57 ` Thomas Monjalon
2019-08-03 9:51 ` David Marchand
2019-08-05 10:19 ` Thomas Monjalon
2019-08-08 11:23 ` David Marchand
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).