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

bazel: fix numactl #25041

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

bazel: fix numactl #25041

wants to merge 3 commits into from

Conversation

rockwotj
Copy link
Contributor

@rockwotj rockwotj commented Feb 5, 2025

Incorporate libnuma into the Bazel build. We force dynamic linking because it's LGPL. The dynamic linking forcing is really annoying, but honestly less annoying than the new way of doing things with cc_shared_library. However I ran into issues using the cc_shared_library approach, so I just fallback to using the "old school" stable ways of making shared libraries.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.3.x
  • v24.2.x
  • v24.1.x

Release Notes

  • none

This breaks in bazel 8, so fix it now
required to work with Bazel 8
@rockwotj rockwotj marked this pull request as ready for review February 7, 2025 22:12
@dotnwat
Copy link
Member

dotnwat commented Feb 7, 2025

but honestly less annoying than the new way of doing things with cc_shared_library

i'm not saying we should do this, but from what I can tell seastar's libnuma dependency is a single call to mbind in memory.cc to set a policy on some memory regions.

looking at this:

https://github.com/lrita/numa/blob/29f88905963797c57362861a2dd8adb14a6f152a/numa_linux.go#L161

i do wonder if freeing ourselves from the libnuma dependency would be as simple as writing our own version of this

func MBind(addr unsafe.Pointer, length, mode, flags int, nodemask Bitmask) (err error) {
	var mask, maxnode uintptr
	if maxnode = uintptr(nodemask.Len()); maxnode != 0 {
		mask = uintptr(unsafe.Pointer(&nodemask[0]))
	}
	_, _, errno := syscall.Syscall6(syscall.SYS_MBIND, uintptr(addr),
		uintptr(length), uintptr(mode), mask, maxnode, uintptr(flags))
	if errno != 0 {
		err = errno
	}
	return
}

@@ -6,7 +6,8 @@ edition = "2021"
publish = false

[lib]
path = "/dev/null"
# Use a fake path here, but rules_rust requires something.
path = "./dev/null"
Copy link
Member

Choose a reason for hiding this comment

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

nice. i had gone with fake.rs

@rockwotj
Copy link
Contributor Author

rockwotj commented Feb 8, 2025

but honestly less annoying than the new way of doing things with cc_shared_library

i'm not saying we should do this, but from what I can tell seastar's libnuma dependency is a single call to mbind in memory.cc to set a policy on some memory regions.

looking at this:

https://github.com/lrita/numa/blob/29f88905963797c57362861a2dd8adb14a6f152a/numa_linux.go#L161

i do wonder if freeing ourselves from the libnuma dependency would be as simple as writing our own version of this


func MBind(addr unsafe.Pointer, length, mode, flags int, nodemask Bitmask) (err error) {

	var mask, maxnode uintptr

	if maxnode = uintptr(nodemask.Len()); maxnode != 0 {

		mask = uintptr(unsafe.Pointer(&nodemask[0]))

	}

	_, _, errno := syscall.Syscall6(syscall.SYS_MBIND, uintptr(addr),

		uintptr(length), uintptr(mode), mask, maxnode, uintptr(flags))

	if errno != 0 {

		err = errno

	}

	return

}

ooh maybe I will send a patch to seastar and see what drama unfolds...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants