Re: [PATCH] powerpc: Use sysconf (_SC_SIGSTKSZ) to set SIGSTKSZ and MINSIGSTKSZ.

[Tulio Magno Quites Machado Filho <tuliom@ascii.art.br>]

Manjunath Matti via Libc-alpha <libc-alpha@sourceware.org> writes:

> Add support in PowerPC to use sysconf (_SC_SIGSTKSZ) to set SIGSTKSZ
> and MINSIGSTKSZ similar to x86.

This commit message explains what is being done, but it doesn't make it
clear why this commit is important.
If I understand correctly, the goal is to have dynamic values for the
signal stack size and minimum signal stack size. Is this correct?

I also suggest to mention the kernel commit ID that started doing this:
2896b2dff49d0377e4372f470dcddbcb26f2be59

> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/sigstksz.h b/sysdeps/unix/sysv/linux/powerpc/bits/sigstksz.h
> new file mode 100644
> index 0000000000..2bec1e7917
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/sigstksz.h
> @@ -0,0 +1,33 @@
> +/* Definition of MINSIGSTKSZ and SIGSTKSZ.  Linux/PowerPC version.
> +   Copyright (C) 2020-2023 Free Software Foundation, Inc.

                    ^ I believe this should be just 2023.

> +#if defined __USE_DYNAMIC_STACK_SIZE && __USE_DYNAMIC_STACK_SIZE
> +# include <unistd.h>
> +
> +/* Default stack size for a signal handler: sysconf (SC_SIGSTKSZ).  */
> +# undef SIGSTKSZ
> +# define SIGSTKSZ sysconf (_SC_SIGSTKSZ)

I have been told that structs could be using SIGSTKSZ to set their size.
If that's happening, the softwares using them will stop building after this
change.
Is this reasonable?

> +/* Minimum stack size for a signal handler: SIGSTKSZ/4.  */
> +# undef MINSIGSTKSZ
> +# define MINSIGSTKSZ (SIGSTKSZ >> 2)
> +#endif

I didn't understand this part.
Why SIGSTKSZ/4 ? I know this is correct now, but I think the kernel is
allowed to use another value.
Why is this part not using sysconf(_SC_MINSIGSTKSZ)?
I'm not suggesting to use sysconf() here, but I'm trying to understand
why the same source of value for both SIGSTKSZ and MINSIGSTKSZ is not
being used.

If we reach consensus that both macros in this file can have values set
at runtime, then I it might be worth adding a test in order to check that
dl_minsigstacksize, MINSIGSTKSZ and AT_MINSIGSTKSZ passed by the kernel
are identical.

-- 
Tulio Magno