Page MenuHomeFreeBSD

Implement CloudABI memory management system calls.
ClosedPublic

Authored by ed on Jul 9 2015, 9:07 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sep 29 2024, 7:49 AM
Unknown Object (File)
Sep 29 2024, 7:49 AM
Unknown Object (File)
Sep 29 2024, 7:49 AM
Unknown Object (File)
Sep 29 2024, 6:41 AM
Unknown Object (File)
Sep 25 2024, 2:16 AM
Unknown Object (File)
Sep 24 2024, 4:13 AM
Unknown Object (File)
Sep 17 2024, 3:04 PM
Unknown Object (File)
Sep 14 2024, 7:13 PM
Subscribers

Details

Summary

Add support for the <sys/mman.h> functions by wrapping around our own implementations. There are no kern_*() variants of these system calls, but we also don't need them in this case. It is sufficient to just call into the sys_*() functions.

Test Plan

These system calls already pass the CloudABI test suite.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ed retitled this revision from to Implement CloudABI memory management system calls..
ed updated this object.
ed edited the test plan for this revision. (Show Details)
ed added reviewers: brooks, emaste.
ed set the repository for this revision to rS FreeBSD src repository - subversion.
brooks edited edge metadata.

Looks good to me.

sys/compat/cloudabi/cloudabi_mem.c
108 ↗(On Diff #6809)

Would it make sense to kassert fd == -1 on this case and just an unconditional assignment?

This revision is now accepted and ready to land.Jul 10 2015, 6:34 PM
sys/compat/cloudabi/cloudabi_mem.c
108 ↗(On Diff #6809)

These arguments come directly from userspace, so we shouldn't attach KASSERTs to them, right? That way the user can trigger a panic.

This is actually where Linux and BSD differ: on Linux, if MAP_ANON is set, the fd argument is ignored. BSD requires the fd to be -1 in that case. For CloudABI I thought it would make sense to relax this requirement.

sys/compat/cloudabi/cloudabi_mem.c
108 ↗(On Diff #6809)

Right, I realized kassert was wrong later that night.

I don't like the inherent sloppyness of allowing any random integer to be passed for "don't-use-this". It seems like people will end up failing late due to calling mem_map(fd_i_want_to_use, ...) with MAP_ANON due to copy and paste errors. Better IMO to fail entirely.

sys/compat/cloudabi/cloudabi_mem.c
108 ↗(On Diff #6809)

Yes. Now that I think of it, that sounds like a good idea.

So one thing I didn't mention: cloudabi_fd_t is actually an unsigned type in CloudABI, for the reason that the system call interface almost never overloads values with error codes. The userspace system call wrappers always return the file descriptor by reference and return a cloudabi_errno_t. There is no need to express negative file descriptor numbers:

https://github.com/NuxiNL/cloudlibc/blob/master/src/common/syscalldefs_mi.h#L330

This means that we'd better not test against -1 in this piece of code, as we should just treat the file descriptor as some kind of opaque value. I've just extended cloudlibc's mmap() function to enforce that fd == -1 in this case:

https://github.com/NuxiNL/cloudlibc/blob/master/src/libc/sys/mman/mmap.c#L23

In other words, we still enforce it, but it doesn't need to be done in the kernel. Does this look all right to you?

sys/compat/cloudabi/cloudabi_mem.c
108 ↗(On Diff #6809)

That suggests there should be a (subject to spelling)

#define CLOUDABI_MEM_MAP_ANON_FD (uint32_t)-1

Otherwise people are going to have to cast when mapping anon memory (I think).

sys/compat/cloudabi/cloudabi_mem.c
108 ↗(On Diff #6809)

Done!

This revision was automatically updated to reflect the committed changes.