Page MenuHomeFreeBSD

Ignore object->handle for OBJ_ANON objects.
ClosedPublic

Authored by kib on Nov 20 2019, 7:55 PM.

Details

Summary

Note that the change in vm_object_collapse() is arguably a correctness fix. We must not collapse into content-identity carrying objects.

[This is a preparational commit, the plan is to use handle for OBJ_ANON to point to the end of the shadow chain].

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

kib created this revision.Nov 20 2019, 7:55 PM
jeff added inline comments.Nov 20 2019, 8:26 PM
sys/vm/vm_object.c
571 ↗(On Diff #64640)

This could be an assert that it is an ANON object. Non-anon should never have a shadow_count now.

603 ↗(On Diff #64640)

I'm a little confused at some of the dead checks in the shadow chains. I don't know where they are possible and where they are not. I suspect some code may be unnecessarily checking and handling them. a vnode may become dead but another anon with a ref should not right?

1752 ↗(On Diff #64640)

Here again, can this actually become dead now that we limit to anon objects?

kib marked an inline comment as done.Nov 20 2019, 8:56 PM
kib added inline comments.
sys/vm/vm_object.c
603 ↗(On Diff #64640)

Object collapse creates dead anon objects with refcount > 0.

jeff added inline comments.Nov 20 2019, 8:59 PM
sys/vm/vm_object.c
603 ↗(On Diff #64640)

But it should only do so when ref_count == 1. Which means if we hold the object with the final ref it can't disappear out from under us?

kib updated this revision to Diff 64644.Nov 20 2019, 9:01 PM

shadow_count > 0 implies OBJ_ANON.

kib added inline comments.Nov 20 2019, 9:08 PM
sys/vm/vm_object.c
603 ↗(On Diff #64640)

Definitely it cannot disappear, but I am not sure that collapse cannot run in parallel while we temporary unlock the object, the case when this code is run is not when the last ref was deallocated, it is pre-last.

jeff accepted this revision.Nov 20 2019, 10:44 PM
jeff added inline comments.
sys/vm/vm_object.c
603 ↗(On Diff #64640)

I see now that we drop locks in collapse scan and that could possibly expose OBJ_DEAD in some cases.

This revision is now accepted and ready to land.Nov 20 2019, 10:44 PM
kib added a subscriber: pho.Nov 22 2019, 11:22 AM
pho added a comment.Nov 24 2019, 12:40 PM

I ran the full stress2 test set without finding any problems.

This revision was automatically updated to reflect the committed changes.