Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android and old glibc NUMA incompatibility bugfixes #5557

Merged
merged 7 commits into from
Feb 19, 2024

Conversation

bmtwl
Copy link
Contributor

@bmtwl bmtwl commented Feb 17, 2024

#ifdef out some code NUMA blocks for Android due to lack of support
Attempt at addressing Android build problem.
I don't have an Android dev environment, so this may take a few tries

@bmtwl bmtwl mentioned this pull request Feb 17, 2024
root added 3 commits February 18, 2024 00:07
…IBC prior to 2.29 to use a syscall for getcpu instead of the wrapper
…hat's the only model that's being followed anyways
@bmtwl bmtwl changed the title Android NUMA incompatibility bugfixes Android and old glibc NUMA incompatibility bugfixes Feb 18, 2024
bmtwl referenced this pull request Feb 18, 2024
* Added numa options to allow finer grained control as well as plumbing for a new mirror mode that will require numa.h

* Reverted Makefile

* Fixed include

* Removed sched.h from ggml.h, moved ggml_get_numa_affinity into ggml.c, removed trailing whitespace and fixed up a few inconsistent variables

* removed trailing whitespace

* Added numa options to allow finer grained control as well as plumbing for a new mirror mode that will require numa.h

* Reverting Makefile

* Fixed a number of issues with the move from BOOL to ggml_numa_strategies. Added a note about mirror mode note being implemented yet

* Removing MIRROR_MODE code for this PR

* Removing last bit of MIRROR_MODE code for this PR

* Removing unneeded branch in server.cpp example and moving get_numa_affinity and making it static

* Fixed lingering init_llama_backend() bool calls in tests and examples

* Remote enum llama_numa_strategies

* Revert bad merge with dynatemp flags

* add missing enum ggml_numa_strategies declaration and revert sync problem with master

* add missing enum ggml_numa_strategies declaration

* fixed ggml_init_numa variable

* Update ggml.h

Co-authored-by: Jared Van Bortel <cebtenzzre@gmail.com>

* Update READMEs with info about numa flags, change INTERLEAVE strategy name to DISTRIBUTE everywhere, implement the improved distribution strategy from @rankaiyx, fix a spelling mistake and un-merge some bad merges

* split numa init out from llama_backend_init and created llama_numa_init. Updated all code paths and samples

* Fix up some boolean vs enum comparisons

* Added #ifdefs for non-Linux OS that don't have cpu_set_t datatype

* Update ggml.h

Align enum values

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>

* Update ggml.c

Remove whitespace

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>

* Update ggml.c

align paremeters

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>

* Update examples/server/server.cpp

remove whitespace and align brace

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>

* Update common/common.cpp

Remove whitespace and align brace

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>

* unified ggml_numa_strategy enum and fixed text alignment in server.cpp example

* Update ggml.c

simplified return for platforms without NUMA support

Co-authored-by: Jared Van Bortel <cebtenzzre@gmail.com>

* removed redundant else from cli argument processing of --numa

* whitespace

---------

Co-authored-by: root <root@nenya.lothlorien.ca>
Co-authored-by: Jared Van Bortel <cebtenzzre@gmail.com>
Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
Co-authored-by: Jared Van Bortel <jared@nomic.ai>
@bmtwl
Copy link
Contributor Author

bmtwl commented Feb 18, 2024

@ggerganov do you see any problems with this PR? I've tested across Linux and Windows including old glibc for the syscall
Is there anyone else that should look it over, or do you want to look at a different strategy?

@ggerganov
Copy link
Owner

Let's wait for the CI. We know the android build will fail because it fetches from master, so if the others are green we can merge

@Jeximo
Copy link
Contributor

Jeximo commented Feb 18, 2024

Confirmed this PR let me build in Termux on my Android device.

@thistleknot
Copy link

this pr allowed me to build on ol8

@ggerganov ggerganov merged commit f0d1faf into ggerganov:master Feb 19, 2024
43 of 54 checks passed
@LostRuins
Copy link
Collaborator

LostRuins commented Feb 21, 2024

Am getting a compile error after this:

undeclared identifier SYS_getcpu

getcpu_ret = syscall(SYS_getcpu,&current_cpu,&g_state.numa.current_node);

I cannot find any reference to SYS_getcpu anywhere else.

image

@bmtwl
Copy link
Contributor Author

bmtwl commented Feb 21, 2024

@LostRuins what system is this being compiled under? OS, release, kernel version and glibc versions?
What are your make parameters?
Thanks!

@LostRuins
Copy link
Collaborator

It's a debian based runpod, but i'm not too sure on the specifics, sorry.

@bmtwl
Copy link
Contributor Author

bmtwl commented Feb 23, 2024

@LostRuins try "uname -a" and "ld --version"
The getcpu syscall has been around since the ancient "Linux 2.6.19 (x86-64 and i386), glibc 2.29" days, so its puzzling that it wouldn't work.
Those two commands above will let us know if its on some unusual CPU architecture or LIBC so I can troubleshoot further.

@PeronGH
Copy link

PeronGH commented Feb 23, 2024

@bmtwl I encountered the same issue and here is some info about my system:

$ uname -a
Linux xxx 3.10.0-1160.105.1.el7.x86_64 #1 SMP Mon Nov 6 06:58:51 EST 2023 x86_64 x86_64 x86_64 GNU/Linux

$ gcc --version
gcc (conda-forge gcc 13.2.0-5) 13.2.0
...

$ ldd --version
ldd (GNU libc) 2.17
...

I opened a PR attempting to fix the issue #5694

jordankanter pushed a commit to jordankanter/llama.cpp that referenced this pull request Mar 13, 2024
…#5557)

* #ifdef out some code NUMA blocks for Android due to lack of support

* added in some __ANDROID__ if def gates around numa code and forced GLIBC prior to 2.29 to use a syscall for getcpu instead of the wrapper

* Changed gates on numa platform specific stuff to __gnu_linux__ to skip any platforms without glibc

* harmonizing #if defined blocks for numa code to __gnu_linux__ since that's the only model that's being followed anyways

---------

Co-authored-by: root <root@nenya.lothlorien.ca>
hodlen pushed a commit to hodlen/llama.cpp that referenced this pull request Apr 1, 2024
…#5557)

* #ifdef out some code NUMA blocks for Android due to lack of support

* added in some __ANDROID__ if def gates around numa code and forced GLIBC prior to 2.29 to use a syscall for getcpu instead of the wrapper

* Changed gates on numa platform specific stuff to __gnu_linux__ to skip any platforms without glibc

* harmonizing #if defined blocks for numa code to __gnu_linux__ since that's the only model that's being followed anyways

---------

Co-authored-by: root <root@nenya.lothlorien.ca>
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.

6 participants