Correct syscall name SYS_get_cpu used with old glibc where getcpu() is missing. #1897
Conversation
…s missing. Handle both SYS_get_cpu and SYS_getcpu, even though I am unsure if both ever existed, and make sure to error out if neither of them are available. Fixes ggml-org#1894.
petterreinholdtsen
force-pushed
the
old-glibc-get-cpu
branch
from
5613f24 to
7469011
Compare
| // old glibc doesn't have a wrapper for this call. Fall back on | ||
| // direct syscall | ||
| getcpu_ret = syscall( | ||
| # if defined(SYS_getcpu) | ||
| SYS_getcpu, | ||
| # elif defined(SYS_get_cpu) | ||
| SYS_get_cpu, | ||
| # else | ||
| # error "Unable fo find getcpu syscall define" | ||
| # endif /* SYS_getcpu */ | ||
| ¤t_cpu,&g_state.numa.current_node); |
There was a problem hiding this comment.
I think something like this would be more readable:
| // old glibc doesn't have a wrapper for this call. Fall back on | |
| // direct syscall | |
| getcpu_ret = syscall( | |
| # if defined(SYS_getcpu) | |
| SYS_getcpu, | |
| # elif defined(SYS_get_cpu) | |
| SYS_get_cpu, | |
| # else | |
| # error "Unable fo find getcpu syscall define" | |
| # endif /* SYS_getcpu */ | |
| ¤t_cpu,&g_state.numa.current_node); | |
| # if !defined(SYS_getcpu) && defined(SYS_get_cpu) | |
| # define SYS_getcpu SYS_get_cpu | |
| # endif | |
| // old glibc doesn't have a wrapper for this call. Fall back on direct syscall | |
| getcpu_ret = syscall(SYS_getcpu, ¤t_cpu, &g_state.numa.current_node); |
There was a problem hiding this comment.
@cebtenzzre that looks like an elegant solution.
@petterreinholdtsen are you able to test cebtenzzre's proposed fix and confirm that it works in your case?
|
[bmwl]
@cebtenzzre that looks like an elegant solution.
@petterreinholdtsen are you able to test cebtenzzre's proposed fix and
confirm that it works in your case?
I did not really see the point to test it, as it seemed obvious that his
proposal would work too. The only difference is how the two proposals
will fail if both of the two defines are missing. His proposal will
report the unknown symbol, while mine will report the message in the
#error line.
In any case, because you asked and it only take a few minutes, I did a
test build on Debian 12 Bookworm of his proposal and can confirm that it
build just fine here.
…--
Happy hacking
Petter Reinholdtsen
|
True enough. I suppose what we really need is @themanyone to test to make sure the regression on his OS is fixed with this patch as its only very old Linux kernels that would be hit with this issue. |
|
The issue has been resolved with the changes made in ggml-org/llama.cpp#5906 |
4 participants
Fixes #1894.