[Buildroot] [PATCH 1/1] ltp-testsuite: Add upstream patch to fix build numa detection

[Buildroot] [PATCH 1/1] ltp-testsuite: Add upstream patch to fix build numa detection

[Petr Vorel]

This removes fix added in commit 3c937807a3 ("ltp-testsuite: disable numa tests")

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
 ...x-numa-v2-detection-for-cross-compilation.patch | 37 ++++++++++++++++++++++
 package/ltp-testsuite/ltp-testsuite.mk             |  3 +-
 2 files changed, 38 insertions(+), 2 deletions(-)
 create mode 100644 package/ltp-testsuite/0002-numa-Fix-numa-v2-detection-for-cross-compilation.patch

diff --git a/package/ltp-testsuite/0002-numa-Fix-numa-v2-detection-for-cross-compilation.patch b/package/ltp-testsuite/0002-numa-Fix-numa-v2-detection-for-cross-compilation.patch
new file mode 100644
index 0000000000..6ad1db046e
--- /dev/null
+++ b/package/ltp-testsuite/0002-numa-Fix-numa-v2-detection-for-cross-compilation.patch
@@ -0,0 +1,37 @@
+From 01c134f30be755a43af12a4ae2c31177d04be790 Mon Sep 17 00:00:00 2001
+From: Petr Vorel <pvorel@suse.cz>
+Date: Mon, 29 Jan 2018 10:31:40 +0100
+Subject: [PATCH] numa: Fix numa v2 detection for cross compilation
+
+Changed to AC_COMPILE_IFELSE as AC_RUN_IFELSE cannot be run while
+cross compiling.
+
+Bug found by Buildroot project.
+
+Signed-off-by: Petr Vorel <pvorel@suse.cz>
+Reported-by: Baruch Siach <baruch@tkos.co.il>
+---
+ m4/ltp-numa.m4 | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/m4/ltp-numa.m4 b/m4/ltp-numa.m4
+index fc3383e97..633f54c18 100644
+--- a/m4/ltp-numa.m4
++++ b/m4/ltp-numa.m4
+@@ -28,11 +28,11 @@ AC_DEFUN([LTP_CHECK_SYSCALL_NUMA], [
+ 	AC_CHECK_HEADERS([numa.h numaif.h], [], [have_numa_headers=no])
+ 
+ 	if test "x$have_numa_headers" != "xno"; then
+-		AC_RUN_IFELSE([AC_LANG_PROGRAM([
++		AC_COMPILE_IFELSE([AC_LANG_PROGRAM([
+ #include <numa.h>
+ 		], [
+ #if LIBNUMA_API_VERSION < 2
+-exit(1);
++# error Required numa headers >= 2
+ #endif
+ 		])], [have_numa_headers_v2=yes])
+ 	fi
+-- 
+2.16.0
+
diff --git a/package/ltp-testsuite/ltp-testsuite.mk b/package/ltp-testsuite/ltp-testsuite.mk
index 28dc24749e..87854568d8 100644
--- a/package/ltp-testsuite/ltp-testsuite.mk
+++ b/package/ltp-testsuite/ltp-testsuite.mk
@@ -56,8 +56,7 @@ LTP_TESTSUITE_CONF_ENV += \
 	CFLAGS="$(LTP_TESTSUITE_CFLAGS)" \
 	CPPFLAGS="$(LTP_TESTSUITE_CPPFLAGS)" \
 	LIBS="$(LTP_TESTSUITE_LIBS)" \
-	SYSROOT="$(STAGING_DIR)" \
-	have_numa_headers=no
+	SYSROOT="$(STAGING_DIR)"
 
 # Requires uClibc fts and bessel support, normally not enabled
 ifeq ($(BR2_TOOLCHAIN_USES_UCLIBC),y)
-- 
2.16.0

[Buildroot] [PATCH 1/1] ltp-testsuite: Add upstream patch to fix build numa detection

[Thomas Petazzoni]

Hello,

On Mon, 29 Jan 2018 21:23:14 +0100, Petr Vorel wrote:
> This removes fix added in commit 3c937807a3 ("ltp-testsuite: disable numa tests")
> 
> Signed-off-by: Petr Vorel <petr.vorel@gmail.com>

Thanks for this update. There is however an issue, see below.

> ---
>  ...x-numa-v2-detection-for-cross-compilation.patch | 37 ++++++++++++++++++++++
>  package/ltp-testsuite/ltp-testsuite.mk             |  3 +-
>  2 files changed, 38 insertions(+), 2 deletions(-)
>  create mode 100644 package/ltp-testsuite/0002-numa-Fix-numa-v2-detection-for-cross-compilation.patch
> 
> diff --git a/package/ltp-testsuite/0002-numa-Fix-numa-v2-detection-for-cross-compilation.patch b/package/ltp-testsuite/0002-numa-Fix-numa-v2-detection-for-cross-compilation.patch
> new file mode 100644
> index 0000000000..6ad1db046e
> --- /dev/null
> +++ b/package/ltp-testsuite/0002-numa-Fix-numa-v2-detection-for-cross-compilation.patch
> @@ -0,0 +1,37 @@
> +From 01c134f30be755a43af12a4ae2c31177d04be790 Mon Sep 17 00:00:00 2001
> +From: Petr Vorel <pvorel@suse.cz>
> +Date: Mon, 29 Jan 2018 10:31:40 +0100
> +Subject: [PATCH] numa: Fix numa v2 detection for cross compilation
> +
> +Changed to AC_COMPILE_IFELSE as AC_RUN_IFELSE cannot be run while
> +cross compiling.
> +
> +Bug found by Buildroot project.
> +
> +Signed-off-by: Petr Vorel <pvorel@suse.cz>
> +Reported-by: Baruch Siach <baruch@tkos.co.il>
> +---
> + m4/ltp-numa.m4 | 4 ++--
> + 1 file changed, 2 insertions(+), 2 deletions(-)

You are changing m4 files, which requires regenerating the configure
script. Therefore, the package should have LTP_TESTSUITE_AUTORECONF =
YES, with a comment above it that says it's needed for patch
0002-numa-Fix-numa-v2-detection-for-cross-compilation.patch.

Could you check this, and submit an updated version?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] ltp-testsuite: Add upstream patch to fix build numa detection

[Baruch Siach]

Hi Petr,

On Mon, Jan 29, 2018 at 09:23:14PM +0100, Petr Vorel wrote:
> This removes fix added in commit 3c937807a3 ("ltp-testsuite: disable numa tests")
> 
> Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> ---
>  ...x-numa-v2-detection-for-cross-compilation.patch | 37 ++++++++++++++++++++++
>  package/ltp-testsuite/ltp-testsuite.mk             |  3 +-
>  2 files changed, 38 insertions(+), 2 deletions(-)
>  create mode 100644 package/ltp-testsuite/0002-numa-Fix-numa-v2-detection-for-cross-compilation.patch

[...]

> diff --git a/package/ltp-testsuite/ltp-testsuite.mk b/package/ltp-testsuite/ltp-testsuite.mk
> index 28dc24749e..87854568d8 100644
> --- a/package/ltp-testsuite/ltp-testsuite.mk
> +++ b/package/ltp-testsuite/ltp-testsuite.mk
> @@ -56,8 +56,7 @@ LTP_TESTSUITE_CONF_ENV += \
>  	CFLAGS="$(LTP_TESTSUITE_CFLAGS)" \
>  	CPPFLAGS="$(LTP_TESTSUITE_CPPFLAGS)" \
>  	LIBS="$(LTP_TESTSUITE_LIBS)" \
> -	SYSROOT="$(STAGING_DIR)" \
> -	have_numa_headers=no
> +	SYSROOT="$(STAGING_DIR)"
>  
>  # Requires uClibc fts and bessel support, normally not enabled
>  ifeq ($(BR2_TOOLCHAIN_USES_UCLIBC),y)

You should also make numactl an optional dependency to make the build more 
reproducible.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] [PATCH 1/1] ltp-testsuite: Add upstream patch to fix build numa detection

[Petr Vorel]

Hi Baruch,

> On Mon, Jan 29, 2018 at 09:23:14PM +0100, Petr Vorel wrote:
> > This removes fix added in commit 3c937807a3 ("ltp-testsuite: disable numa tests")

> > Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> > ---
> >  ...x-numa-v2-detection-for-cross-compilation.patch | 37 ++++++++++++++++++++++
> >  package/ltp-testsuite/ltp-testsuite.mk             |  3 +-
> >  2 files changed, 38 insertions(+), 2 deletions(-)
> >  create mode 100644 package/ltp-testsuite/0002-numa-Fix-numa-v2-detection-for-cross-compilation.patch

> [...]

> > diff --git a/package/ltp-testsuite/ltp-testsuite.mk b/package/ltp-testsuite/ltp-testsuite.mk
> > index 28dc24749e..87854568d8 100644
> > --- a/package/ltp-testsuite/ltp-testsuite.mk
> > +++ b/package/ltp-testsuite/ltp-testsuite.mk
> > @@ -56,8 +56,7 @@ LTP_TESTSUITE_CONF_ENV += \
> >  	CFLAGS="$(LTP_TESTSUITE_CFLAGS)" \
> >  	CPPFLAGS="$(LTP_TESTSUITE_CPPFLAGS)" \
> >  	LIBS="$(LTP_TESTSUITE_LIBS)" \
> > -	SYSROOT="$(STAGING_DIR)" \
> > -	have_numa_headers=no
> > +	SYSROOT="$(STAGING_DIR)"

> >  # Requires uClibc fts and bessel support, normally not enabled
> >  ifeq ($(BR2_TOOLCHAIN_USES_UCLIBC),y)

> You should also make numactl an optional dependency to make the build more 
> reproducible.

Can you be more specific, what you mean? numactl is required as a command in numa based
tests (used by testcases/kernel/numa/numa01.sh). But that's not build dependency, it's a
runtime dependency.

> baruch


Kind regards,
Petr

[Buildroot] [PATCH 1/1] ltp-testsuite: Add upstream patch to fix build numa detection

[Baruch Siach]

Hi Petr,

On Thu, Feb 01, 2018 at 11:19:51PM +0100, Petr Vorel wrote:
> > On Mon, Jan 29, 2018 at 09:23:14PM +0100, Petr Vorel wrote:
> > > This removes fix added in commit 3c937807a3 ("ltp-testsuite: disable numa tests")
> 
> > > Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> > > ---
> > >  ...x-numa-v2-detection-for-cross-compilation.patch | 37 ++++++++++++++++++++++
> > >  package/ltp-testsuite/ltp-testsuite.mk             |  3 +-
> > >  2 files changed, 38 insertions(+), 2 deletions(-)
> > >  create mode 100644 package/ltp-testsuite/0002-numa-Fix-numa-v2-detection-for-cross-compilation.patch
> 
> > [...]
> 
> > > diff --git a/package/ltp-testsuite/ltp-testsuite.mk b/package/ltp-testsuite/ltp-testsuite.mk
> > > index 28dc24749e..87854568d8 100644
> > > --- a/package/ltp-testsuite/ltp-testsuite.mk
> > > +++ b/package/ltp-testsuite/ltp-testsuite.mk
> > > @@ -56,8 +56,7 @@ LTP_TESTSUITE_CONF_ENV += \
> > >  	CFLAGS="$(LTP_TESTSUITE_CFLAGS)" \
> > >  	CPPFLAGS="$(LTP_TESTSUITE_CPPFLAGS)" \
> > >  	LIBS="$(LTP_TESTSUITE_LIBS)" \
> > > -	SYSROOT="$(STAGING_DIR)" \
> > > -	have_numa_headers=no
> > > +	SYSROOT="$(STAGING_DIR)"
> 
> > >  # Requires uClibc fts and bessel support, normally not enabled
> > >  ifeq ($(BR2_TOOLCHAIN_USES_UCLIBC),y)
> 
> > You should also make numactl an optional dependency to make the build more 
> > reproducible.
> 
> Can you be more specific, what you mean? numactl is required as a command in numa based
> tests (used by testcases/kernel/numa/numa01.sh). But that's not build dependency, it's a
> runtime dependency.

numactl is also an optional build time dependency of ltp-testsuite. When the 
ltp-testsuite configure script detects libnuma some additional tests are 
built. Currently when the numactl package is enabled, ltp-testsuite would 
detect libnuma only if numactl happens to build before ltp-testsuite. To make 
builds consistent we need to make sure that numactl always build before 
ltp-testsuite by adding it to _DEPENDENCIES. Here is how the hwloc package 
does that:

ifeq ($(BR2_PACKAGE_NUMACTL),y)
HWLOC_CONF_OPTS += --enable-libnuma
HWLOC_DEPENDENCIES += numactl
else
HWLOC_CONF_OPTS += --disable-libnuma
endif

ltp-testsuite needs something similar.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

[Buildroot] [PATCH 1/1] ltp-testsuite: Add upstream patch to fix build numa detection

[Petr Vorel]

Hi Baruch,

thanks for your explanation!

> numactl is also an optional build time dependency of ltp-testsuite. When the 
> ltp-testsuite configure script detects libnuma some additional tests are 
> built.
I was thinking some time ago to add this support, but it's not how it works.
All sources which depends on NUMA have a preprocessor condition like his: [1]
#ifdef HAVE_NUMA_V2
...
#else
	TST_TEST_TCONF("test requires libnuma >= 2 and it's development packages");
#endif
(These are using new LTP test API, tests using old API or not using API at all have it
implemented differently.)

I added HAVE_NUMA_V2 in commit bf3441268 ("numa: Check for API >= v2 with autoconf"), but
it the check  has always been like that (*) all the tests are actually being build
regardless of NUMA support on SUT. I mean, there is no rule in Makefile which would
prevent build them because missing NUMA. This is unusual, but the reason why tests are
build just to say "you need libnuma support", that these files are expected in runtest
files (files in $LTPROOT/runtest).

(*) This approach was added in the sources in 2010.

> Currently when the numactl package is enabled, ltp-testsuite would 
> detect libnuma only if numactl happens to build before ltp-testsuite. To make 
> builds consistent we need to make sure that numactl always build before 
> ltp-testsuite by adding it to _DEPENDENCIES. Here is how the hwloc package 
> does that:

> ifeq ($(BR2_PACKAGE_NUMACTL),y)
> HWLOC_CONF_OPTS += --enable-libnuma
> HWLOC_DEPENDENCIES += numactl
> else
> HWLOC_CONF_OPTS += --disable-libnuma
> endif
So this is not needed.

> ltp-testsuite needs something similar.


> baruch


Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/mem/cpuset/cpuset01.c

[Buildroot] [PATCH 1/1] ltp-testsuite: Add upstream patch to fix build numa detection

[Baruch Siach]

Hi Petr,

On Fri, Feb 02, 2018 at 02:59:57PM +0100, Petr Vorel wrote:
> thanks for your explanation!
> 
> > numactl is also an optional build time dependency of ltp-testsuite. When the 
> > ltp-testsuite configure script detects libnuma some additional tests are 
> > built.
> I was thinking some time ago to add this support, but it's not how it works.
> All sources which depends on NUMA have a preprocessor condition like his: [1]
> #ifdef HAVE_NUMA_V2
> ...
> #else
> 	TST_TEST_TCONF("test requires libnuma >= 2 and it's development packages");
> #endif
> (These are using new LTP test API, tests using old API or not using API at all have it
> implemented differently.)

So the behaviour of ltp-testsuite changes when its configure script detects 
libnuma, right? We want this change to be consistent whenever numactl is 
enabled in buildroot, so that ltp-testsuite would always detect libnuma when 
it is available. We don't want to leave it up to the pseudo-random packages 
build order.

> I added HAVE_NUMA_V2 in commit bf3441268 ("numa: Check for API >= v2 with autoconf"), but
> it the check  has always been like that (*) all the tests are actually being build
> regardless of NUMA support on SUT. I mean, there is no rule in Makefile which would
> prevent build them because missing NUMA. This is unusual, but the reason why tests are
> build just to say "you need libnuma support", that these files are expected in runtest
> files (files in $LTPROOT/runtest).
> 
> (*) This approach was added in the sources in 2010.
> 
> > Currently when the numactl package is enabled, ltp-testsuite would 
> > detect libnuma only if numactl happens to build before ltp-testsuite. To make 
> > builds consistent we need to make sure that numactl always build before 
> > ltp-testsuite by adding it to _DEPENDENCIES. Here is how the hwloc package 
> > does that:
> 
> > ifeq ($(BR2_PACKAGE_NUMACTL),y)
> > HWLOC_CONF_OPTS += --enable-libnuma
> > HWLOC_DEPENDENCIES += numactl
> > else
> > HWLOC_CONF_OPTS += --disable-libnuma
> > endif
> So this is not needed.

So this is needed to make the build consistent, IMO.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

[Buildroot] [PATCH 1/1] ltp-testsuite: Add upstream patch to fix build numa detection

[Petr Vorel]

Hi Baruch,

thanks for info!

> So the behaviour of ltp-testsuite changes when its configure script detects 
> libnuma, right? We want this change to be consistent whenever numactl is 
> enabled in buildroot, so that ltp-testsuite would always detect libnuma when 
> it is available. We don't want to leave it up to the pseudo-random packages 
> build order.
Understand. I'm sorry I didn't realize that numactl is not name of binary package,
but name of the whole library. I didn't know that upstream uses it as name of the whole
project.

> > > Currently when the numactl package is enabled, ltp-testsuite would 
> > > detect libnuma only if numactl happens to build before ltp-testsuite. To make 
> > > builds consistent we need to make sure that numactl always build before 
> > > ltp-testsuite by adding it to _DEPENDENCIES. Here is how the hwloc package 
> > > does that:

> > > ifeq ($(BR2_PACKAGE_NUMACTL),y)
> > > HWLOC_CONF_OPTS += --enable-libnuma
> > > HWLOC_DEPENDENCIES += numactl
> > > else
> > > HWLOC_CONF_OPTS += --disable-libnuma
> > > endif
> > So this is not needed.

> So this is needed to make the build consistent, IMO.
This makes sense. Would you please prepare patch?


Kind regards,
Petr