This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 4/4] rseq registration tests (v2)


----- On Mar 27, 2019, at 5:21 PM, Carlos O'Donell codonell@redhat.com wrote:

> 
>> +
>> +static int
>> +do_rseq_threads_test (int nr_threads)
>> +{
>> +  pthread_t th[nr_threads];
>> +  int i;
>> +  int result = 0;
>> +  pthread_attr_t at;
>> +
>> +  if (pthread_attr_init (&at) != 0)
>> +    {
>> +      FAIL_EXIT1 ("attr_init failed");
>> +    }
>> +
>> +  if (pthread_attr_setstacksize (&at, 1 * 1024 * 1024) != 0)
> 
> Why set it that small? Why set it at all?

I copied that boilerplate code from pre-existing glibc nptl tests,
in a true "monkey see, monkey do" fashion.

If we look at e.g. nptl/tst-fork1.c, those stack size limitations
were introduced by this commit:

commit 4d1a02efc1117763c67fe012642381e3106500cf
Author: Ulrich Drepper <drepper@redhat.com>
Date:   Sun Mar 7 10:40:53 2004 +0000

    Update.

    2004-03-07  Ulrich Drepper  <drepper@redhat.com>
    
            * tst-once4.c: Remove unnecessary macro definition.
    
            * tst-mutex7.c (do_test): Limit thread stack size.
            * tst-once2.c (do_test): Likewise.
            * tst-tls3.c (do_test): Likewise.
            * tst-tls1.c (do_test): Likewise.
            * tst-signal3.c (do_test): Likewise.
            * tst-kill6.c (do_test): Likewise.
            * tst-key4.c (do_test): Likewise.
            * tst-join4.c (do_test): Likewise.
            * tst-fork1.c (do_test): Likewise.
            * tst-context1.c (do_test): Likewise.
            * tst-cond2.c (do_test): Likewise.
            * tst-cond10.c (do_test): Likewise.
            * tst-clock2.c (do_test): Likewise.
            * tst-cancel10.c (do_test): Likewise.
            * tst-basic2.c (do_test): Likewise.
            * tst-barrier4.c (do_test): Likewise.

And that commit does not state _why_ the thread stack size needs to
be limited. Nor on which architectures it matters, and what are the
parameters (e.g. number of threads) for which it matters.

In the case of rseq nptl tests, we spawn up to 50 threads in addition
to main.

Should we limit the thread stack size to 1MB ? That's indeed a good
question. Anyone would like to voice an opinion on the matter ?

>> +static void
>> +do_rseq_create_key (void)
>> +{
>> +  if (pthread_key_create (&rseq_test_key, rseq_key_destructor))
>> +    FAIL_EXIT1 ("error in pthread_key_create");
>> +}
>> +
>> +static void
>> +do_rseq_delete_key (void)
>> +{
>> +  if (pthread_key_delete (rseq_test_key))
>> +    FAIL_EXIT1 ("error in pthread_key_delete");
>> +}
> 
> OK. These two could do with becoming xthread_key_create/xthread_key_delete
> wrappers in support/*, but it's not critical.

I'll add them in a separate commit and modify this commit to use them
instead.

Thanks!

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]