Re: [Qemu-devel] [PATCH 17/23] bsd-user: add shims for thread related system calls

[Stacey Son]

On Jun 24, 2013, at 12:37 PM, Peter Maydell wrote:

> On 24 June 2013 03:03, Stacey Son <address@hidden> wrote:
>> diff --git a/configure b/configure
>> index ba90975..812ea3f 100755
>> --- a/configure
>> +++ b/configure
>> @@ -1438,6 +1438,15 @@ fi
>> 
>> if test "$nptl" != "no" ; then
>>   cat > $TMPC <<EOF
>> +#ifdef __FreeBSD__
>> +#include <sys/thr.h>
>> +int main(void) {
>> +#if !defined(THR_SUSPENDED)
>> +#error bork
>> +#endif
>> +  return 0;
>> +}
>> +#else
>> #include <sched.h>
>> #include <linux/futex.h>
>> int main(void) {
>> @@ -1446,6 +1455,7 @@ int main(void) {
>> #endif
>>   return 0;
>> }
>> +#endif
>> EOF
>> 
>>   if compile_object ; then
>> @@ -4202,6 +4212,7 @@ case "$target_name" in
>>     TARGET_ARCH=mips64
>>     TARGET_BASE_ARCH=mips
>>     echo "TARGET_ABI_MIPSN64=y" >> $config_target_mak
>> +    target_nptl="yes"
>>   ;;
>>   moxie)
>>   ;;
> 
> I think this is the wrong way to do this. target_nptl should
> be a linux-user specific define (and we really ought to try
> to get rid of it altogether there at some point).
> 
> For bsd-user, the ideal would be if you could just support
> guest threads always, ie no #defines or configure checks needed.
> If you do need a configure check, then it should be its own,
> totally separate from CONFIG_NPTL. In any case, definitely
> do not make the Linux mistake of having "do we support threads?"
> be target specific if you can avoid it. (You might not be
> able to avoid it, though -- not sure :-( )

Yes, my plan is to get rid of the NPTL flag at some point for bsd-user once the 
FreeBSD thread emulation code proves to be stable.  (Yes, I was lazy and didn't 
create a new thread flag for FreeBSD in configure.)  It seems to fairly stable 
now but it is nice to turn off thread emulation now and then to eliminate the 
possibility of threads being a problem.  I suspect that we can just turn 
threads up for good and be all right, however.

> (I suspect these other MIPS targets should actually have
> target_nptl set as your patch fragment does, but that should
> be done as a linux-user bugfix patch, not buried in the
> bsd-user support patchset.)
> 
>> diff --git a/include/qemu/tls.h b/include/qemu/tls.h
>> index b92ea9d..ae7d79d 100644
>> --- a/include/qemu/tls.h
>> +++ b/include/qemu/tls.h
>> @@ -38,7 +38,7 @@
>>  * TODO: proper implementations via Win32 .tls sections and
>>  * POSIX pthread_getspecific.
>>  */
>> -#ifdef __linux__
>> +#if defined(__linux__) || defined(__FreeBSD__)
>> #define DECLARE_TLS(type, x) extern DEFINE_TLS(type, x)
>> #define DEFINE_TLS(type, x)  __thread __typeof__(type) tls__##x
>> #define tls_var(x)           tls__##x
> 
> This should be its own patch (especially as it affects the
> system emulation code on FreeBSD hosts).

Yes, that is a good point.  I separate into its own patch so it can be reviewed 
and tested individually.

-stacey.