Details

Reviewers

alc
markj

Commits

rG1aa249c93588: swapoff(8): add -f flag
rG79d650f26277: swapoff(2): document extended syscall arguments
rGe8dc2ba29c62: swapoff(2): add a SWAPOFF_FORCE flag
rGa4e4132fa3bf: swapoff(2): replace special device name argument with a structure
rG6df359449f39: swap_pager.c: Remove MPSAFE and ARGSUSED annotations

Summary

swapoff(2): replace special device name argument with a structure

For compatibility, add a placeholder pointer to the start of the
added struct swapoff_new_args, and use it to distinguish old vs. new
style of syscall invocation.

swapoff(2): add a SW_OFF_FORCE flag

The flag requests skipping the heuristic which tries to avoid leaving system
with more allocated memory than available from RAM and remanining swap.

swapoff: add -f flag

to force swapout by ignoring the heuristic that calculates amount of
allocated memory against total of RAM plus remaining swap.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

kib created this revision.Nov 29 2021, 4:44 PM

Herald added a subscriber: imp. · View Herald TranscriptNov 29 2021, 4:44 PM

kib requested review of this revision.Nov 29 2021, 4:44 PM

markj added inline comments.Nov 29 2021, 5:01 PM

sbin/swapon/swapon.8
133	The heuristic is inexact, but without it swapoff can easily hang the system, I believe. This should be noted.
sbin/swapon/swapon.c
802	Doesn't it need to be `swapoff(&sa)`?
sys/vm/swap_pager.c
2543	Perhaps pass the flag through directly.
sys/vm/swap_pager.h
80	SWAPOFF_FORCE is only one character longer, I would just call it that.

Handle notes.
Check for known flags.

This seems ok to me, thank you. To be honest I still do not really understand why this approach is preferable to adding a new system call.

sbin/swapon/swapon.8
128–129	I think "very inexact" will just use confuse users. Rather, it is very conservative.
134	Maybe, "if there is unsufficient swap space remaining."
sys/vm/swap_pager.c
2509	IMO it is worth explaining why we do this.

This revision is now accepted and ready to land.Dec 1 2021, 5:10 PM

In D33165#750838, @markj wrote:

This seems ok to me, thank you. To be honest I still do not really understand why this approach is preferable to adding a new system call.

Essentially, we add a whole new syscall just for 'force' flag. IMO it is too much. Our syscalls interface is not logical of course, but such interface expansion is outlawed IMO.

Handle Mark' notes.
Update swapoff(2) man page.

This revision now requires review to proceed.Dec 2 2021, 3:21 AM

Fix warning on cast.

alc added inline comments.Dec 3 2021, 8:02 AM

lib/libc/sys/swapon.2
71–72 ↗	(On Diff #99339)
85 ↗	(On Diff #99339)
86 ↗	(On Diff #99339)
sbin/swapon/swapon.8
134

Fix grammar in comments.
Remove MPSAFE and ARGUSED annotations.

markj accepted this revision.Dec 3 2021, 3:05 PM

This revision is now accepted and ready to land.Dec 3 2021, 3:05 PM

I think that's all I have to say.

lib/libc/sys/swapon.2
80 ↗	(On Diff #99384)
85 ↗	(On Diff #99384)
86 ↗	(On Diff #99384)
97 ↗	(On Diff #99384)
98 ↗	(On Diff #99384)
99 ↗	(On Diff #99384)
sbin/swapon/swapon.8
128–129
129
133