Re: [PATCH] Fix nptl/tst-cond1{6,7} on 32-bit with many cpus

["Carlos O'Donell" <carlos at systemhalted dot org>]

On Sun, Mar 25, 2012 at 9:47 PM, David Miller <davem@davemloft.net> wrote:
>
> On my machine NR_CPUS=128 and ulimit -s is 8MB. ?nptl/tst-cond1{6,7}
> tries to create NR_CPUS * 4 threads.
>
> And the amount of stack that tries to allocate is... pretty much the
> entire 32-bit address space.
>
> We're just calling one function which invokes some pthread routines.
> Therefore using 64K of stack ought to be enough. ?Just to be safe
> I make sure we use a value which is at least __getpagesize(). ?At
> least on powerpc, sparc, and IA-64 larger default page sizes are
> possible.
>
> Any objections?

No objection to the idea of the fix, but I have some questions about
making it more robust.

> nptl/
>
> 2012-03-25 ?David S. Miller ?<davem@davemloft.net>
>
> ? ? ? ?* tst-cond16.c (do_test): Use a thread stack size which is either
> ? ? ? ?64K or the page size, whichever is larger.
>
> diff --git a/nptl/tst-cond16.c b/nptl/tst-cond16.c
> index 44b9863..388acfe 100644
> --- a/nptl/tst-cond16.c
> +++ b/nptl/tst-cond16.c
> @@ -76,9 +76,15 @@ do_test (void)
> ? count *= 4;
>
> ? pthread_t th[count];
> - ?int i, ret;
> + ?pthread_attr_t attr;
> + ?int i, ret, sz;
> + ?pthread_attr_init (&attr);
> + ?sz = __getpagesize ();
> + ?if (sz < 64 * 1024)
> + ? ? ? ? sz = 64 * 1024;

Should this be PTHREAD_STACK_MIN instead to allow for per-machine
variations? Is it sufficient to use PTHREAD_STACK_MIN?

Once we resolve BZ#11787 we'll have a clearer notion of what qualifies
as stack i.e. does it include implementation details like static TLS,
the thread descriptor, and a guard page? We appear to be heading
towards the consensus that nptl/allocatestack.c or ntpl/nptl-init.c
should not coun't implementation details in the allocation of stacks
fully under control of the implementation.

> + ?pthread_attr_setstacksize (&attr, sz);
> ? for (i = 0; i < count; ++i)
> - ? ?if ((ret = pthread_create (&th[i], NULL, tf, NULL)) != 0)
> + ? ?if ((ret = pthread_create (&th[i], &attr, tf, NULL)) != 0)
> ? ? ? {
> ? ? ? ?errno = ret;
> ? ? ? ?printf ("pthread_create %d failed: %m\n", i);

Cheers,
Carlos.