Message ID | 1406586824-1692-1-git-send-email-gary.robertson@linaro.org |
---|---|
State | New |
Headers | show |
----- 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
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.
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 > ------------------------------------------------------------------------------
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);