Page MenuHomeFreeBSD

Release laundered vnode pages to the head of the inactive queue
ClosedPublic

Authored by markj on Nov 20 2016, 5:28 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 5, 11:07 PM
Unknown Object (File)
Nov 16 2024, 8:28 PM
Unknown Object (File)
Oct 25 2024, 5:11 AM
Unknown Object (File)
Oct 21 2024, 7:21 AM
Unknown Object (File)
Oct 20 2024, 12:07 AM
Unknown Object (File)
Oct 19 2024, 2:29 AM
Unknown Object (File)
Oct 19 2024, 1:32 AM
Unknown Object (File)
Oct 16 2024, 5:24 AM
Subscribers
None

Details

Summary

The swap pager enqueues laundered pages near the head of the inactive queue to avoid another trip through LRU before reclamation. This change implements corresponding support for this behaviour to the vnode pager and makes use of it in UFS and ext2fs.

No changes to ZFS are required, due to the missing VM integration: zfs_putpages() copies the dirty pages for writing and returns VM_PAGER_OK in the non-error case, and vm_pageout_flush() takes care of moving the pages to the inactive queue. msdosfs requires a bit more work, so I've held off on that in this review.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6017
Build 6284: arc lint + arc unit

Event Timeline

markj retitled this revision from to Add noreuse to ufs and ext2..
markj edited the test plan for this revision. (Show Details)
markj retitled this revision from Add noreuse to ufs and ext2. to Release laundered vnode pages to the head of the inactive queue.Nov 20 2016, 5:49 PM
markj updated this object.
markj added reviewers: alc, kib.
sys/ufs/ffs/ffs_vnops.c
461

Why do you need fs-specific variants of the function ? IMO it is cleaner to put the helper into vfs_bio.c.

Note that b_dep could be non-empty only for FFS.

sys/ufs/ffs/ffs_vnops.c
461

No good reason, I'll move it.

  • Use a common function to release read bufs.
kib edited edge metadata.

This breaks LRU for buffers, but this is probably fine when pagedaemon wants to reclaim the pages anyway.

sys/ufs/ffs/ffs_vnops.c
808

If you add another boolean parameter to vfs_bio_rbrelse(), to indicate should brelse()/bqrelse() be called, you may avoid inlining this snippet there as well as in ext2 code.

This revision is now accepted and ready to land.Nov 21 2016, 11:47 AM
alc edited edge metadata.
In D8589#178507, @kib wrote:

This breaks LRU for buffers, but this is probably fine when pagedaemon wants to reclaim the pages anyway.

Yes, I think so. In this case, the page(s) are known to be in the laundry queue and not wired into a buffer. In other words, we're creating a buffer just to perform the page out, so flagging that buffer as "no reuse" seems ok. Am I missing something?

In D8589#178560, @alc wrote:
In D8589#178507, @kib wrote:

This breaks LRU for buffers, but this is probably fine when pagedaemon wants to reclaim the pages anyway.

Yes, I think so. In this case, the page(s) are known to be in the laundry queue and not wired into a buffer. In other words, we're creating a buffer just to perform the page out, so flagging that buffer as "no reuse" seems ok. Am I missing something?

This consideration was mostly what I mean. I am sure that laundered pages are returned to the inactive queue, which is confirmed by the description of the revision. Note that if buffer is waited for by other thread meantime, LRU is broken. I agree that this is acceptable.

markj edited edge metadata.
  • Move more duplicated code to vfs_bio.c.
This revision now requires review to proceed.Nov 22 2016, 11:58 PM
markj added inline comments.
sys/ufs/ffs/ffs_vnops.c
808

I made it a separate function since I couldn't come up with a name that made sense for both usages (name suggestions welcome). I also pushed IO_DIRECT handling there. IO_INVAL was left alone as it appears to be unused.

markj marked an inline comment as done.
markj edited edge metadata.
  • Fix a typo.
kib edited edge metadata.
kib added inline comments.
sys/kern/vfs_bio.c
4423

b_io_dismiss() or vfs_bio_io_finish() ?

This revision is now accepted and ready to land.Nov 23 2016, 9:16 AM
alc edited edge metadata.
sys/kern/vfs_bio.c
4423

b_io_dismiss() seems marginally better, but still doesn't feel quite right. I'll just go with that.

This revision was automatically updated to reflect the committed changes.