vm_swapout: Restore handling of RLIMIT_RSS

[markj]

Summary

In commit 13a1129d700c, I removed the mechanism by which the pagedaemon
signals the swapout thread when page reclamation is unable to keep up
with demand. This is because the swapout thread's main action in this
case is to swap out sleeping processes, but we removed this support.

However, it had the secondary effect of causing the swapout thread to
enforce RLIMIT_RSS when racct is not enabled. Without it, if
racct_enabled is false, nothing ever kicks the swapout thread.

Restore the old behaviour of trying to enforce RLIMIT_RSS when the page
daemon is unable to keep up with demand. I'm not at all convinced this
is a good way to implement the limit, but the change wasn't intentional,
so let's restore it for now.

Fixes: 13a1129d700c ("vm: Remove kernel stack swapping support, part 1")

[olce]

sys/vm/vm_swapout.c

310–312

While here, leftover of swapout?

336–352

Not sure why we require all threads of a proc to be in one of the enumerated states here. Is that because, before, we wanted to rule out TD_IS_SWAPPED(), or is there another reason? Skipping a process on this basis could be unfair to other processes.

[olce]

sys/vm/vm_swapout.c

144

Isn't that assignment superfluous?

[kib]

sys/vm/vm_swapout.c

336–352

I think that this currently avoids process if some thread is inhibited due to waiting on a lock.

There is some sense in avoiding deactivation for it. Since thread is waiting for a lock (we do not know which one) it is quite possible that the lock is owned by another thread in the same process, which could be slowed down if user pages are reclaimed. E.g. the thread might wait for the vm_map sx, while another thread is handling some stages of the page fault.

I am not saying that this is required for correctness, but assessment that this check is harmful requires some care.

[markj]

sys/vm/vm_swapout.c

336–352

In your example, we will not skip the process, since a thread waiting for an sx lock will be sleeping, and TD_IS_SLEEPING() will be true.

In general this whole loop makes little sense to me as a mechanism to implement memory limits. IMO, limit enforcement should be done within the process itself. I would guess that the swapout daemon was just a convenient place to implement a rudimentary RLIMIT_RSS and later the racct variant. Scanning the whole process list once a second (only done for racct) is particularly weird.

I would opt for redesigning these limits rather than trying to improve this implementation.

[olce]

sys/vm/vm_swapout.c

336–352

I would opt for redesigning these limits rather than trying to improve this implementation.

Makes sense.