Allow mutex initialization without PI configured

Message ID 1406586824-1692-1-git-send-email-gary.robertson@linaro.org
State New
Headers show

Commit Message

gary.robertson@linaro.org July 28, 2014, 10:33 p.m.
From: "Gary S. Robertson" <gary.robertson@linaro.org>

LTP seems primarily intended to be built and run on the same machine,
and thus uses autotools / autoconf to determine the features of the host
machine's environment prior to compilation.  However, when distributing
precompiled binaries, or when building in a cross-compilation environment,
it is better not to disable features based on the compile-time environment.

This patch removes compile-time dependencies on priority-inheriting mutexes
and allows the code to attempt initialization of PI mutexes even
if the runtime environment lacks that feature... in which case
the associated test(s) should fail.

Without this patch, init_pi_mutex() may be compiled as an empty function,
allowing subsequent operations to be attempted on an un-inititalized mutex.
Likewise, testpi-3 may fail to initialize a mutex before starting pthreads
which attempt to use the mutex.

Signed-off-by: Gary S. Robertson <gary.robertson@linaro.org>
---
 testcases/realtime/lib/librttest.c            |    2 --
 testcases/realtime/stress/pi-tests/testpi-3.c |    2 --
 2 files changed, 4 deletions(-)

Comments

Jan Stancek July 29, 2014, 8:05 a.m. | #1
----- Original Message -----
> From: "Gary S. Robertson" <gary.robertson@linaro.org>
> To: ltp-list@lists.sourceforge.net
> Cc: "mike holmes" <mike.holmes@linaro.org>
> Sent: Tuesday, 29 July, 2014 12:33:44 AM
> Subject: [LTP] [PATCH] Allow mutex initialization without PI configured
> 
> From: "Gary S. Robertson" <gary.robertson@linaro.org>
> 
> LTP seems primarily intended to be built and run on the same machine,
> and thus uses autotools / autoconf to determine the features of the host
> machine's environment prior to compilation.  However, when distributing
> precompiled binaries, or when building in a cross-compilation environment,
> it is better not to disable features based on the compile-time environment.

Isn't the build going to fail then if you miss those features?

> This patch removes compile-time dependencies on priority-inheriting mutexes

Seems to me, the problem is:

-#if HAVE_DECL_PTHREAD_PRIO_INHERIT
+#if HAS_PRIORITY_INHERIT

I'm not against removing that check entirely. I'm not sure anyone is using
distro so old it's missing it. We also don't check for it in openposix testsuite.

> and allows the code to attempt initialization of PI mutexes even
> if the runtime environment lacks that feature... in which case
> the associated test(s) should fail.
> 
> Without this patch, init_pi_mutex() may be compiled as an empty function,
> allowing subsequent operations to be attempted on an un-inititalized mutex.
> Likewise, testpi-3 may fail to initialize a mutex before starting pthreads
> which attempt to use the mutex.

Then it shouldn't run at all and end with TCONF.

> 
> Signed-off-by: Gary S. Robertson <gary.robertson@linaro.org>
> ---
>  testcases/realtime/lib/librttest.c            |    2 --
>  testcases/realtime/stress/pi-tests/testpi-3.c |    2 --
>  2 files changed, 4 deletions(-)
> 
> diff --git a/testcases/realtime/lib/librttest.c
> b/testcases/realtime/lib/librttest.c
> index 0dd6019..9f37d08 100644
> --- a/testcases/realtime/lib/librttest.c
> +++ b/testcases/realtime/lib/librttest.c
> @@ -586,7 +586,6 @@ void *busy_work_us(int us)
>  
>  void init_pi_mutex(pthread_mutex_t * m)
>  {
> -#if HAVE_DECL_PTHREAD_PRIO_INHERIT
>  	pthread_mutexattr_t attr;
>  	int ret;
>  	int protocol;
> @@ -609,7 +608,6 @@ void init_pi_mutex(pthread_mutex_t * m)
>  	if ((ret = pthread_mutex_init(m, &attr)) != 0) {
>  		printf("Failed to init mutex: %d (%s)\n", ret, strerror(ret));
>  	}
> -#endif1G

         ^^ There's extra "1G" here

Regards,
Jan

>  
>  	/* FIXME: does any of this need to be destroyed ? */
>  }
> diff --git a/testcases/realtime/stress/pi-tests/testpi-3.c
> b/testcases/realtime/stress/pi-tests/testpi-3.c
> index 30f38f6..807c9cc 100644
> --- a/testcases/realtime/stress/pi-tests/testpi-3.c
> +++ b/testcases/realtime/stress/pi-tests/testpi-3.c
> @@ -365,7 +365,6 @@ int main(int argc, char *argv[])
>  
>  	printf("Start %s\n", argv[0]);
>  
> -#if HAVE_DECL_PTHREAD_PRIO_INHERIT
>  	if (!nopi) {
>  		pthread_mutexattr_t mutexattr;
>  		int protocol;
> @@ -386,7 +385,6 @@ int main(int argc, char *argv[])
>  			printf("Failed to init mutex: %d\n", retc);
>  		}
>  	}
> -#endif
>  
>  	startThread(&arg1);
>  	startThread(&arg2);
> --
> 1.7.9.5
> 
> 
> ------------------------------------------------------------------------------
> Infragistics Professional
> Build stunning WinForms apps today!
> Reboot your WinForms applications with our WinForms controls.
> Build a bridge from your legacy apps to the future.
> http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
> _______________________________________________
> Ltp-list mailing list
> Ltp-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ltp-list
> 

