On Fri, 11 Oct 2024 at 03:15, Robert Engels <reng...@ix.netcom.com> wrote: > > That’s what I was thinking. Thanks for providing the details. Rule of thumb - > if you’re trying to do something and you’re the only one that’s trying it - > you’re probably doing something wrong - or at least there is a much easier > way.
I wish I could take the credit for the idea, but one of our contributors referred me to this package: https://github.com/Jille/gcmmap. Yes, it doesn't have any importers and it's unpolished, but we thought the idea had merit, so we adapted it to our use case. As for there being an easier way -- it's hard to argue against KISS, but the example I gave leaves out many details that made us prefer this approach, see below. > On Oct 10, 2024, at 6:31 PM, 'Keith Randall' via golang-nuts > <golang-nuts@googlegroups.com> wrote: > > type Variable struct { > *atomic.Uint64 > m *Memory > } > > I agree that if you have a finalizer on a Variable that unmaps m, and you've > given out a reference to the internal Uint64 to callers, then you can get in > a situation where the Variable is dead, its finalizer is run, and the memory > is unmapped while someone is doing operations on the atomic Uint64. > > So don't embed like that. Instead just have a private field and implement the > methods you need. Like: > > type Variable struct { > u *atomic.Uint64 > m *Memory > } > func (v *Variable) Load() uint64 { > x := v.u.Load() > runtime.KeepAlive(v) > return x > } Yes, we couldn't allow the caller to obtain a reference to Uint64 because of the risk of losing the Memory pointer. My initial implementation was slightly different; it defined unexported type aliases for all integer types in sync/atomic and embedded those instead, so the caller could only invoke the methods promoted from e.g. atomic.Uint64 to Variable. However, that still left the possibility of Memory becoming unreachable at the start of atomic.Uint64.Load(), we'd need to wrap all accessors with runtime.KeepAlive() as you suggested above. The reason this wasn't feasible is because the size of a Variable is not known (by us, the library) at compile time. We parse an eBPF ELF into a bunch of objects, including a slice of Variables, which essentially includes all global vars and consts in the ELF. A Variable has a size and an offset into a Memory. We can't make a Variable generic, because again, types are not known at Go compile time and different-size Variables need to be put into a slice. Additionally, Variables can be structs as well, and don't necessarily need to be atomic. My initial implementation contained a bunch of boilerplate for atomics, but this relegated all struct operations to reflection-based marshaling using Variable.Set() and .Get() (some amalgamation of binary.Read and .Write underneath). At the end of the day, to keep things somewhat flexible and ergonomic, this was the only option we were left with. Now, the caller can do: ``` type foo struct { a, b, uint64 } var v *Variable s, _ := VariablePointer[foo](v) // returns (*foo, error) foo.a = 123 ``` VariablePointer performs a bounds check on the underlying Memory and the size of foo, and ensures it doesn't contain any pointers. (binary.Size will enforce this) Accesses to foo now go to shared kernel memory. foo can be passed around the Go app without fear of the underlying memory getting unmapped. *Memory and *Variable can be lost without issues. > ... and ditto for other methods of atomic.Uint64 that your clients need. It's > a bit of boilerplate, but it should work without having to mmap Go heap pages. > > > On Thursday, October 10, 2024 at 12:41:45 PM UTC-7 Robert Engels wrote: >> >> I think you had a bug in your Memory implementation. That is the correct way >> to implement and it’s straightforward. Not sure why you ran into a problem. >> >> > On Oct 10, 2024, at 9:31 AM, 'Timo Beckers' via golang-nuts >> > <golan...@googlegroups.com> wrote: >> > >> > On Thu, 10 Oct 2024 at 14:48, Jan Mercl <0xj...@gmail.com> wrote: >> >> >> >>> On Thu, Oct 10, 2024 at 2:23 PM 'Timo Beckers' via golang-nuts >> >>> <golan...@googlegroups.com> wrote: >> >>> >> >>> I've been searching around for some info or existing conversations >> >>> around this topic, but that hasn't turned up anything useful so far. I >> >>> had a question around some implicit behaviour of Go's heap allocator. >> >> >> >> From the specs point of view, there's no heap and no allocator. The >> >> language specification covers allocation at >> >> https://go.dev/ref/spec#Allocation, but the allocator is an >> >> implementation detail with no particular guarantees. >> > >> > Thanks, I didn't realize this was mentioned in the spec. I guess that >> > puts an end to the conversation immediately. :) >> > I'll allocate an extra page to make sure we can always align to a page >> > boundary. >> > >> >> >> >>> I'm working on implementing BPF map operations through direct shared >> >>> memory access (without going through syscalls). To make the API somewhat >> >>> ergonomic, I've settled on mmapping BPF map (kernel) memory over a part >> >>> of the Go heap using MAP_FIXED. Feel free to tell me how bad of an idea >> >>> this is, I can elaborate if needed. >> >> >> >> I'm not sure what exactly do you mean by "over" as there is more than >> >> one possibility I can think of. Go managed memory and manually mmaped >> >> memory can intermix under certain conditions. One of them is that the >> >> runtime must know which is which, ie. be able to distinguish pointers >> >> to memory allocated by itself from memory allocated by something else >> >> - manually mmaped, C allocated etc. >> >> >> > >> > Precisely, we want this memory to be both tracked by the GC _and_ >> > backed by shared kernel memory. >> > I can only hope this isn't too common. :) >> > >> >>> In order for this to work and to minimize allocations, I need a heap >> >>> allocation that starts on a page boundary. Initially, I experimented >> >>> with //go:linkname'ing mallocgc(), but I figured allocating a regular >> >>> slice the size of a page has basically the same effect. Here's a >> >>> playground link: https://go.dev/play/p/ua2NJ-rEIlC. As long as the >> >>> slice/backing array is a multiple of the architecture's page size, it >> >>> seems to start on a page boundary. I've tried allocating a bunch of >> >>> ballast, forcing GCs, etc. and it hasn't failed once. >> >> >> >> Those are guarantees possibly provided by the kernel. No promises from Go. >> > >> > The kernel guarantees handing out page-aligned slabs of memory to Go, >> > but after that point, isn't it up to Go to slice that up however it >> > pleases. >> > >> >> >> >>> Here's my question: which property of the Go allocator is this banking >> >>> on? Intuitively, this makes sense to me. An 8-byte alloc needs to be >> >>> 8-byte aligned in case it's a pointer. Does a 4k allocation need to be >> >>> 4k-aligned as well (e.g. in case it's a struct), since a compiler would >> >>> align members to the start of the struct? I'm reading larger (> 8 or 16 >> >>> bytes depending on arch?) allocs have an alignment of 1 byte, and >> >>> unsafe.Alignof([4096]byte) tells me the same, but that's not the >> >>> behaviour exhibited by the allocator. >> >> >> >> Implementation details that can change. You cannot rely on it. You >> >> have to code it manually and look for platform details to work out. >> >> >> >> -j >> > >> > Yep, clear. Thanks again for the pointer to the spec, not sure why I >> > didn't check there first. >> > >> > Take care, >> > >> > Timo >> > >> > -- >> > You received this message because you are subscribed to the Google Groups >> > "golang-nuts" group. >> > To unsubscribe from this group and stop receiving emails from it, send an >> > email to golang-nuts...@googlegroups.com. >> > To view this discussion on the web visit >> > https://groups.google.com/d/msgid/golang-nuts/CANgQc9gD3uphTpX3V46caqQvuSyNJ3ieOuz%2BOJJcyXPThTi%3D1Q%40mail.gmail.com. > > -- > You received this message because you are subscribed to the Google Groups > "golang-nuts" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-nuts+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/golang-nuts/65ec6573-03a0-4586-8ca1-2da2497628a0n%40googlegroups.com. > > -- > You received this message because you are subscribed to a topic in the Google > Groups "golang-nuts" group. > To unsubscribe from this topic, visit > https://groups.google.com/d/topic/golang-nuts/LzUMguyouXM/unsubscribe. > To unsubscribe from this group and all its topics, send an email to > golang-nuts+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/golang-nuts/F9F590B4-A75E-4846-BF3E-A53AE41A2489%40ix.netcom.com. -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CANgQc9id%3DAa-C%3DL8Hqyi__W%2BcreY1N8m8j22mThNq-3g1TWXQQ%40mail.gmail.com.
