Page MenuHomeFreeBSD

(vm object 1) Replace busy checks and sleeps with acquires where it is trivial to do so.
AcceptedPublic

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

Details

Reviewers
alc
kib
markj
dougm
Summary

This patch converts most busy tests to acquires. This furthers the goal of converting busy to an actual lock that functions without the object lock held. This should be functionally a nop for now.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 26307
Build 24786: arc lint + arc unit

Event Timeline

jeff created this revision.Fri, Sep 6, 12:43 PM
jeff edited the summary of this revision. (Show Details)Fri, Sep 6, 12:45 PM
jeff added reviewers: alc, kib, markj, dougm.
jeff retitled this revision from Replace busy checks and sleeps with acquires where it is trivial to do so. to (vm object 1) Replace busy checks and sleeps with acquires where it is trivial to do so..Tue, Sep 10, 7:59 PM
kib added inline comments.Sat, Sep 14, 5:30 PM
sys/kern/vfs_bio.c
3644

So effectively this drains the xbusy state on bdwrite(). Do we need this ?

sys/vm/vm_object.c
718

Why is this assert moved ?

893

I somewhat dislike the lost of unique wmsgs for busy acquire. Might be, add wmsg arg to vm_page_busy_acquire() and use default msg if the arg is NULL ?

1425

Assert that ?

jeff added a comment.Sat, Sep 14, 6:45 PM

One possible refinement that I should mention;

We have many places that want to do I/O and really just want to be certain that they are the first shared busy holder but don't really want an exclusive lock. Today I implement that with xbusy & downgrade. It could be a special kind of first-sbusy-only acquire. This is relevant in page clustering for write back, for example, where you don't want to do work that is already being done.

sys/kern/vfs_bio.c
3644

Yes without it we are relying on the object lock to protect valid/dirty. Before we would make sure that busy was not held by checking it, now we make sure by acquiring it. The result is the same until you have consumers that busy without the object lock.

At the end of the patchset this is no longer sufficient and the object lock is not even acquired here.

sys/vm/vm_object.c
718

While the page is wired it could be busied by something like the buffer cache. I don't believe there is code that would trip this race today because we won't tear down a vm object while pages are in the buffer cache but it seemed prudent at the time. Perhaps it is unnecessary to move it.

893

I can add the argument back if we really want.

1425

In patch 2 this is changed to not release and the comment goes away. This comment is mirroring another one from vm_fault.c which documents the same behavior.

kib accepted this revision.Sat, Sep 14, 7:39 PM
kib added inline comments.
sys/vm/vm_object.c
893

I think it would be useful. Often even knowing the wchan for busy wait provides clue.

This revision is now accepted and ready to land.Sat, Sep 14, 7:39 PM
alc added inline comments.Sun, Sep 15, 3:27 AM
sys/vm/phys_pager.c
214

This assignment is redundant. Passing TRUE to vm_page_zero_invalid() results in the page being marked valid.