Skip to content

Conversation

@petterreinholdtsen
Copy link
Contributor

Fixes #1894.

…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.
Comment on lines +2087 to +2097
// 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 */
&current_cpu,&g_state.numa.current_node);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think something like this would be more readable:

Suggested change
// 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 */
&current_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, &current_cpu, &g_state.numa.current_node);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

@petterreinholdtsen
Copy link
Contributor Author

petterreinholdtsen commented Mar 6, 2024 via email

@bmtwl
Copy link
Contributor

bmtwl commented Mar 6, 2024

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.

@bobqianic
Copy link
Collaborator

The issue has been resolved with the changes made in ggml-org/llama.cpp#5906
Therefore, this pull request will now be closed.

@bobqianic bobqianic closed this Mar 6, 2024
@petterreinholdtsen petterreinholdtsen deleted the old-glibc-get-cpu branch May 14, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ggml.c: error: 'SYS_getcpu' undeclared did you mean 'SYS_get_cpu'

4 participants