(vm object 2) Don't release xbusy in vm_page_remove(), defer to vm_page_free_prep().

[jeff]

Summary

This persists the busy state across compound operations like rename and replace. Eliminating spurious wakeups and visibility of incomplete state changes.

[markj]

sys/vm/vm_page.c

3474 ↗

(On Diff #61726)

Can't we move this under the m->object != NULL check?

[kib]

sys/vm/vm_page.c

1213 ↗

(On Diff #61726)

This is the strange action on the fake page structure that is going away.

Why do you need this chunk ?

[jeff]

sys/vm/vm_page.c

1213 ↗

(On Diff #61726)

I could just set the busy state to zero. However, if you have some code which is waiting for an identity change to wake it up and it has sloppy atomicity it may never wakeup. I _think_ there would have to be another bug for that to happen but this seemed safer.

3474 ↗

(On Diff #61726)

You mean to say that it couldn't possibly be busy with a NULL object? I believe that is true but I'm not sure how confident I am in that. Since this is tested as is I would prefer to do a follow up patch with extra asserts and re-arranging if you think it is valuable.

[kib]

sys/vm/vm_page.c

1213 ↗

(On Diff #61726)

I do not think it is needed. Note that the memory of vm_page is released a line below. Anybody sleeping on busy for the fake page when it goes to dismount is already broken.

[jeff]

sys/vm/vm_page.c

1213 ↗

(On Diff #61726)

After the wakeup callers do not touch the page they were sleeping on. So it is valid to wait for the change in identity on something that is going away. I believe it is safer this way and carries forward existing behavior which would've issued the wakeup from remove().

[markj]

sys/vm/vm_page.c

3474 ↗

(On Diff #61726)

In fact, it is not true anymore with this change. Fragments like this, in vm_object_collapse_scan() for example, would be wrong:

if (vm_page_remove(m)) {
    vm_page_free(m);
} else {
    vm_page_xunbusy(m);
}