[dpdk-dev] [PATCH] test: remove meson dependency on /proc file

[dpdk-dev] [PATCH] test: remove meson dependency on /proc file

[Thomas Monjalon]

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

Re: [dpdk-dev] [PATCH] test: remove meson dependency on /proc file

[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>
> ---
>   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

Re: [dpdk-dev] [PATCH] test: remove meson dependency on /proc file

[Lukasz Wojciechowski]

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

Re: [dpdk-dev] [PATCH] test: remove meson dependency on /proc file

[Thomas Monjalon]

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 :-)

[dpdk-dev] [PATCH v2] test: remove meson dependency on /proc file

[Thomas Monjalon]

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

Re: [dpdk-dev] [PATCH v2] test: remove meson dependency on /proc file

[Lukasz Wojciechowski]

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

Re: [dpdk-dev] [PATCH v2] test: remove meson dependency on /proc file

[Aaron Conole]

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))

Re: [dpdk-dev] [PATCH v2] test: remove meson dependency on /proc file

[Ruifeng Wang]

> -----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>

[dpdk-dev] [PATCH v3] test: remove meson dependency on /proc file

[Thomas Monjalon]

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

Re: [dpdk-dev] [PATCH v3] test: remove meson dependency on /proc file

[Bruce Richardson]

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
> 

Re: [dpdk-dev] [PATCH v3] test: remove meson dependency on /proc file

[Thomas Monjalon]

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?

Re: [dpdk-dev] [PATCH v3] test: remove meson dependency on /proc file

[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.

/Bruce

Re: [dpdk-dev] [PATCH v3] test: remove meson dependency on /proc file

[Aaron Conole]

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.

Re: [dpdk-dev] [PATCH v3] test: remove meson dependency on /proc file

[Thomas Monjalon]

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.

Re: [dpdk-dev] [PATCH v3] test: remove meson dependency on /proc file

[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.

>
-- 

Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com

Re: [dpdk-dev] [PATCH v3] test: remove meson dependency on /proc file

[Stephen Hemminger]

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.

Re: [dpdk-dev] [PATCH v3] test: remove meson dependency on /proc file

[Thomas Monjalon]

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

[dpdk-dev] [PATCH v4] test: remove meson dependency on /proc file

[Thomas Monjalon]

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

Re: [dpdk-dev] [PATCH v4] test: remove meson dependency on /proc file

[Richardson, Bruce]

> -----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>

Re: [dpdk-dev] [PATCH v4] test: remove meson dependency on /proc file

[Thomas Monjalon]

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

Re: [dpdk-dev] [PATCH v4] test: remove meson dependency on /proc file

[David Marchand]

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

Re: [dpdk-dev] [PATCH v4] test: remove meson dependency on /proc file

[Thomas Monjalon]

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

Re: [dpdk-dev] [PATCH v4] test: remove meson dependency on /proc file

[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.

-- 
David Marchand

Re: [dpdk-dev] [PATCH v4] test: remove meson dependency on /proc file

[Thomas Monjalon]

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