Page MenuHomeFreeBSD

vm: Convert vm_map_clip_end from a macro to a regular inline function
ClosedPublic

Authored by cem on Jun 15 2020, 5:28 PM.

Details

Summary

No functional change.

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

cem requested review of this revision.Jun 15 2020, 5:28 PM
cem created this revision.
sys/vm/vm_map.c
2462 ↗(On Diff #73142)

Why not do the same for vm_map_clip_start()?

Do we even need a separate _vm_map_clip_end()?

sys/vm/vm_map.c
2462 ↗(On Diff #73142)

start: I just didn't have a reason to modify vm_map_clip_start. If it also sometimes allocates memory, it's worth doing something similar. The rationale for this change is exclusively D25283 ; this was just done separately to highlight the functional change in the child revision.

Do we even need the wrapper? I don't know. I assume the idea of the macro was that it was more equivalent to __always_inline rather than plain inline, and that the longer function might not always be inlined. We could use __always_inline over inline here. I don't know if compiler heuristics result in the wrapped function being inlined or not. Maybe _vm_map_clip_end used to be in a separate CU, so it couldn't be inlined from the header, while the macro could? I don't know the history of this code very well.

markj added inline comments.
sys/vm/vm_map.c
2462 ↗(On Diff #73142)

Please modify vm_map_clip_start() to avoid making them inconsistent. It can also allocate an entry. vm_map_clip_start() and _end() do pretty much the same thing, they just operate on different ends of a VA range.

I think the wrapper made more sense before recent refactorings. If you look at e.g., stable/11, _vm_map_clip_start()/end() were much longer. Doug has refactored them since then. It should be fine to just merge them.

sys/vm/vm_map.c
2462 ↗(On Diff #73142)

Sounds good to me.

cem marked 2 inline comments as done.

Unwrap both vm_map_clip_{start,end}

Correct parameter names

Upload the right diff...

sys/vm/vm_map.c
2390 ↗(On Diff #73185)

Drop the underscore, or use "%s" and func.

2441 ↗(On Diff #73185)

Drop the underscore, or just use %s and func.

cem marked 2 inline comments as done.

Fix func names in KASSERT.

This revision is now accepted and ready to land.Jun 16 2020, 9:38 PM
This revision was automatically updated to reflect the committed changes.