Page MenuHomeFreeBSD

Introduce kern_mmap_req().
ClosedPublic

Authored by brooks on Jan 14 2020, 5:19 AM.

Details

Summary

This presents an extensible interface to the generic mmap(2)
implementation via a struct pointer intended to use a designated
initializer or compount literal. We take advantage of the mandatory
zeroing of fields not listed in the initializer.

Remove kern_mmap_fpcheck() and use kern_mmap_req().

The motivation for this change is a desire to keep the core
implementation from growing an ever-increasing number of arguments that
must be specified in the correct order for the lowest-level
implementations. In CheriBSD we have already added two more arguments.

Obtained from: CheriBSD

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

brooks created this revision.Jan 14 2020, 5:19 AM
brooks edited the summary of this revision. (Show Details)Jan 14 2020, 5:21 AM

Compiles, but needs testing.

brooks added inline comments.Jan 14 2020, 5:29 AM
sys/compat/linux/linux_mmap.c
197 ↗(On Diff #66717)

We could alternatively do the declaration above and assign with a compound literal here. Style(9) doesn't really have an opinion.

I've done limited testing (booted an amd64) system with this patch. I chose to remove kern_mmap_fpcheck entirely, but could restore it if that feels like a cleaner option.

I've done limited testing (booted an amd64) system with this patch. I chose to remove kern_mmap_fpcheck entirely, but could restore it if that feels like a cleaner option.

+1 for removing it completely; it's new, so there's not much sense in keeping it around when we have a better alternative. We'll call it a short-lived mistake. =-)

brooks edited the summary of this revision. (Show Details)Feb 20 2020, 6:38 AM
brooks added a reviewer: kib.
kib added a comment.Feb 21 2020, 4:26 PM

Overall this looks reasonable.

sys/compat/linux/linux_mmap.c
208 ↗(On Diff #66717)

I do not see why mr_fixed needs to be defined there and not at the start of the function.
If you keep the declaration there, then the blank line is needed between it and later code.

sys/sys/mman.h
259 ↗(On Diff #66717)

Can the structure definition moved somewhere else ? mman.h is user-visible standard-defined header. Even hidden under #ifdef _KERNEL bits are not too good to add.

I was equally unhappy about struct shmfd appearing there.

brooks updated this revision to Diff 69140.Mar 3 2020, 8:42 PM
  • Rebase
  • Use a compound literal to move decleration to the top of the function.
  • Move struct mmap_req to syscallsubr.h.
brooks marked an inline comment as done.Mar 3 2020, 8:42 PM
kib added inline comments.Mar 4 2020, 4:49 PM
sys/compat/linux/linux_mmap.c
208 ↗(On Diff #66717)

This comment is still relevant.

212 ↗(On Diff #69140)

end line with comma, as we usually do ?

brooks updated this revision to Diff 69168.Mar 4 2020, 6:10 PM
  • Declare mr_fixed a the top of the function.
  • Add comma
brooks marked 2 inline comments as done.Mar 4 2020, 6:11 PM
kib accepted this revision.Mar 4 2020, 7:54 PM
This revision is now accepted and ready to land.Mar 4 2020, 7:54 PM
This revision was automatically updated to reflect the committed changes.