Do less sleepqueue processing post-switch so that we can avoid holding thethread lock.

[jeff]

Summary

Various restructuring has made it possible to do all post-wakeup processing in sleepq_resume_thread() where the thread lock is already held. This will permit a follow-up commit where sched_switch() does not return with the thread lock held. I also believe the resulting code looks cleaner.

This has passed stress2

[jhb]

A few small suggestions.

sys/kern/subr_sleepqueue.c

384 ↗

(On Diff #65448)

Shouldn't this always be clear at this point?

638 ↗

(On Diff #65448)

We didn't used to have the td_sleeptimo check and that is when some of this became redundant.

686 ↗

(On Diff #65448)

Now that sleepq_check_signals() doesn't do any real work, we can make this a bit simpler perhaps:

if (rcatch)
    return (rcatch);
return (td->td_intrval);

This would also mean just axing sleepq_check_signals() entirely.

A futher refinement would be to change sleepq_catch_signals to set td_intrval directly instead of 'ret =' for the non-zero return values and to change it to return void. You can then remove 'rcatch' and all the 'if (rcatch)' checks.

707 ↗

(On Diff #65448)

Similarly, axe rval and just use:

return (sleepq_check_timeout());

722 ↗

(On Diff #65448)

This one is the only one still somewhat complex:

thread_unlock(curthread);
rvalt = sleepq_check_timeout();

if (rcatch)
    return (rcatch);
if (td->td_intrval)
    return (td->td_intrval)
return (rvalt);

1056 ↗

(On Diff #65448)

This comment is kind of wrong. What this case is handling is when the callout_stop failed to cancel the callout and it is running spuriously after the thread has already resumed.

1058 ↗

(On Diff #65448)

This can probably just be TD_IS_SLEEPING? You can't be sleeping and not be on a sleepq.

1060 ↗

(On Diff #65448)

This comment is also stale now and probably can just be axed.

[jeff]

sys/kern/subr_sleepqueue.c

384 ↗

(On Diff #65448)

This was just defensive. I can make it an assert.

686 ↗

(On Diff #65448)

Yeah I can simplify the control flow here. I like keeping the function though as it's a good reminder of what it's for.

[kib]

sys/kern/subr_sleepqueue.c

868 ↗

(On Diff #65448)

Doesn't this recurse into the sleepq code and establish an order between sleepq locks ? At least if we raced.

[jeff]

sys/kern/subr_sleepqueue.c

868 ↗

(On Diff #65448)

This was a good question which I had not considered as I forgot about the sleepq relationship with callout. In any event, we do not provide a lock and we do not call with DRAIN so no sleepq locks, our own or otherwise, are inspected or acquired in callout_stop(). We simply mark the thread and the cpu executing the callout will resume once we unblock the lock and discover that the callout is no longer scheduled.

[kib]

The patch looks good, modulo John' notes.

sys/kern/subr_sleepqueue.c

868 ↗

(On Diff #65448)

I see, perpahs a comment should also note this.