Page MenuHomeFreeBSD

vm_pageout_scans: correct detection of active object
ClosedPublic

Authored by kib on Jan 18 2022, 3:28 PM.

Details

Summary
For non-anonymous swap objects, there is always a reference from the
owner to the object to keep it from recycling.  Account for it when
deciding should be query pmap for hardware active references for the
page.

Diff Detail

Repository
rG 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 requested review of this revision.Jan 18 2022, 3:28 PM

Am I right that this is effectively an optimization? For pages belonging to a named swap object we will call pmap_ts_referenced() unnecessarily, but the returned value will be correct (i.e., 0 if the page is unmapped).

sys/vm/vm_pageout.c
729

This comment is out of date, the page lock is no longer used. If the page is deallocated, the references are generally lost. I can fix this in a follow-up.

744

Extra newline.

861

This should presumably check whether ref_count > 1 for non-anonymous object.

1569

Presumably this check should be updated too.

kib marked 3 inline comments as done.Jan 18 2022, 10:03 PM

Am I right that this is effectively an optimization? For pages belonging to a named swap object we will call pmap_ts_referenced() unnecessarily, but the returned value will be correct (i.e., 0 if the page is unmapped).

Now that you formulate it this way, I think yes. Then it is somewhat mistery what I saw on my machine, where active queue stayed infinitely long with large unmapped tmpfs objects. Anyway, this patch indeed saves at least one lock/unlock of pv lock per scanned page.

sys/vm/vm_pageout.c
729

I do not think that references of the page being deallocated serve any use.

Plug two more places where object reference is used.

In D33924#767364, @kib wrote:

Am I right that this is effectively an optimization? For pages belonging to a named swap object we will call pmap_ts_referenced() unnecessarily, but the returned value will be correct (i.e., 0 if the page is unmapped).

Now that you formulate it this way, I think yes. Then it is somewhat mistery what I saw on my machine, where active queue stayed infinitely long with large unmapped tmpfs objects.

Hmm. I haven't seen such a problem before. Was the system otherwise idle? Which kernel revision was it running?

Anyway, this patch indeed saves at least one lock/unlock of pv lock per scanned page.

Yes, I think it's a worthy change.

sys/vm/vm_pageout.c
1569

I think the code would be more clear if we instead had bool vm_pageout_object_mapped(vm_object) which returned true if there is a mapping reference, and false otherwise.

1607

This should be updated as well.

kib marked 2 inline comments as done.

Change vm_pageout_object_act() to do the ref comparision.
Move the big comment again (I did not touched the page lock part).
Plug one more ref_count != 0 check.

sys/vm/vm_pageout.c
716
kib marked an inline comment as done.

Fix grammar in the comment.

This revision is now accepted and ready to land.Jan 20 2022, 7:26 PM