Bug 444488 - Use glibc.pthread.stack_cache_size tunable
Summary: Use glibc.pthread.stack_cache_size tunable
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Paul Floyd
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-10-27 14:32 UTC by Mark Wielaard
Modified: 2022-12-23 15:54 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
patch to set tunable (8.84 KB, patch)
2022-11-13 16:52 UTC, Paul Floyd
Details
Updated patch now handles existing GLIBC_TUNABLES and no lnger prints a warning (9.94 KB, patch)
2022-11-13 20:41 UTC, Paul Floyd
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Wielaard 2021-10-27 14:32:54 UTC
Since glibc 2.34 the internal/private stack_cache_maxsize variable isn't available anymore, which causes "sched WARNING: pthread stack cache cannot be disabled!" when the simhint no_nptl_pthread_stackcache is set (e.g. in helgrind/tests/tls_threads.vgtest)

See coregrind/pub_core_clientstate.h:

/* glibc nptl pthread systems only, when no-nptl-pthread-stackcache
   was given in --sim-hints.
   Used for a (kludgy) way to disable the cache of stacks as implemented in
   nptl glibc. 
   Based on internal knowledge of the pthread glibc nptl/allocatestack.c code:
   a huge value in stack_cache_actsize (bigger than the constant
   stack_cache_maxsize) makes glibc believes the cache is full
   and so stacks are always released when a pthread terminates.
   Several ugliness in this kludge:
    * hardcodes private glibc var name "stack_cache_maxsize"
    * based on knowledge of the code of the functions
      queue_stack and __free_stacks
    * static symbol for "stack_cache_maxsize" must be in
      the debug info.
   It would be much cleaner to have a documented and supported
   way to disable the pthread stack cache. */
extern SizeT* VG_(client__stack_cache_actsize__addr);

Instead we should use the new (in glibc 2.34) tunable:

@deftp Tunable glibc.pthread.stack_cache_size
This tunable configures the maximum size of the stack cache.  Once the
stack cache exceeds this size, unused thread stacks are returned to
the kernel, to bring the cache size below this limit.

The value is measured in bytes.  The default is @samp{41943040}
(fourty mibibytes).
@end deftp
Comment 1 Philippe Waroquiers 2022-11-12 13:20:15 UTC
In the discussion on valgrind-users mailing list,
Paul reported tthat:
  'It looks like "stack_cache_actsize" in libc moved to be _dl_stack_cache_actsize in ld-linux-x86-64.so.2'

