Page MenuHomeFreeBSD

libc: fix memfd_create's HUGETLB handling
AcceptedPublic

Authored by kevans on Fri, Mar 27, 7:58 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 2, 8:36 AM
Unknown Object (File)
Wed, Apr 1, 10:33 PM
Unknown Object (File)
Wed, Apr 1, 10:01 AM
Unknown Object (File)
Wed, Apr 1, 7:36 AM
Unknown Object (File)
Tue, Mar 31, 4:49 PM
Unknown Object (File)
Tue, Mar 31, 9:37 AM
Unknown Object (File)
Sat, Mar 28, 1:19 PM
Subscribers

Details

Reviewers
markj
kib
Group Reviewers
manpages
Summary

The 'simplification' commit referenced below actually broke one aspect
of MFD_HUGETLB: the caller isn't supposed to be required to specify a
size. MFD_HUGETLB by itself without a shift mask just requests a large
page, so we revert that part of memfd_create() back.

While we're here, fix up the related parts of the manpages a little bit,
since MFD_HUGETLB is actually supported. The manpage claims that we
would return ENOSYS if forced mappings weren't supported, but this was
actually not true. However, that seems like a very important
distinction to make between ENOSYS and EOPNOTSUPP, so fix the
implementation to match the docs.

Fixes: 8b8cf4ece660f ("memfd_create: simplify HUGETLB support [...]")

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 71755
Build 68638: arc lint + arc unit

Event Timeline

Typo in the message: OPNOTSUPP

lib/libc/gen/memfd_create.c
107

This loop allowed the normal page size before the patch. Should we change pgidx = 0 to pgidx = 1?

lib/libsys/shm_open.2
352

Which one is first?

lib/libc/gen/memfd_create.c
107

Yeah, I think that would make sense

lib/libsys/shm_open.2
352

Is there a standard name for psidx=1? I guess maybe "the smallest supported large page size"

lib/libsys/shm_open.2
352

This sounds right. Might be the man page needs to explain what it means by 'large page'.

kevans marked 3 inline comments as done.
  • Don't allow non-largepage shift mask
  • Try to massage the manpage a bit more
lib/libsys/shm_open.2
352

Looking over the page again, I think we do a pretty good job of connecting most of the dots in the description for shm_create_largepage() and the psind argument. How about this version that refers back to that? I think it's also useful to note that SHM_LARGEPAGE_ALLOC_DEFAULT is used for the ftruncate(2) policy, to be explicit.

kib added inline comments.
lib/libsys/shm_open.2
494
This revision is now accepted and ready to land.Sat, Mar 28, 2:32 AM
markj added inline comments.
lib/libc/gen/memfd_create.c
99

It'd be cleaner to check this case before creating the object, though I suppose there's no leak since this is an anonymous object.

lib/libsys/shm_open.2
352

A bit tangential, but it'd be good to mention the hw.pagesizes sysctl somewhere. The output from sysctl(8) is formatted, so you get a list:

$ sysctl hw.pagesizes
hw.pagesizes: { 4096, 2097152, 1073741824 }