Page MenuHomeFreeBSD

vmspace: Convert to refcount(9)
ClosedPublic

Authored by markj on Nov 2 2020, 5:58 PM.

Details

Summary

This is mostly mechanical except for vmspace_exit(). There, I used the
new refcount_release_if_last() to avoid switching to vmspace0 unless
other processes are sharing the vmspace. In that case, upon switching
to vmspace0 we can unconditionally release the reference.

Remove the volatile qualifier from vm_refcnt now that accesses are
protected using refcount(9) KPIs.

Diff Detail

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

Event Timeline

markj requested review of this revision.Nov 2 2020, 5:58 PM
sys/vm/vm_glue.c
553 ↗(On Diff #79077)

Should this be refcount_load > 1 ?

sys/vm/vm_map.c
350 ↗(On Diff #79077)

s/dummy/kernel/

markj marked 2 inline comments as done.

Address feedback.

This revision is now accepted and ready to land.Nov 2 2020, 11:51 PM
alc added inline comments.
sys/vm/vm_map.c
365–366 ↗(On Diff #79108)

There is nothing wrong with the patch here. I just want to make an observation.

Setting aside the exact KASSERT()s in some of the pmap implementations, we really don't need the pmap to be active on the underlying processor. We really only need that the pmap not be active on any other processor, because some of the pmap implementations intentionally perform non-atomic read-and-clear operations to minimize the cost of checking the dirty bit and clearing the PTE.

I wonder how often we pointlessly activate the pmap here.

400 ↗(On Diff #79108)

I would suggest merging this "if" statement with the one above it.

Tested on ARM and ARM64

markj marked an inline comment as done.Nov 4 2020, 4:26 PM
markj added inline comments.
sys/vm/vm_map.c
365–366 ↗(On Diff #79108)

By "pointlessly activate the pmap" do you mean switching to vmspace0's pmap and back again? I believe it should be fairly rare since it requires a process to exit while using a shared vmspace.

I ran a kernel build with the following dtrace script running in the background: dtrace -n 'fbt::vmspace_exit:entry {@[args[0]->td_proc->p_vmspace->vm_refcnt] = count();}' and got:

61                1
 1            53328

i.e., in all but one case the vmspace had one reference at the time the function was called. The sole exception was a case where p->p_vm == &vmspace0. I'm not sure how that arises.

This revision was automatically updated to reflect the committed changes.
sys/vm/vm_map.c
365–366 ↗(On Diff #79108)

I'm not sure how that arises.

It is from exiting NFS client async I/O threads.