Zygo / bees

Best-Effort Extent-Same, a btrfs dedupe agent
GNU General Public License v3.0
647 stars 55 forks source link

glibc alternative musl does not define pthread_getname_np() #143

Closed kakra closed 2 years ago

kakra commented 4 years ago

This is a hack and probably wrong.

kakra commented 4 years ago

@Zygo It looks like you need this function solely to store the original thread name and restore it later. Could the proposed thread name just be stored somewhere else instead, and restored from there? So every worker would know its name and just set it when it's running?

Zygo commented 4 years ago

You can mostly replace it with a function that reads or stores a string in a thread_local variable; however, without pthread_setname_np, the thread names will not show up in kernel logs, iotop, and other externally visible places.

kakra commented 4 years ago

You can mostly replace it with a function that reads or stores a string in a thread_local variable; however, without pthread_setname_np, the thread names will not show up in kernel logs, iotop, and other externally visible places.

Actually, pthread_SETname_np is not the problem, I just thought: Why set it when we don't get it. ;-) The single real problem is: pthread_GETname_np does not exist in musl.

But back to topic: We could emulate the getter function by storing a thread local variable which we'll sync to the kernel through the setter whenever the name should change?

mjeveritt commented 4 years ago

Depending on what you want/need to achieve, you may be interested in the libunwind project.

stintel commented 3 years ago

I doubt checking for _GNU_SOURCE is the right check here, as for some applications to build against musl libc, it's actually necessary to define _GNU_SOURCE.

@kakra to confirm the above, I can't even build bees on Gentoo with your patch and I'm not defining _GNU_SOURCE via /etc/portage/{package.,}env

Zygo commented 3 years ago

pthread_{get,set}name_np set the externally visible thread name, so the thread will be marked "hash_writeback" or "crawl_256" in iotop, gdb, kernel stack traces, etc.

The glibc implementation does something with /proc--and it will crash if you don't have /proc mounted in the process's namespace.

If we just want it to build, you could replace pthread_setname_np with something that copies the string to a thread_local global variable. This removes the external visibility of the thread name, but it will still appear in bees-generated log messages.

The tricky part is figuring out whether pthread_setname_np exists in the build environment--we either need to add a feature test to the build system, or need to find a #define in musl that we can use as a proxy for pthread_setname_np. Ideally, musl would have these functions implemented somewhere under a slightly different name or behind a feature macro that can be enabled.

libunwind is far too low level for this. The stack traces are meant to give a high-level report of what bees is doing, including filenames and function arguments, and not including 20 layers of C++ template goo that would appear in the machine-level stack trace. Also it's important for performance that the stack trace code is not executed unless there is a stack trace being printed--all the trace messages are buried in function objects for lazy evaluation, and the stack trace printer will execute them as it runs.

Zygo commented 3 years ago

Wait, does musl have set but not get? We can totally emulate the get function.

stintel commented 3 years ago

I'm currently doing this and defining __MUSL__ in CXXFLAGS in the Gentoo ebuild, which works:

https://github.com/stintel/bees/commit/10fdeef1982fba0542a1780dd4ea25ca61558ca5

But still a hack.

stintel commented 3 years ago

Wait, does musl have set but not get? We can totally emulate the get function.

Indeed. See https://www.openwall.com/lists/musl/2019/07/17/3 - I'll ping Rich about the matter.

stintel commented 3 years ago

Indeed. See https://www.openwall.com/lists/musl/2019/07/17/3 - I'll ping Rich about the matter.

Good news, it should be in the next release: https://twitter.com/stintel/status/1397917157187493900

kakra commented 3 years ago

Cool, so I could push a new ebuild when musl stabilizes in Gentoo? Which version then?

stintel commented 3 years ago

Cool, so I could push a new ebuild when musl stabilizes in Gentoo? Which version then?

I just checked musl.git but I don't see it yet :(

kakra commented 3 years ago

I removed the hacky patch from the 9999 ebuild, hoping that musl will add the function in time. The patch had multiple conflicts now on rebase.

Zygo commented 2 years ago

musl git contains this commit:

commit bd3b9c4ca5e93f10f7fd891b8c07cc0c5dfd198f
Author: �rico Rolim <ericonr@disroot.org>
Date:   Tue Apr 20 16:15:15 2021 -0300

    add pthread_getname_np function

    based on the pthread_setname_np implementation

Does that mean we can close this now?

kakra commented 2 years ago

Yeah, let's close it. Sooner or later, that commit will land in musl - and that's it. I'm not even sure if we should state that musl is a supported libc implementation for bees, people are on their own. If it is missing functions that other software needs, and other implementations provide, it's the job of the musl project or downstream package maintainer to provide proper patches to make it work with other applications of the distribution.

So, if anything, maybe add to the documentation that glibc is the supported and tested libc implementation, and other may work but are not officially supported.