Page MenuHomeFreeBSD

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

Authored by jeff on Fri, Sep 6, 12:46 PM.

Details

Reviewers
kib
markj
alc
dougm
Summary

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

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 26308
Build 24787: arc lint + arc unit

Event Timeline

jeff created this revision.Fri, Sep 6, 12:46 PM
jeff retitled this revision from Don't release xbusy in vm_page_remove(), defer to vm_page_free_prep(). Some callers like vm_page_remove/replace want busy state to persist. to Don't release xbusy in vm_page_remove(), defer to vm_page_free_prep()..Fri, Sep 6, 2:05 PM
jeff edited the summary of this revision. (Show Details)
jeff added reviewers: kib, markj, alc, dougm.
markj added inline comments.Sun, Sep 8, 8:05 PM
sys/vm/vm_page.c
3474

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

kib added inline comments.Sun, Sep 8, 8:11 PM
sys/vm/vm_page.c
1213

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

Why do you need this chunk ?

jeff added inline comments.Tue, Sep 10, 4:18 PM
sys/vm/vm_page.c
1213

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

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 added inline comments.Tue, Sep 10, 5:46 PM
sys/vm/vm_page.c
1213

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 added inline comments.Tue, Sep 10, 7:13 PM
sys/vm/vm_page.c
1213

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().

kib accepted this revision.Tue, Sep 10, 7:38 PM
This revision is now accepted and ready to land.Tue, Sep 10, 7:38 PM
jeff retitled this revision from Don't release xbusy in vm_page_remove(), defer to vm_page_free_prep(). to (vm object 2) Don't release xbusy in vm_page_remove(), defer to vm_page_free_prep()..Tue, Sep 10, 7:59 PM
markj accepted this revision.Tue, Sep 10, 8:58 PM
markj added inline comments.
sys/vm/vm_page.c
3474

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);
}