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.
Tags
None
Referenced Files
F103018712: D25282.id73142.diff
Tue, Nov 19, 9:41 PM
Unknown Object (File)
Mon, Nov 11, 5:01 PM
Unknown Object (File)
Thu, Oct 31, 11:00 PM
Unknown Object (File)
Thu, Oct 31, 5:43 AM
Unknown Object (File)
Oct 11 2024, 1:38 AM
Unknown Object (File)
Oct 10 2024, 3:59 AM
Unknown Object (File)
Oct 3 2024, 10:37 PM
Unknown Object (File)
Sep 23 2024, 9:35 AM
Subscribers

Details

Summary

No functional change.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 31719
Build 29288: arc lint + arc unit

Event Timeline

cem requested review of this revision.Jun 15 2020, 5:28 PM
cem created this revision.
sys/vm/vm_map.c
2462

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

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

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

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
2397

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

2442

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.