Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat in app/test/meson.build and then adding it as a build dependency. This causes build loop if the timestamp of this file keeps changing. It is fixed by hiding hugepage check in a shell script. Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests") Signed-off-by: Thomas Monjalon <thomas@monjalon.net> --- MAINTAINERS | 1 + app/test/has-hugepage.sh | 11 +++++++++++ app/test/meson.build | 8 ++------ 3 files changed, 14 insertions(+), 6 deletions(-) create mode 100755 app/test/has-hugepage.sh diff --git a/MAINTAINERS b/MAINTAINERS index 4800f6884a..aa619b6762 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1471,6 +1471,7 @@ F: app/test/Makefile F: app/test/autotest* F: app/test/commands.c F: app/test/get-coremask.sh +F: app/test/has-hugepage.sh F: app/test/packet_burst_generator.c F: app/test/packet_burst_generator.h F: app/test/process.h diff --git a/app/test/has-hugepage.sh b/app/test/has-hugepage.sh new file mode 100755 index 0000000000..484fc5541f --- /dev/null +++ b/app/test/has-hugepage.sh @@ -0,0 +1,11 @@ +#! /bin/sh -e +# SPDX-License-Identifier: BSD-3-Clause +# Copyright 2020 Mellanox Technologies, Ltd + +linux_hugepages_number=/proc/sys/vm/nr_hugepages + +if [ -r "$linux_hugepages_number" ] ; then + cat /proc/sys/vm/nr_hugepages +else + echo 0 +fi diff --git a/app/test/meson.build b/app/test/meson.build index 351d29cb65..542408d614 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -399,12 +399,8 @@ dpdk_test = executable('dpdk-test', has_hugepage = true if is_linux - check_hugepage = run_command('cat', - '/proc/sys/vm/nr_hugepages') - if (check_hugepage.returncode() != 0 or - check_hugepage.stdout().strip() == '0') - has_hugepage = false - endif + check_hugepage = find_program('has-hugepage.sh') + has_hugepage = run_command(check_hugepage).stdout().strip() != '0' endif message('hugepage availability: @0@'.format(has_hugepage)) -- 2.26.0
W dniu 09.04.2020 o 20:03, Thomas Monjalon pisze: > Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat > in app/test/meson.build and then adding it as a build dependency. > This causes build loop if the timestamp of this file keeps changing. > > It is fixed by hiding hugepage check in a shell script. > > Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests") > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net> > --- > MAINTAINERS | 1 + > app/test/has-hugepage.sh | 11 +++++++++++ > app/test/meson.build | 8 ++------ > 3 files changed, 14 insertions(+), 6 deletions(-) > create mode 100755 app/test/has-hugepage.sh > > diff --git a/MAINTAINERS b/MAINTAINERS > index 4800f6884a..aa619b6762 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1471,6 +1471,7 @@ F: app/test/Makefile > F: app/test/autotest* > F: app/test/commands.c > F: app/test/get-coremask.sh > +F: app/test/has-hugepage.sh > F: app/test/packet_burst_generator.c > F: app/test/packet_burst_generator.h > F: app/test/process.h > diff --git a/app/test/has-hugepage.sh b/app/test/has-hugepage.sh > new file mode 100755 > index 0000000000..484fc5541f > --- /dev/null > +++ b/app/test/has-hugepage.sh > @@ -0,0 +1,11 @@ > +#! /bin/sh -e > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright 2020 Mellanox Technologies, Ltd > + > +linux_hugepages_number=/proc/sys/vm/nr_hugepages > + > +if [ -r "$linux_hugepages_number" ] ; then > + cat /proc/sys/vm/nr_hugepages cat $linux_hugepages_number > +else > + echo 0 > +fi > diff --git a/app/test/meson.build b/app/test/meson.build > index 351d29cb65..542408d614 100644 > --- a/app/test/meson.build > +++ b/app/test/meson.build > @@ -399,12 +399,8 @@ dpdk_test = executable('dpdk-test', > > has_hugepage = true > if is_linux > - check_hugepage = run_command('cat', > - '/proc/sys/vm/nr_hugepages') > - if (check_hugepage.returncode() != 0 or > - check_hugepage.stdout().strip() == '0') > - has_hugepage = false > - endif > + check_hugepage = find_program('has-hugepage.sh') > + has_hugepage = run_command(check_hugepage).stdout().strip() != '0' > endif > message('hugepage availability: @0@'.format(has_hugepage)) > -- Lukasz Wojciechowski Principal Software Engineer Samsung R&D Institute Poland Samsung Electronics Office +48 22 377 88 25 l.wojciechow@partner.samsung.com
works fine
Tested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
W dniu 09.04.2020 o 20:03, Thomas Monjalon pisze:
> Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat
> in app/test/meson.build and then adding it as a build dependency.
> This causes build loop if the timestamp of this file keeps changing.
>
> It is fixed by hiding hugepage check in a shell script.
>
> Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests")
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> MAINTAINERS | 1 +
> app/test/has-hugepage.sh | 11 +++++++++++
> app/test/meson.build | 8 ++------
> 3 files changed, 14 insertions(+), 6 deletions(-)
> create mode 100755 app/test/has-hugepage.sh
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4800f6884a..aa619b6762 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1471,6 +1471,7 @@ F: app/test/Makefile
> F: app/test/autotest*
> F: app/test/commands.c
> F: app/test/get-coremask.sh
> +F: app/test/has-hugepage.sh
> F: app/test/packet_burst_generator.c
> F: app/test/packet_burst_generator.h
> F: app/test/process.h
> diff --git a/app/test/has-hugepage.sh b/app/test/has-hugepage.sh
> new file mode 100755
> index 0000000000..484fc5541f
> --- /dev/null
> +++ b/app/test/has-hugepage.sh
> @@ -0,0 +1,11 @@
> +#! /bin/sh -e
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2020 Mellanox Technologies, Ltd
> +
> +linux_hugepages_number=/proc/sys/vm/nr_hugepages
> +
> +if [ -r "$linux_hugepages_number" ] ; then
> + cat /proc/sys/vm/nr_hugepages
> +else
> + echo 0
> +fi
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 351d29cb65..542408d614 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -399,12 +399,8 @@ dpdk_test = executable('dpdk-test',
>
> has_hugepage = true
> if is_linux
> - check_hugepage = run_command('cat',
> - '/proc/sys/vm/nr_hugepages')
> - if (check_hugepage.returncode() != 0 or
> - check_hugepage.stdout().strip() == '0')
> - has_hugepage = false
> - endif
> + check_hugepage = find_program('has-hugepage.sh')
> + has_hugepage = run_command(check_hugepage).stdout().strip() != '0'
> endif
> message('hugepage availability: @0@'.format(has_hugepage))
>
--
Lukasz Wojciechowski
Principal Software Engineer
Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com
09/04/2020 20:10, Lukasz Wojciechowski:
>
> W dniu 09.04.2020 o 20:03, Thomas Monjalon pisze:
> > Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat
> > in app/test/meson.build and then adding it as a build dependency.
> > This causes build loop if the timestamp of this file keeps changing.
> >
> > It is fixed by hiding hugepage check in a shell script.
> >
> > Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests")
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> > +linux_hugepages_number=/proc/sys/vm/nr_hugepages
> > +
> > +if [ -r "$linux_hugepages_number" ] ; then
> > + cat /proc/sys/vm/nr_hugepages
> cat $linux_hugepages_number
True, thank you :-)
Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat in app/test/meson.build and then adding it as a build dependency. This causes build loop if the timestamp of this file keeps changing. It is fixed by hiding hugepage check in a shell script. Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests") Signed-off-by: Thomas Monjalon <thomas@monjalon.net> Tested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> --- v2: use variable as pointed by Lukasz --- MAINTAINERS | 1 + app/test/has-hugepage.sh | 11 +++++++++++ app/test/meson.build | 8 ++------ 3 files changed, 14 insertions(+), 6 deletions(-) create mode 100755 app/test/has-hugepage.sh diff --git a/MAINTAINERS b/MAINTAINERS index 4800f6884a..aa619b6762 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1471,6 +1471,7 @@ F: app/test/Makefile F: app/test/autotest* F: app/test/commands.c F: app/test/get-coremask.sh +F: app/test/has-hugepage.sh F: app/test/packet_burst_generator.c F: app/test/packet_burst_generator.h F: app/test/process.h diff --git a/app/test/has-hugepage.sh b/app/test/has-hugepage.sh new file mode 100755 index 0000000000..fc6cb7efaa --- /dev/null +++ b/app/test/has-hugepage.sh @@ -0,0 +1,11 @@ +#! /bin/sh -e +# SPDX-License-Identifier: BSD-3-Clause +# Copyright 2020 Mellanox Technologies, Ltd + +linux_hugepages_number=/proc/sys/vm/nr_hugepages + +if [ -r "$linux_hugepages_number" ] ; then + cat $linux_hugepages_number +else + echo 0 +fi diff --git a/app/test/meson.build b/app/test/meson.build index 351d29cb65..542408d614 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -399,12 +399,8 @@ dpdk_test = executable('dpdk-test', has_hugepage = true if is_linux - check_hugepage = run_command('cat', - '/proc/sys/vm/nr_hugepages') - if (check_hugepage.returncode() != 0 or - check_hugepage.stdout().strip() == '0') - has_hugepage = false - endif + check_hugepage = find_program('has-hugepage.sh') + has_hugepage = run_command(check_hugepage).stdout().strip() != '0' endif message('hugepage availability: @0@'.format(has_hugepage)) -- 2.26.0
It's perfect now and it still works ;)
Reviewed-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
W dniu 09.04.2020 o 21:34, Thomas Monjalon pisze:
> Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat
> in app/test/meson.build and then adding it as a build dependency.
> This causes build loop if the timestamp of this file keeps changing.
>
> It is fixed by hiding hugepage check in a shell script.
>
> Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests")
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Tested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> ---
>
> v2: use variable as pointed by Lukasz
>
> ---
> MAINTAINERS | 1 +
> app/test/has-hugepage.sh | 11 +++++++++++
> app/test/meson.build | 8 ++------
> 3 files changed, 14 insertions(+), 6 deletions(-)
> create mode 100755 app/test/has-hugepage.sh
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4800f6884a..aa619b6762 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1471,6 +1471,7 @@ F: app/test/Makefile
> F: app/test/autotest*
> F: app/test/commands.c
> F: app/test/get-coremask.sh
> +F: app/test/has-hugepage.sh
> F: app/test/packet_burst_generator.c
> F: app/test/packet_burst_generator.h
> F: app/test/process.h
> diff --git a/app/test/has-hugepage.sh b/app/test/has-hugepage.sh
> new file mode 100755
> index 0000000000..fc6cb7efaa
> --- /dev/null
> +++ b/app/test/has-hugepage.sh
> @@ -0,0 +1,11 @@
> +#! /bin/sh -e
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2020 Mellanox Technologies, Ltd
> +
> +linux_hugepages_number=/proc/sys/vm/nr_hugepages
> +
> +if [ -r "$linux_hugepages_number" ] ; then
> + cat $linux_hugepages_number
> +else
> + echo 0
> +fi
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 351d29cb65..542408d614 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -399,12 +399,8 @@ dpdk_test = executable('dpdk-test',
>
> has_hugepage = true
> if is_linux
> - check_hugepage = run_command('cat',
> - '/proc/sys/vm/nr_hugepages')
> - if (check_hugepage.returncode() != 0 or
> - check_hugepage.stdout().strip() == '0')
> - has_hugepage = false
> - endif
> + check_hugepage = find_program('has-hugepage.sh')
> + has_hugepage = run_command(check_hugepage).stdout().strip() != '0'
> endif
> message('hugepage availability: @0@'.format(has_hugepage))
>
--
Lukasz Wojciechowski
Principal Software Engineer
Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com
Thomas Monjalon <thomas@monjalon.net> writes: > Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat > in app/test/meson.build and then adding it as a build dependency. > This causes build loop if the timestamp of this file keeps changing. > > It is fixed by hiding hugepage check in a shell script. > > Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests") > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net> > Tested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> > --- Acked-by: Aaron Conole <aconole@redhat.com> > v2: use variable as pointed by Lukasz > > --- > MAINTAINERS | 1 + > app/test/has-hugepage.sh | 11 +++++++++++ > app/test/meson.build | 8 ++------ > 3 files changed, 14 insertions(+), 6 deletions(-) > create mode 100755 app/test/has-hugepage.sh > > diff --git a/MAINTAINERS b/MAINTAINERS > index 4800f6884a..aa619b6762 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1471,6 +1471,7 @@ F: app/test/Makefile > F: app/test/autotest* > F: app/test/commands.c > F: app/test/get-coremask.sh > +F: app/test/has-hugepage.sh > F: app/test/packet_burst_generator.c > F: app/test/packet_burst_generator.h > F: app/test/process.h > diff --git a/app/test/has-hugepage.sh b/app/test/has-hugepage.sh > new file mode 100755 > index 0000000000..fc6cb7efaa > --- /dev/null > +++ b/app/test/has-hugepage.sh > @@ -0,0 +1,11 @@ > +#! /bin/sh -e > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright 2020 Mellanox Technologies, Ltd > + > +linux_hugepages_number=/proc/sys/vm/nr_hugepages > + > +if [ -r "$linux_hugepages_number" ] ; then > + cat $linux_hugepages_number > +else > + echo 0 > +fi > diff --git a/app/test/meson.build b/app/test/meson.build > index 351d29cb65..542408d614 100644 > --- a/app/test/meson.build > +++ b/app/test/meson.build > @@ -399,12 +399,8 @@ dpdk_test = executable('dpdk-test', > > has_hugepage = true > if is_linux > - check_hugepage = run_command('cat', > - '/proc/sys/vm/nr_hugepages') > - if (check_hugepage.returncode() != 0 or > - check_hugepage.stdout().strip() == '0') > - has_hugepage = false > - endif > + check_hugepage = find_program('has-hugepage.sh') > + has_hugepage = run_command(check_hugepage).stdout().strip() != '0' > endif > message('hugepage availability: @0@'.format(has_hugepage))
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Friday, April 10, 2020 3:34 AM
> To: dev@dpdk.org
> Cc: bruce.richardson@intel.com; Lukasz Wojciechowski
> <l.wojciechow@partner.samsung.com>; Aaron Conole
> <aconole@redhat.com>; Ruifeng Wang <Ruifeng.Wang@arm.com>; David
> Marchand <david.marchand@redhat.com>; Gavin Hu <Gavin.Hu@arm.com>
> Subject: [PATCH v2] test: remove meson dependency on /proc file
>
> Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat in
> app/test/meson.build and then adding it as a build dependency.
> This causes build loop if the timestamp of this file keeps changing.
>
> It is fixed by hiding hugepage check in a shell script.
>
> Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests")
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Tested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> ---
>
> v2: use variable as pointed by Lukasz
>
> ---
> MAINTAINERS | 1 +
> app/test/has-hugepage.sh | 11 +++++++++++
> app/test/meson.build | 8 ++------
> 3 files changed, 14 insertions(+), 6 deletions(-) create mode 100755
> app/test/has-hugepage.sh
>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat in app/test/meson.build and then adding it as a build dependency. This causes build loop if the timestamp of this file keeps changing. It is fixed by hiding hugepage check in a shell script. Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests") Signed-off-by: Thomas Monjalon <thomas@monjalon.net> Tested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> Reviewed-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> Acked-by: Aaron Conole <aconole@redhat.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> --- v2: use variable as pointed by Lukasz v3: avoid TOCTOU issue as suggested by David I'm afraid it requires to re-confirm every reviews above. --- MAINTAINERS | 1 + app/test/has-hugepage.sh | 9 +++++++++ app/test/meson.build | 8 ++------ 3 files changed, 12 insertions(+), 6 deletions(-) create mode 100755 app/test/has-hugepage.sh diff --git a/MAINTAINERS b/MAINTAINERS index 4800f6884a..aa619b6762 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1471,6 +1471,7 @@ F: app/test/Makefile F: app/test/autotest* F: app/test/commands.c F: app/test/get-coremask.sh +F: app/test/has-hugepage.sh F: app/test/packet_burst_generator.c F: app/test/packet_burst_generator.h F: app/test/process.h diff --git a/app/test/has-hugepage.sh b/app/test/has-hugepage.sh new file mode 100755 index 0000000000..865e66cddd --- /dev/null +++ b/app/test/has-hugepage.sh @@ -0,0 +1,9 @@ +#! /bin/sh +# SPDX-License-Identifier: BSD-3-Clause +# Copyright 2020 Mellanox Technologies, Ltd + +if [ "$(uname)" = "Linux" ] ; then + cat /proc/sys/vm/nr_hugepages || echo 0 +else + echo 0 +fi diff --git a/app/test/meson.build b/app/test/meson.build index 351d29cb65..542408d614 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -399,12 +399,8 @@ dpdk_test = executable('dpdk-test', has_hugepage = true if is_linux - check_hugepage = run_command('cat', - '/proc/sys/vm/nr_hugepages') - if (check_hugepage.returncode() != 0 or - check_hugepage.stdout().strip() == '0') - has_hugepage = false - endif + check_hugepage = find_program('has-hugepage.sh') + has_hugepage = run_command(check_hugepage).stdout().strip() != '0' endif message('hugepage availability: @0@'.format(has_hugepage)) -- 2.26.0
On Fri, Apr 10, 2020 at 12:29:50PM +0200, Thomas Monjalon wrote: > Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat > in app/test/meson.build and then adding it as a build dependency. > This causes build loop if the timestamp of this file keeps changing. > > It is fixed by hiding hugepage check in a shell script. > > Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests") > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net> > Tested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> > Reviewed-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> > Acked-by: Aaron Conole <aconole@redhat.com> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> > --- > > v2: use variable as pointed by Lukasz > v3: avoid TOCTOU issue as suggested by David > I'm afraid it requires to re-confirm every reviews above. > > --- > MAINTAINERS | 1 + > app/test/has-hugepage.sh | 9 +++++++++ > app/test/meson.build | 8 ++------ > 3 files changed, 12 insertions(+), 6 deletions(-) > create mode 100755 app/test/has-hugepage.sh > > diff --git a/MAINTAINERS b/MAINTAINERS > index 4800f6884a..aa619b6762 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1471,6 +1471,7 @@ F: app/test/Makefile > F: app/test/autotest* > F: app/test/commands.c > F: app/test/get-coremask.sh > +F: app/test/has-hugepage.sh > F: app/test/packet_burst_generator.c > F: app/test/packet_burst_generator.h > F: app/test/process.h > diff --git a/app/test/has-hugepage.sh b/app/test/has-hugepage.sh > new file mode 100755 > index 0000000000..865e66cddd > --- /dev/null > +++ b/app/test/has-hugepage.sh > @@ -0,0 +1,9 @@ > +#! /bin/sh > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright 2020 Mellanox Technologies, Ltd > + > +if [ "$(uname)" = "Linux" ] ; then > + cat /proc/sys/vm/nr_hugepages || echo 0 > +else > + echo 0 > +fi > diff --git a/app/test/meson.build b/app/test/meson.build > index 351d29cb65..542408d614 100644 > --- a/app/test/meson.build > +++ b/app/test/meson.build > @@ -399,12 +399,8 @@ dpdk_test = executable('dpdk-test', > > has_hugepage = true > if is_linux Since you check for linux in the script, you can drop these two lines, and then just replace the whole branch with: has_hugepages = run_command('has-hugepage.sh').stdout().strip() != '0' > - check_hugepage = run_command('cat', > - '/proc/sys/vm/nr_hugepages') > - if (check_hugepage.returncode() != 0 or > - check_hugepage.stdout().strip() == '0') > - has_hugepage = false > - endif > + check_hugepage = find_program('has-hugepage.sh') > + has_hugepage = run_command(check_hugepage).stdout().strip() != '0' > endif > message('hugepage availability: @0@'.format(has_hugepage)) > > -- > 2.26.0 >
10/04/2020 12:42, Bruce Richardson:
> On Fri, Apr 10, 2020 at 12:29:50PM +0200, Thomas Monjalon wrote:
> > --- a/app/test/meson.build
> > +++ b/app/test/meson.build
> > has_hugepage = true
> > if is_linux
>
> Since you check for linux in the script, you can drop these two lines,
The issue is for Windows.
I am not sure how we will skip shell scripts
when adding Windows support for this application.
So there are two options now:
a) remove Linux check before calling the script and ignore Windows support for now
b) keep Linux check without knowing whether it will be useful for Windows support
We vote a?
On Fri, Apr 10, 2020 at 02:27:30PM +0200, Thomas Monjalon wrote:
> 10/04/2020 12:42, Bruce Richardson:
> > On Fri, Apr 10, 2020 at 12:29:50PM +0200, Thomas Monjalon wrote:
> > > --- a/app/test/meson.build
> > > +++ b/app/test/meson.build
> > > has_hugepage = true
> > > if is_linux
> >
> > Since you check for linux in the script, you can drop these two lines,
>
> The issue is for Windows.
> I am not sure how we will skip shell scripts
> when adding Windows support for this application.
> So there are two options now:
> a) remove Linux check before calling the script and ignore Windows support for now
> b) keep Linux check without knowing whether it will be useful for Windows support
>
> We vote a?
>
c) Write all such scripts in python, to allow them to run everywhere. :-)
Given that windows is the problem, having the is_linux check in the
meson.build file makes most sense - in which case I don't think we need the
check for linux in the script.
/Bruce
Thomas Monjalon <thomas@monjalon.net> writes:
> Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat
> in app/test/meson.build and then adding it as a build dependency.
> This causes build loop if the timestamp of this file keeps changing.
>
> It is fixed by hiding hugepage check in a shell script.
>
> Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests")
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Tested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> Reviewed-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> Acked-by: Aaron Conole <aconole@redhat.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>
> v2: use variable as pointed by Lukasz
> v3: avoid TOCTOU issue as suggested by David
> I'm afraid it requires to re-confirm every reviews above.
Still looks good to me.
10/04/2020 15:25, Bruce Richardson:
> On Fri, Apr 10, 2020 at 02:27:30PM +0200, Thomas Monjalon wrote:
> > 10/04/2020 12:42, Bruce Richardson:
> > > On Fri, Apr 10, 2020 at 12:29:50PM +0200, Thomas Monjalon wrote:
> > > > --- a/app/test/meson.build
> > > > +++ b/app/test/meson.build
> > > > has_hugepage = true
> > > > if is_linux
> > >
> > > Since you check for linux in the script, you can drop these two lines,
> >
> > The issue is for Windows.
> > I am not sure how we will skip shell scripts
> > when adding Windows support for this application.
> > So there are two options now:
> > a) remove Linux check before calling the script and ignore Windows support for now
> > b) keep Linux check without knowing whether it will be useful for Windows support
> >
> > We vote a?
> >
> c) Write all such scripts in python, to allow them to run everywhere. :-)
>
> Given that windows is the problem, having the is_linux check in the
> meson.build file makes most sense - in which case I don't think we need the
> check for linux in the script.
Yes, let's make it simple for now.
W dniu 10.04.2020 o 16:41, Thomas Monjalon pisze: > 10/04/2020 15:25, Bruce Richardson: >> On Fri, Apr 10, 2020 at 02:27:30PM +0200, Thomas Monjalon wrote: >>> 10/04/2020 12:42, Bruce Richardson: >>>> On Fri, Apr 10, 2020 at 12:29:50PM +0200, Thomas Monjalon wrote: >>>>> --- a/app/test/meson.build >>>>> +++ b/app/test/meson.build >>>>> has_hugepage = true >>>>> if is_linux >>>> Since you check for linux in the script, you can drop these two lines, >>> The issue is for Windows. >>> I am not sure how we will skip shell scripts >>> when adding Windows support for this application. >>> So there are two options now: >>> a) remove Linux check before calling the script and ignore Windows support for now >>> b) keep Linux check without knowing whether it will be useful for Windows support >>> >>> We vote a? >>> >> c) Write all such scripts in python, to allow them to run everywhere. :-) >> >> Given that windows is the problem, having the is_linux check in the >> meson.build file makes most sense - in which case I don't think we need the >> check for linux in the script. > Yes, let's make it simple for now. > a) is ok as the current meson.build won't work in Windows environment at all. c) would be best, but I guess it's a little bit to much for this patch and the whole build system could be adjusted to windows in separate thread. And I agree with Bruce, that the check for Linux in script is not needed. > -- Lukasz Wojciechowski Principal Software Engineer Samsung R&D Institute Poland Samsung Electronics Office +48 22 377 88 25 l.wojciechow@partner.samsung.com
On Fri, 10 Apr 2020 22:47:44 +0200
Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> wrote:
> W dniu 10.04.2020 o 16:41, Thomas Monjalon pisze:
> > 10/04/2020 15:25, Bruce Richardson:
> >> On Fri, Apr 10, 2020 at 02:27:30PM +0200, Thomas Monjalon wrote:
> >>> 10/04/2020 12:42, Bruce Richardson:
> >>>> On Fri, Apr 10, 2020 at 12:29:50PM +0200, Thomas Monjalon wrote:
> >>>>> --- a/app/test/meson.build
> >>>>> +++ b/app/test/meson.build
> >>>>> has_hugepage = true
> >>>>> if is_linux
> >>>> Since you check for linux in the script, you can drop these two lines,
> >>> The issue is for Windows.
> >>> I am not sure how we will skip shell scripts
> >>> when adding Windows support for this application.
> >>> So there are two options now:
> >>> a) remove Linux check before calling the script and ignore Windows support for now
> >>> b) keep Linux check without knowing whether it will be useful for Windows support
> >>>
> >>> We vote a?
> >>>
> >> c) Write all such scripts in python, to allow them to run everywhere. :-)
> >>
> >> Given that windows is the problem, having the is_linux check in the
> >> meson.build file makes most sense - in which case I don't think we need the
> >> check for linux in the script.
> > Yes, let's make it simple for now.
> >
>
> a) is ok as the current meson.build won't work in Windows environment at
> all.
> c) would be best, but I guess it's a little bit to much for this patch
> and the whole build system could be adjusted to windows in separate thread.
>
> And I agree with Bruce, that the check for Linux in script is not needed.
>
> >
Building in a container may not give access to /proc.
10/04/2020 22:47, Lukasz Wojciechowski:
> W dniu 10.04.2020 o 16:41, Thomas Monjalon pisze:
> > 10/04/2020 15:25, Bruce Richardson:
> >> On Fri, Apr 10, 2020 at 02:27:30PM +0200, Thomas Monjalon wrote:
> >>> 10/04/2020 12:42, Bruce Richardson:
> >>>> On Fri, Apr 10, 2020 at 12:29:50PM +0200, Thomas Monjalon wrote:
> >>>>> --- a/app/test/meson.build
> >>>>> +++ b/app/test/meson.build
> >>>>> has_hugepage = true
> >>>>> if is_linux
> >>>> Since you check for linux in the script, you can drop these two lines,
> >>> The issue is for Windows.
> >>> I am not sure how we will skip shell scripts
> >>> when adding Windows support for this application.
> >>> So there are two options now:
> >>> a) remove Linux check before calling the script and ignore Windows support for now
> >>> b) keep Linux check without knowing whether it will be useful for Windows support
> >>>
> >>> We vote a?
> >>>
> >> c) Write all such scripts in python, to allow them to run everywhere. :-)
> >>
> >> Given that windows is the problem, having the is_linux check in the
> >> meson.build file makes most sense - in which case I don't think we need the
> >> check for linux in the script.
> > Yes, let's make it simple for now.
> >
>
> a) is ok as the current meson.build won't work in Windows environment at
> all.
> c) would be best, but I guess it's a little bit to much for this patch
> and the whole build system could be adjusted to windows in separate thread.
>
> And I agree with Bruce, that the check for Linux in script is not needed.
I decide to go with initial Bruce's suggestion:
check OS in script and remove check from meson.
The reasons:
- the script is standalone and can be extended for FreeBSD
- if Windows support is required, converting to python is better
than a check in meson
Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat in app/test/meson.build and then adding it as a build dependency. This causes build loop if the timestamp of this file keeps changing. It is fixed by hiding hugepage check in a shell script. Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests") Signed-off-by: Thomas Monjalon <thomas@monjalon.net> Tested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> Reviewed-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> Acked-by: Aaron Conole <aconole@redhat.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> --- v2: use variable as pointed by Lukasz v3: avoid TOCTOU issue as suggested by David v4: remove Linux check in meson file and skip find_program() call --- MAINTAINERS | 1 + app/test/has-hugepage.sh | 9 +++++++++ app/test/meson.build | 10 +--------- 3 files changed, 11 insertions(+), 9 deletions(-) create mode 100755 app/test/has-hugepage.sh diff --git a/MAINTAINERS b/MAINTAINERS index 144cf5d8ea..fe59f0224f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1469,6 +1469,7 @@ F: app/test/Makefile F: app/test/autotest* F: app/test/commands.c F: app/test/get-coremask.sh +F: app/test/has-hugepage.sh F: app/test/packet_burst_generator.c F: app/test/packet_burst_generator.h F: app/test/process.h diff --git a/app/test/has-hugepage.sh b/app/test/has-hugepage.sh new file mode 100755 index 0000000000..865e66cddd --- /dev/null +++ b/app/test/has-hugepage.sh @@ -0,0 +1,9 @@ +#! /bin/sh +# SPDX-License-Identifier: BSD-3-Clause +# Copyright 2020 Mellanox Technologies, Ltd + +if [ "$(uname)" = "Linux" ] ; then + cat /proc/sys/vm/nr_hugepages || echo 0 +else + echo 0 +fi diff --git a/app/test/meson.build b/app/test/meson.build index 777c536ae0..04b59cffa4 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -396,15 +396,7 @@ dpdk_test = executable('dpdk-test', install_rpath: driver_install_path, install: true) -has_hugepage = true -if is_linux - check_hugepage = run_command('cat', - '/proc/sys/vm/nr_hugepages') - if (check_hugepage.returncode() != 0 or - check_hugepage.stdout().strip() == '0') - has_hugepage = false - endif -endif +has_hugepage = run_command('has-hugepage.sh').stdout().strip() != '0' message('hugepage availability: @0@'.format(has_hugepage)) # some perf tests (eg: memcpy perf autotest)take very long -- 2.26.0
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, April 15, 2020 2:20 PM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Lukasz Wojciechowski
> <l.wojciechow@partner.samsung.com>; Aaron Conole <aconole@redhat.com>;
> Ruifeng Wang <ruifeng.wang@arm.com>; David Marchand
> <david.marchand@redhat.com>; Gavin Hu <gavin.hu@arm.com>
> Subject: [PATCH v4] test: remove meson dependency on /proc file
>
> Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat
> in app/test/meson.build and then adding it as a build dependency.
> This causes build loop if the timestamp of this file keeps changing.
>
> It is fixed by hiding hugepage check in a shell script.
>
> Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests")
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Tested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> Reviewed-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> Acked-by: Aaron Conole <aconole@redhat.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
15/04/2020 15:28, Richardson, Bruce:
> From: Thomas Monjalon <thomas@monjalon.net>
> >
> > Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat
> > in app/test/meson.build and then adding it as a build dependency.
> > This causes build loop if the timestamp of this file keeps changing.
> >
> > It is fixed by hiding hugepage check in a shell script.
> >
> > Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests")
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > Tested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> > Reviewed-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> > Acked-by: Aaron Conole <aconole@redhat.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > ---
> >
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Applied
On Wed, Apr 15, 2020 at 3:20 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> Meson is detecting the path /proc/sys/vm/nr_hugepages in the call to cat
> in app/test/meson.build and then adding it as a build dependency.
> This causes build loop if the timestamp of this file keeps changing.
>
> It is fixed by hiding hugepage check in a shell script.
>
> Fixes: 77784ef0fba8 ("test: allow no-huge mode for fast-tests")
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Tested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> Reviewed-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> Acked-by: Aaron Conole <aconole@redhat.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>
> v2: use variable as pointed by Lukasz
> v3: avoid TOCTOU issue as suggested by David
> v4: remove Linux check in meson file and skip find_program() call
>
> ---
> MAINTAINERS | 1 +
> app/test/has-hugepage.sh | 9 +++++++++
> app/test/meson.build | 10 +---------
> 3 files changed, 11 insertions(+), 9 deletions(-)
> create mode 100755 app/test/has-hugepage.sh
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 144cf5d8ea..fe59f0224f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1469,6 +1469,7 @@ F: app/test/Makefile
> F: app/test/autotest*
> F: app/test/commands.c
> F: app/test/get-coremask.sh
> +F: app/test/has-hugepage.sh
> F: app/test/packet_burst_generator.c
> F: app/test/packet_burst_generator.h
> F: app/test/process.h
> diff --git a/app/test/has-hugepage.sh b/app/test/has-hugepage.sh
> new file mode 100755
> index 0000000000..865e66cddd
> --- /dev/null
> +++ b/app/test/has-hugepage.sh
> @@ -0,0 +1,9 @@
> +#! /bin/sh
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2020 Mellanox Technologies, Ltd
> +
> +if [ "$(uname)" = "Linux" ] ; then
> + cat /proc/sys/vm/nr_hugepages || echo 0
> +else
> + echo 0
> +fi
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 777c536ae0..04b59cffa4 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -396,15 +396,7 @@ dpdk_test = executable('dpdk-test',
> install_rpath: driver_install_path,
> install: true)
>
> -has_hugepage = true
> -if is_linux
> - check_hugepage = run_command('cat',
> - '/proc/sys/vm/nr_hugepages')
> - if (check_hugepage.returncode() != 0 or
> - check_hugepage.stdout().strip() == '0')
> - has_hugepage = false
> - endif
> -endif
> +has_hugepage = run_command('has-hugepage.sh').stdout().strip() != '0'
This change will force no-huge mode on FreeBSD while before we were
running with hugepages.
--
David Marchand
15/04/2020 15:36, David Marchand:
> On Wed, Apr 15, 2020 at 3:20 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > --- /dev/null
> > +++ b/app/test/has-hugepage.sh
> > @@ -0,0 +1,9 @@
> > +#! /bin/sh
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright 2020 Mellanox Technologies, Ltd
> > +
> > +if [ "$(uname)" = "Linux" ] ; then
> > + cat /proc/sys/vm/nr_hugepages || echo 0
> > +else
> > + echo 0
> > +fi
> > diff --git a/app/test/meson.build b/app/test/meson.build
> > index 777c536ae0..04b59cffa4 100644
> > --- a/app/test/meson.build
> > +++ b/app/test/meson.build
> > @@ -396,15 +396,7 @@ dpdk_test = executable('dpdk-test',
> > install_rpath: driver_install_path,
> > install: true)
> >
> > -has_hugepage = true
> > -if is_linux
> > - check_hugepage = run_command('cat',
> > - '/proc/sys/vm/nr_hugepages')
> > - if (check_hugepage.returncode() != 0 or
> > - check_hugepage.stdout().strip() == '0')
> > - has_hugepage = false
> > - endif
> > -endif
> > +has_hugepage = run_command('has-hugepage.sh').stdout().strip() != '0'
>
> This change will force no-huge mode on FreeBSD while before we were
> running with hugepages.
Indeed
I propose to squash this change to maintain the old behaviour:
+elif [ "$(uname)" = "FreeBSD" ] ; then
+ echo 1 # assume FreeBSD always has hugepages
On Wed, Apr 15, 2020 at 3:40 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > This change will force no-huge mode on FreeBSD while before we were
> > running with hugepages.
>
> Indeed
>
> I propose to squash this change to maintain the old behaviour:
>
> +elif [ "$(uname)" = "FreeBSD" ] ; then
> + echo 1 # assume FreeBSD always has hugepages
>
Ok for me.
--
David Marchand
15/04/2020 15:49, David Marchand: > On Wed, Apr 15, 2020 at 3:40 PM Thomas Monjalon <thomas@monjalon.net> wrote: > > > This change will force no-huge mode on FreeBSD while before we were > > > running with hugepages. > > > > Indeed > > > > I propose to squash this change to maintain the old behaviour: > > > > +elif [ "$(uname)" = "FreeBSD" ] ; then > > + echo 1 # assume FreeBSD always has hugepages > > > > Ok for me. OK, squashed: http://git.dpdk.org/dpdk/commit/?id=5f1a4a8a125672