Skip to content

proposal: x/sys/unix: GCed mmap #70673

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

Closed
Jorropo opened this issue Dec 4, 2024 · 3 comments
Closed

proposal: x/sys/unix: GCed mmap #70673

Jorropo opened this issue Dec 4, 2024 · 3 comments
Labels
Milestone

Comments

@Jorropo
Copy link
Member

Jorropo commented Dec 4, 2024

Proposal Details

Add theses functions:

// MmapGc is just like [Mmap] but the mapping will be destroyed if the GC is unable to find Go code holding a pointer anywhere this mapping.
// If the mapping is manually unmapped or truncated the GC will continue to track only the remaining pages if any and unmap theses as one smaller mapping once no Go code is found to hold a reference to the remaining mapping.
// If it this mapping is mapped over, this region is considered a new different mapping and they will be kept track individually.
func MmapGc(fd int, offset int64, length int, prot int, flags int) (data []byte, err error)

// MmapGcPtr is just like [MmapGc] but with [MmapPtr]'s signature, see [MmapGc] for rules and edgecases.
func MmapGcPtr(fd int, offset int64, addr unsafe.Pointer, length uintptr, prot int, flags int) (ret unsafe.Pointer, err error)


// MmapThrough is just like [Mmap] but it does not update the GC's tracking for mappings, that means mapping over parts of a GC mapping does not create two independently tracked GC mappings.
func MmapThrough(fd int, offset int64, length int, prot int, flags int) (data []byte, err error)

// MmapThroughPtr is just like [MmapThrough] but with [MmapPtr]'s signature, see [MmapThrough] for rules and edgecases.
func MmapThroughPtr(fd int, offset int64, addr unsafe.Pointer, length uintptr, prot int, flags int) (ret unsafe.Pointer, err error)

What is motivating this change ?

I have tried writing an mmu accelerated ring buffer however it is dangerous because the consumer is responsible for calling .Close after which all pointers to the ring buffer become invalid.
See Close's impl:

func (r *Ring) Close() error {
	buf := r.buffer
	if buf == nil {
		return nil
	}
	r.buffer = nil
	return unix.MunmapPtr(unsafe.Pointer(buf), r.size*2)
}

I am not holding any other resources than the two mappings, there is a backing in-memory file (memfd on linux) however the OS keeps track of that, the file is closed upon creation and the OS cleans up the file when the mappings are destroyed.

Example of how it would be used

 // Init initializes the ring buffer with the given size.
 func (r *Ring) Init(size uintptr) (err error) {
 	if r.buffer != nil {
 		return fmt.Errorf("ring already initialized")
 	}
 
 	if pageSize&(pageSize-1) != 0 {
 		return fmt.Errorf("page size must be a power of 2")
 	}
 
 	if size%pageSize != 0 {
 		return fmt.Errorf("size must be a multiple of the page size")
 	}
 
 	file, err := unix.MemfdCreate("github.com/Jorropo/mmu-ring", unix.MFD_CLOEXEC) // name does nothing and just used for debug
 	if err != nil {
 		return fmt.Errorf("memfd_create: %w", err)
 	}
 	defer unix.Close(file) // linux will cleanup the files once the mappings are unmapped
 
 	if err := unix.Ftruncate(file, int64(size)); err != nil {
 		return fmt.Errorf("ftruncate: %w", err)
 	}
 	totalMappingSize := size * 2
 	if totalMappingSize < size {
 		return fmt.Errorf("size overflow when creating MMU-ring")
 	}
 
 	// temporary mapping to allocate twice the ring of virtual memory
-	orig, err := unix.MmapPtr(-1, 0, nil, totalMappingSize, unix.PROT_READ|unix.PROT_WRITE, unix.MAP_ANONYMOUS|unix.MAP_NORESERVE|unix.MAP_PRIVATE)
+	orig, err := unix.MmapGcPtr(-1, 0, nil, totalMappingSize, unix.PROT_READ|unix.PROT_WRITE, unix.MAP_ANONYMOUS|unix.MAP_NORESERVE|unix.MAP_PRIVATE)
 	if err != nil {
 		return fmt.Errorf("virtual mmap: %w", err)
 	}
 	defer func() {
 		if err != nil {
 			unix.MunmapPtr(orig, totalMappingSize)
 		}
 	}()
 
 	// replace the virtual reservation with the physical memory tail to head
-	_, err = unix.MmapPtr(file, 0, orig, size, unix.PROT_READ|unix.PROT_WRITE, unix.MAP_SHARED|unix.MAP_FIXED)
+	_, err = unix.MmapThroughPtr(file, 0, orig, size, unix.PROT_READ|unix.PROT_WRITE, unix.MAP_SHARED|unix.MAP_FIXED)
 	if err != nil {
 		return fmt.Errorf("first physical mmap: %w", err)
 	}
-	_, err = unix.MmapPtr(file, 0, unsafe.Add(orig, size), size, unix.PROT_READ|unix.PROT_WRITE, unix.MAP_SHARED|unix.MAP_FIXED)
+	_, err = unix.MmapThroughPtr(file, 0, unsafe.Add(orig, size), size, unix.PROT_READ|unix.PROT_WRITE, unix.MAP_SHARED|unix.MAP_FIXED)
 	if err != nil {
 		return fmt.Errorf("second physical mmap: %w", err)
 	}
 
 	*r = Ring{size: size, buffer: (*byte)(orig)}
 	return nil
 }
-
-// Close frees up the mmaped memory and must be manually called before the Ring is garbage collected.
-// Very important, any buffer returned by .Context or .Unused, or leaked from .Write or .Read will become invalid and point to who knows what and cannot be used.
-// If you *retain* theses buffers, you might need to set them to nil before calling Close.
-func (r *Ring) Close() error {
-	buf := r.buffer
-	if buf == nil {
-		return nil
-	}
-	r.buffer = nil
-	return unix.MunmapPtr(unsafe.Pointer(buf), r.size*2)
-}

Other applications I would have used it were it available is mmaping a file for processing and having to manually manage it's lifetime with manual memory manage (mmap & munmap).

Implementation details

Destroying a mapping means in this question having the OS release this mapping resources.
It could be mapping over anonymous pages or calling munmap.

@gopherbot gopherbot added this to the Proposal milestone Dec 4, 2024
@gabyhelp
Copy link

gabyhelp commented Dec 4, 2024

Related Issues

Related Discussions

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@Jorropo
Copy link
Member Author

Jorropo commented Dec 4, 2024

The solution proposed is different than #70224 but 70224's is more generic and would work for me. In it I find #67535 which didn't crossed my mind either and would work too as something reliable.

@Jorropo Jorropo closed this as not planned Won't fix, can't repro, duplicate, stale Dec 4, 2024
@Jorropo
Copy link
Member Author

Jorropo commented Dec 9, 2024

#67535 doesn't work well because it panics if attached to an mmaped region fatal error: runtime.AddCleanup: ptr not in allocated block. It's possible to attach the cleanup to some related header but that not memory safe as someone could keep a reference to the mmaped memory region but not the header.

#70224 would solve my issue

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

No branches or pull requests

3 participants