------------------------------------------------------------------------------
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls. 
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
chrubis@suse.cz July 29, 2014, 10:53 a.m. | #2
Hi!
> LTP seems primarily intended to be built and run on the same machine,
> and thus uses autotools / autoconf to determine the features of the host
> machine's environment prior to compilation.  However, when distributing
> precompiled binaries, or when building in a cross-compilation environment,
> it is better not to disable features based on the compile-time environment.
> 
> This patch removes compile-time dependencies on priority-inheriting mutexes
> and allows the code to attempt initialization of PI mutexes even
> if the runtime environment lacks that feature... in which case
> the associated test(s) should fail.
> 
> Without this patch, init_pi_mutex() may be compiled as an empty function,
> allowing subsequent operations to be attempted on an un-inititalized mutex.
> Likewise, testpi-3 may fail to initialize a mutex before starting pthreads
> which attempt to use the mutex.

As far as I can see the configure check tests if PTHREAD_PRIO_INHERIT is
defined in system headers. So if you just remove the ifdefs the
compilation will fail and if that is not the case, the check is broken.
gary.robertson@linaro.org Aug. 15, 2014, 1:43 p.m. | #3
Thank everyone for their consideration and responses.  After reflecting on
the challenges faced by the LTP project in building and running
successfully on a wide variety of platforms, I realize that the conditional
compilation is in fact required.  Our cross-compilation environment is an
unique situation and we need to handle the build-time problems at the local
level.  This patch is therefore not appropriate at the LTP project level
and I consequently wish to withdraw it from further consideration on this
mailing list.  I will instead submit a new patch which ensures that all
defined mutexes get default initialization at least, even if the
conditional compilation results in omission of the code which would
initialize them as PI mutexes.


On Tue, Jul 29, 2014 at 5:53 AM, <chrubis@suse.cz> wrote:

> Hi!
> > LTP seems primarily intended to be built and run on the same machine,
> > and thus uses autotools / autoconf to determine the features of the host
> > machine's environment prior to compilation.  However, when distributing
> > precompiled binaries, or when building in a cross-compilation
> environment,
> > it is better not to disable features based on the compile-time
> environment.
> >
> > This patch removes compile-time dependencies on priority-inheriting
> mutexes
> > and allows the code to attempt initialization of PI mutexes even
> > if the runtime environment lacks that feature... in which case
> > the associated test(s) should fail.
> >
> > Without this patch, init_pi_mutex() may be compiled as an empty function,
> > allowing subsequent operations to be attempted on an un-inititalized
> mutex.
> > Likewise, testpi-3 may fail to initialize a mutex before starting
> pthreads
> > which attempt to use the mutex.
>
> As far as I can see the configure check tests if PTHREAD_PRIO_INHERIT is
> defined in system headers. So if you just remove the ifdefs the
> compilation will fail and if that is not the case, the check is broken.
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
------------------------------------------------------------------------------

Patch hide | download patch | download mbox

diff --git a/testcases/realtime/lib/librttest.c b/testcases/realtime/lib/librttest.c
index 0dd6019..9f37d08 100644
--- a/testcases/realtime/lib/librttest.c
+++ b/testcases/realtime/lib/librttest.c
@@ -586,7 +586,6 @@  void *busy_work_us(int us)
 
 void init_pi_mutex(pthread_mutex_t * m)
 {
-#if HAVE_DECL_PTHREAD_PRIO_INHERIT
 	pthread_mutexattr_t attr;
 	int ret;
 	int protocol;
@@ -609,7 +608,6 @@  void init_pi_mutex(pthread_mutex_t * m)
 	if ((ret = pthread_mutex_init(m, &attr)) != 0) {
 		printf("Failed to init mutex: %d (%s)\n", ret, strerror(ret));
 	}
-#endif1G
 
 	/* FIXME: does any of this need to be destroyed ? */
 }
diff --git a/testcases/realtime/stress/pi-tests/testpi-3.c b/testcases/realtime/stress/pi-tests/testpi-3.c
index 30f38f6..807c9cc 100644
--- a/testcases/realtime/stress/pi-tests/testpi-3.c
+++ b/testcases/realtime/stress/pi-tests/testpi-3.c
@@ -365,7 +365,6 @@  int main(int argc, char *argv[])
 
 	printf("Start %s\n", argv[0]);
 
-#if HAVE_DECL_PTHREAD_PRIO_INHERIT
 	if (!nopi) {
 		pthread_mutexattr_t mutexattr;
 		int protocol;
@@ -386,7 +385,6 @@  int main(int argc, char *argv[])
 			printf("Failed to init mutex: %d\n", retc);
 		}
 	}
-#endif
 
 	startThread(&arg1);
 	startThread(&arg2);