Page MenuHomeFreeBSD

vm_page_free_invalid(): Relax the xbusy assertion.
ClosedPublic

Authored by markj on Jul 26 2020, 5:54 PM.

Details

Summary

vm_page_assert_xbusied() asserts that the busying thread is the current
thread. For some uses of vm_page_free_invalid() (e.g., error handling
in vnode_pager_generic_getpages_done()), this condition does not hold.

Reported by: Jenkins (armv7 tests) via trasz

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

markj requested review of this revision.Jul 26 2020, 5:54 PM
markj created this revision.
This revision is now accepted and ready to land.Jul 26 2020, 6:23 PM

But does this mean that vm_page_free_prep() needs similar change ? Or at least, make assertion conditional on the page validity ?

In D25828#571965, @kib wrote:

But does this mean that vm_page_free_prep() needs similar change ? Or at least, make assertion conditional on the page validity ?

Hmm. vm_page_remove() unbusies the page before freeing it, but it has the same bug.

Use vm_page_xbusy_claim() in vm_page_free_invalid().

This revision now requires review to proceed.Jul 26 2020, 6:50 PM
sys/vm/vm_page.c
1377 ↗(On Diff #74978)

I think a comment would be useful saying that the function is used in contexts where the page is xbusy but not owned by us, because we are typically io completion thread.

markj marked an inline comment as done.

Add a comment.

This revision is now accepted and ready to land.Jul 26 2020, 8:00 PM

ahh, right. the test I added for this fails the I/O in such a way (using gnop) that the iodone is actually called in the start path, so it's the same thread that xbusied the pages in that case. you said this problem was reported by other automated testing, how does that test fail I/O such that the iodone is called in a separate thread?

In D25828#572106, @chs wrote:

ahh, right. the test I added for this fails the I/O in such a way (using gnop) that the iodone is actually called in the start path, so it's the same thread that xbusied the pages in that case. you said this problem was reported by other automated testing, how does that test fail I/O such that the iodone is called in a separate thread?

It was triggered by your test: https://ci.freebsd.org/job/FreeBSD-head-armv7-test/lastBuild/console

I think the difference here is that ARMv7 does not have GEOM direct dispatch enabled, so the completion was handled by the GEOM "up" thread rather than the thread initiating I/O.

This revision was automatically updated to reflect the committed changes.