Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Re: Remove last callers of the lbolt sleep channel
To:
Tim Leslie <tleslie@protonmail.com>
Cc:
"tech@openbsd.org" <tech@openbsd.org>
Date:
Tue, 23 Sep 2025 21:22:10 +0300

Download raw body.

Thread
On Tue, Sep 23, 2025 at 03:30:19PM +0000, Tim Leslie wrote:
> > On Tue, Sep 23, 2025 at 03:04:58PM +0000, Tim Leslie wrote:
> > 
> > > This patch removes the last callers of the lbolt sleep channel, which were in the TTY subsystem.
> > > 
> > > When a process in a background group attempts TTY I/O, it is currently put to sleep on &lbolt. This relies on the 1Hz wakeup() from schedcpu().
> > > 
> > > The proposed change replaces ttysleep(tp, &lbolt, ...) with a call to ttysleep_nsec(tp, pr->ps_pgrp, ..., SEC_TO_NSEC(1)). This now sleeps on the process group's address and uses a one-second timeout to approximate current behavior. The change does not alter the existing TTY locking protocol. In the general case, the sleep is interrupted immediately by SIGCONT via PCATCH, eliminating the up-to-1-second latency for job control. The fallback for processes ignoring SIGTTOU/SIGTTIN should be preserved by the explicit timeout and prevent an indefinite hang.
> > > 
> > > —
> > > Tim Leslie
> > 
> > 
> > Why don't you use `nowake' sleep channel?
> 
> If I understand the "nowake" convention correctly, it's for sleeps that should only be broken by a timeout or a direct signal, and where no other explicit wakeup() call is expected.
> 

This is true. However, pgsignal() sends signal to the given processes
group, not to the sleep channel, so it has no corresponding wakeup() for
this ttysleep_nsec(tp, pr->ps_pgrp, ...); and the code does not expect
it.

So I prefer to use `nowake', because this 100% exclude the fallout from
current or hypothetical newly introduced wakeup(pr->ps_pgrp).

> I was attempting to follow the current behavior, which is that in this TTY case, we are implicitly relying on SIGCONT to break the sleep when a job is foregrounded. While not a wakeup() call, it's a critical event that terminates the sleep.
> 
> It seems like sleeping on the process group's address is the most direct way to model that relationship. The one-second timeout in ttysleep_nsec then acts as the safety net for the SIG_IGN case, which we'd need regardless of the channel.