20/09/2023 12:09, Bruce Richardson: > On Wed, Sep 20, 2023 at 12:00:08PM +0200, David Marchand wrote: > > On Thu, Sep 14, 2023 at 12:42 PM Bruce Richardson > > <bruce.richard...@intel.com> wrote: > > > > > > When examining the IOL testing failures for patch series [1], I observed > > > that the failures reported were in the eal_flags_file_prefix unit test. > > > I was able to reproduce this on my system by passing an additional > > > "--on-pci" flag to the test run, since the log to the test has errors > > > about device availability. Adding the "no-pci" flag to the individual > > > > Something is not clear to me. > > > > While I understand that passing "no-pci" helps avoiding the issue (as > > described below), I have some trouble understanding this passage > > (above) with "--on-pci". > > That's a typo for no-pci. When I ran the test on my system with the main > process using no-pci, I was able to reproduce the issue seen in the IOL > lab. Otherwise I couldn't reproduce it. > > > How did you reproduce the issue? > > > > > > > test commands used by the unit tests fixed the issue thereafter, > > > allowing the test to pass in all cases for me. Therefore, I am > > > submitting this patch in the hopes of making the test more robust, since > > > the observed failures seem unrelated to the original patchset [1] I > > > submitted. > > > > > > [1] http://patches.dpdk.org/project/dpdk/list/?series=29406 > > > > > > Bruce Richardson (1): > > > app/test: skip PCI bus scan when testing prefix flags > > > > > > app/test/test_eal_flags.c | 20 ++++++++++---------- > > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > Iiuc, the problem is that the file_prefix unit test can fail if any > > DPDK subsystem forgets to release some memory and some hugepages are > > left behind at the cleanup step. > > Passing --no-pci as you suggest hides issues coming from PCI drivers. > > > > This is something I tried to fix too, with > > https://patchwork.dpdk.org/project/dpdk/list/?series=29288 though my > > fix only handles a part of the issue (here, the ethdev drivers). > > > > Another way to make the file prefix more robust would be to remove the > > check on released memory, or move it to another test. > > > I actually think the test is a good one to have. Also, taking in your patch > to help with the issue is a good idea also. > > I'd still suggest that this patch be considered anyway, as there is no need > to do PCI bus scanning as part of this test. Therefore I'd view it as a > harmless addition that may help things.
I'm hesitating. This test is checking if some memory is left, and I think it is sane. If we add --no-pci, we reduce the coverage of this check. Now that the root cause is fixed by David in ethdev (https://patches.dpdk.org/project/dpdk/patch/20230821085806.3062613-4-david.march...@redhat.com/) we could continue checking memory freeing with PCI drivers. So I tend to reject this patch. Other opinions?