Is there a way to modify the glibc glibc.pthread.stack_cache_size tunable from valgrind ?
Or do we assume that the user has to tune this ?
Or do we do an alternate implementation of the current valgrind hack using _dl_stack_cache_actsize in ld-linux-x86-64.so.2 ?
Comment 2 Mark Wielaard 2022-11-12 13:38:02 UTC
(In reply to Philippe Waroquiers from comment #1)
> In the discussion on valgrind-users mailing list,
> Paul reported tthat:
>   'It looks like "stack_cache_actsize" in libc moved to be
> _dl_stack_cache_actsize in ld-linux-x86-64.so.2'
> 
> Is there a way to modify the glibc glibc.pthread.stack_cache_size tunable
> from valgrind ?

tunables are set by the GLIBC_TUNABLES environment variable
https://www.gnu.org/software/libc/manual/html_node/Tunables.html

We can set/add to that GLIBC_TUNABLES environment variable in coregrind/m_initimg/initimg-linux.c setup_client_env () where we also set the LD_PRELOAD environment variable.
Comment 3 Philippe Waroquiers 2022-11-12 13:53:31 UTC
(In reply to Mark Wielaard from comment #2)
> (In reply to Philippe Waroquiers from comment #1)
> > In the discussion on valgrind-users mailing list,
> > Paul reported tthat:
> >   'It looks like "stack_cache_actsize" in libc moved to be
> > _dl_stack_cache_actsize in ld-linux-x86-64.so.2'
> > 
> > Is there a way to modify the glibc glibc.pthread.stack_cache_size tunable
> > from valgrind ?
> 
> tunables are set by the GLIBC_TUNABLES environment variable
> https://www.gnu.org/software/libc/manual/html_node/Tunables.html
> 
> We can set/add to that GLIBC_TUNABLES environment variable in
> coregrind/m_initimg/initimg-linux.c setup_client_env () where we also set
> the LD_PRELOAD environment variable.

When running with a newer glibc, we should also avoid producing an error message 
that the old way to disable the stack cache is not working.
Likely this implies to detect the version of glibc.
Comment 4 Paul Floyd 2022-11-12 14:14:44 UTC
Thanks for adding me to the CC Philippe.

If I do this:
export GLIBC_TUNABLES="glibc.pthread.stack_cache_size=0"

Then helgrind/tests/tls_threads fails with just
+--21937:0:   sched WARNING: pthread stack cache cannot be disabled!

Without the env var there are a load of 

+Possible data race during write of size 8 at 0x........ by thread #x

errors

Do we have a way of knowing that GLIBC_TUNABLES did something so that we don't need to twiddle with stack_cache_actsize?
Also --sim-hints=no-nptl-pthread-stackcache isn't turned on by default. Do we want to check for it in setup_client_env ()  and only put GLIBC_TUNABLES in the environment if it is used? Or perhaps add a new simhint.

Bonus points for handling GLIBC_TUNABLES already set by the tuner, and add or replace glibc.pthread.stack_cache_size

This doesn't seem to help the example being discussed in the valgrind-users list.
Comment 5 Philippe Waroquiers 2022-11-12 16:20:16 UTC
(In reply to Paul Floyd from comment #4)
> Thanks for adding me to the CC Philippe.
> 
> If I do this:
> export GLIBC_TUNABLES="glibc.pthread.stack_cache_size=0"
> 
> Then helgrind/tests/tls_threads fails with just
> +--21937:0:   sched WARNING: pthread stack cache cannot be disabled!
> 
> Without the env var there are a load of 
> 
> +Possible data race during write of size 8 at 0x........ by thread #x
> 
> errors
> 
> Do we have a way of knowing that GLIBC_TUNABLES did something so that we
> don't need to twiddle with stack_cache_actsize?
If we can detect the glibc version, then we can avoid using the old stack_cache_actsize hack.

> Also --sim-hints=no-nptl-pthread-stackcache isn't turned on by default. Do
> we want to check for it in setup_client_env ()  and only put GLIBC_TUNABLES
> in the environment if it is used? Or perhaps add a new simhint.
I think we should check and use the existing hint. Current users of the hint 
will/should have the same behaviour whatever the glibc version.

> 
> Bonus points for handling GLIBC_TUNABLES already set by the tuner, and add
> or replace glibc.pthread.stack_cache_size
> 
> This doesn't seem to help the example being discussed in the valgrind-users
> list.
Comment 6 Paul Floyd 2022-11-12 20:16:58 UTC
> I think we should check and use the existing hint. Current users of the hint 
> will/should have the same behaviour whatever the glibc version.

There is gnu_get_libc_version(). Now, how to call that without breaking musl.
Comment 7 Philippe Waroquiers 2022-11-12 21:17:46 UTC
(In reply to Paul Floyd from comment #6)
> > I think we should check and use the existing hint. Current users of the hint 
> > will/should have the same behaviour whatever the glibc version.
> 
> There is gnu_get_libc_version(). Now, how to call that without breaking musl.
I suspect it might be problematic to call a glibc function (as a host function) during client image initialisation.
So, we might have to always set the tunable env variable as part of the client initialisation,
and call gnu_get_libc_version before producing an error message that the old hack did not work.
Comment 8 Paul Floyd 2022-11-13 13:50:47 UTC
I'm having a go at this.

Setting GLIBC_TUNABLES isn't too difficult (though I haven't yet added handling when it is already set).

glibc version seems a lot harder.

In glibc:
paulf> nm /lib64/libc.so.6  | grep libc_version
00000000000296c0 t __gnu_get_libc_version
00000000000296c0 W gnu_get_libc_version
00000000001b8ada r __libc_version

I don't see how to access any of these.
Comment 9 Paul Floyd 2022-11-13 16:52:40 UTC
Created attachment 153718 [details]
patch to set tunable

This fixes helgrind/tests/tls_threads
But it only adds GLIBC_TUNABLES, it doesn't try to modify an existing one.
Comment 10 Paul Floyd 2022-11-13 20:41:52 UTC
Created attachment 153723 [details]
Updated patch now handles existing GLIBC_TUNABLES and no lnger prints a warning

Looks like this is now about ready. I haven't yet tested on other platforms.

I did try to chjeckout and build the latest code on Alpine but there was a link error (without these changes)..
Comment 11 Paul Floyd 2022-11-14 07:36:11 UTC
I've tested this on FreeBSD and musl. Both fail helgrind/tests/tls_threads both before and after so no change with this patch.
Comment 12 Paul Floyd 2022-12-23 15:54:08 UTC
commit 2de91d914cc5ed4ed7ea8e70d5e06c46e991f39f (HEAD -> master, origin/master, origin/HEAD)
Author: Paul Floyd <pjfloyd@wanadoo.fr>
Date:   Fri Dec 23 16:49:20 2022 +0100

    Bug 444488 - Use glibc.pthread.stack_cache_size tunable
    
    Try to use GLIBC_TUNABLES to disable the pthread stack
    cache.