witness: sleepable rm locks are not sleepable in read mode

[rlibby]

Summary

There are two classes of rm lock, one "sleepable" and one not. But even
a "sleepable" rm lock is only sleepable in write mode, and is
non-sleepable when taken in read mode.

Warn about sleepable rm locks in read mode as non-sleepable locks. Do
this by defining a new lock operation flag, LOP_NOSLEEP, to indicate
that a lock is non-sleepable despite what the LO_SLEEPABLE flag would
indicate, and defining a new witness lock instance flag, LI_SLEEPABLE,
to track the product of LO_SLEEPABLE and LOP_NOSLEEP on the lock
instance.

Sponsored by: Dell EMC Isilon

Test Plan

kyua test -k /usr/tests/sys/Kyuafile

I also checked that the new witness_warn check successfully fires from
sysctl_old_user with sysctl_sysctl_name etc.

[markj]

As far as I know the only reason to disallow sleeping readers is that rmlock trackers typically live on the stack, and the stacks of sleeping threads may be swapped out. That seems like a fairly dubious reason in this day and age - do you know of any other reasons?

[rlibby]

In D22527#492846, @markj wrote:

As far as I know the only reason to disallow sleeping readers is that rmlock trackers typically live on the stack, and the stacks of sleeping threads may be swapped out. That seems like a fairly dubious reason in this day and age - do you know of any other reasons?

As in, is it accidental or fundamental that sleepable rm locks are not sleepable in read mode? I don't know.

I'm not sure if stack swapping is the only potential problem. I see the implementation referencing turnstiles... I think that implies a deeper interaction with non-sleepability. To be honest I don't know either turnstiles or the rm lock implementation that well.

[cem]

In D22527#492846, @markj wrote:

As far as I know the only reason to disallow sleeping readers is that rmlock trackers typically live on the stack, and the stacks of sleeping threads may be swapped out. That seems like a fairly dubious reason in this day and age - do you know of any other reasons?

Yeah, that looks about right to me. Also, looks like it's ISLN's fault: rS212112 , rS252209 , rS287833 .

You would need to disallow swapping out the kernel portion of thread stacks which are holding an rmlock read lock (at least, if the tracker looks like it's on the local thread's stack — which is fairly easy to check and we do for some other things already).

It doesn't look like you can solely clear TDF_CANSWAP, because that is set/cleared internally by ULE without checking existing state. It seems like threads with priority < PSOCK are never swapped out, so you could temporarily bump the rlock'd thread's priority and restore at unlock. But obviously that seems brittle / depends on special knowledge of ULE internals (amusingly, the same priority works for sched_4bsd too). Maybe a new flag in td_pflags and slight change to the scheduler? Is it acceptable that sleepable rdlock'd threads are unswappable while sleeping?

[markj]

In D22527#492862, @rlibby wrote:

In D22527#492846, @markj wrote:

As far as I know the only reason to disallow sleeping readers is that rmlock trackers typically live on the stack, and the stacks of sleeping threads may be swapped out. That seems like a fairly dubious reason in this day and age - do you know of any other reasons?

As in, is it accidental or fundamental that sleepable rm locks are not sleepable in read mode? I don't know.

I'm not sure if stack swapping is the only potential problem. I see the implementation referencing turnstiles... I think that implies a deeper interaction with non-sleepability. To be honest I don't know either turnstiles or the rm lock implementation that well.

Ah, blocked writers propagate their priority to readers even with a sleepable lock. Indeed, we don't allow priority propagation to sleeping threads.

This change looks fine to me, thanks!