Page MenuHomeFreeBSD

Low hanging lock reduction based on semi-const flags.
ClosedPublic

Authored by jeff on Wed, Nov 27, 3:42 AM.

Details

Summary

These are pretty minor changes. COLOR is never cleared after it is set. So we can check for it before acquiring and setting. I made a mistake in my ref patch that caused us to use a write ref for the final vnode ref which is not necessary and actually was a regression. The vm_map_pmap_enter() change realistically isn't likely to improve perf but it looks more tidy this way I think.

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

jeff created this revision.Wed, Nov 27, 3:42 AM
jeff edited the summary of this revision. (Show Details)Wed, Nov 27, 3:45 AM
jeff added reviewers: kib, markj, alc, dougm.
jeff set the repository for this revision to rS FreeBSD src repository.
markj added inline comments.Wed, Nov 27, 5:07 AM
sys/vm/vm_map.c
2374 ↗(On Diff #64929)

Don't we formally need to recheck the object type here?

sys/vm/vm_object.c
582 ↗(On Diff #64929)

The indentation is wrong.

jeff added inline comments.Wed, Nov 27, 5:18 AM
sys/vm/vm_map.c
2374 ↗(On Diff #64929)

It can only have become DEAD since we must have a valid reference.

markj added inline comments.Wed, Nov 27, 4:44 PM
sys/vm/vm_map.c
2542 ↗(On Diff #64929)

Isn't this a policy change? Now we are not accounting for swap space reserved for shared memory or tmpfs objects.

kib added inline comments.Wed, Nov 27, 6:21 PM
sys/vm/vm_map.c
2372 ↗(On Diff #64929)

I wonder if it makes sense to just return there, regardless of the locking changes. If the object was device/sg and now it is no longer, then it probably does not make sense do any page table prefilling at all.

2542 ↗(On Diff #64929)

I suspect that this would be more important e.g. for shared read-only mapping (weird case actually).

sys/vm/vm_object.c
531 ↗(On Diff #64929)

I think that this runlock can happen before umtx_shm_object_terminated().

jeff added inline comments.Wed, Nov 27, 8:29 PM
sys/vm/vm_map.c
2542 ↗(On Diff #64929)

I think mark is right. And I may have made the same error in vm_map_copy_entry_anon().

jeff updated this revision to Diff 64977.Wed, Nov 27, 8:52 PM

Address review feedback. Correct a mistaken OBJ_ANON test.

kib accepted this revision.Thu, Nov 28, 7:51 PM
This revision is now accepted and ready to land.Thu, Nov 28, 7:51 PM
markj accepted this revision.Thu, Nov 28, 8:24 PM
markj added inline comments.
sys/vm/vm_map.c
2374 ↗(On Diff #64929)

Perhaps add a comment in the block above, where we recheck object->type == OBJT_DEVICE || object->type == OBJT_SG.