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 18:20:55 +0300

Download raw body.

Thread
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?

> 
> ---
>  sys/kern/sched_bsd.c |  2 --
>  sys/kern/tty.c       | 14 ++++++++------
>  sys/kern/tty_pty.c   |  3 ++-
>  sys/sys/kernel.h     |  1 -
>  sys/sys/tty.h        |  1 +
>  5 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/sys/kern/sched_bsd.c b/sys/kern/sched_bsd.c
> --- a/sys/kern/sched_bsd.c
> +++ b/sys/kern/sched_bsd.c
> @@ -55,7 +55,6 @@
>  #endif
>  
>  uint64_t roundrobin_period;	/* [I] roundrobin period (ns) */
> -int	lbolt;			/* once a second sleep address */
>  
>  struct mutex sched_lock;
>  
> @@ -282,7 +281,6 @@ schedcpu(void *unused)
>  		}
>  		SCHED_UNLOCK();
>  	}
> -	wakeup(&lbolt);
>  	timeout_add_sec(&to, 1);
>  }
>  
> diff --git a/sys/kern/tty.c b/sys/kern/tty.c
> --- a/sys/kern/tty.c
> +++ b/sys/kern/tty.c
> @@ -747,8 +747,8 @@ ttioctl(struct tty *tp, u_long cmd, caddr_t data, int flag, struct proc *p)
>  			if (pr->ps_pgrp->pg_jobc == 0)
>  				return (EIO);
>  			pgsignal(pr->ps_pgrp, SIGTTOU, 1);
> -			error = ttysleep(tp, &lbolt, TTOPRI | PCATCH,
> -			    ttybg);
> +			error = ttysleep_nsec(tp, pr->ps_pgrp, TTOPRI | PCATCH,
> +				ttybg, SEC_TO_NSEC(1));
>  			if (error)
>  				return (error);
>  		}
> @@ -1515,7 +1515,8 @@ loop:	lflag = tp->t_lflag;
>  			goto out;
>  		}
>  		pgsignal(pr->ps_pgrp, SIGTTIN, 1);
> -		error = ttysleep(tp, &lbolt, TTIPRI | PCATCH, ttybg);
> +		error = ttysleep_nsec(tp, pr->ps_pgrp, TTIPRI | PCATCH, ttybg, 
> +			SEC_TO_NSEC(1));
>  		if (error)
>  			goto out;
>  		goto loop;
> @@ -1613,8 +1614,8 @@ read:
>  		    ISSET(lflag, IEXTEN | ISIG) == (IEXTEN | ISIG)) {
>  			pgsignal(tp->t_pgrp, SIGTSTP, 1);
>  			if (first) {
> -				error = ttysleep(tp, &lbolt, TTIPRI | PCATCH,
> -				    ttybg);
> +				error = ttysleep_nsec(tp, pr->ps_pgrp, TTIPRI | PCATCH, 
> +					ttybg, SEC_TO_NSEC(1));
>  				if (error)
>  					break;
>  				goto loop;
> @@ -1765,7 +1766,8 @@ loop:
>  			goto out;
>  		}
>  		pgsignal(pr->ps_pgrp, SIGTTOU, 1);
> -		error = ttysleep(tp, &lbolt, TTIPRI | PCATCH, ttybg);
> +		error = ttysleep_nsec(tp, pr->ps_pgrp, TTIPRI | PCATCH, ttybg, 
> +			SEC_TO_NSEC(1));
>  		if (error)
>  			goto out;
>  		goto loop;
> diff --git a/sys/kern/tty_pty.c b/sys/kern/tty_pty.c
> --- a/sys/kern/tty_pty.c
> +++ b/sys/kern/tty_pty.c
> @@ -294,7 +294,8 @@ again:
>  			    pr->ps_flags & PS_PPWAIT)
>  				return (EIO);
>  			pgsignal(pr->ps_pgrp, SIGTTIN, 1);
> -			error = ttysleep(tp, &lbolt, TTIPRI | PCATCH, ttybg);
> +			error = ttysleep_nsec(tp, pr->ps_pgrp, TTIPRI | PCATCH, 
> +				ttybg, SEC_TO_NSEC(1));
>  			if (error)
>  				return (error);
>  		}
> diff --git a/sys/sys/kernel.h b/sys/sys/kernel.h
> --- a/sys/sys/kernel.h
> +++ b/sys/sys/kernel.h
> @@ -55,7 +55,6 @@ extern int ticks;		/* # of hardclock ticks */
>  extern int hz;			/* system clock's frequency */
>  extern int stathz;		/* statistics clock's frequency */
>  extern int profhz;		/* profiling clock's frequency */
> -extern int lbolt;		/* once a second sleep address */
>  
>  #ifndef HZ
>  #define HZ 100
> diff --git a/sys/sys/tty.h b/sys/sys/tty.h
> --- a/sys/sys/tty.h
> +++ b/sys/sys/tty.h
> @@ -290,6 +290,7 @@ void	 ttypend(struct tty *tp);
>  int	 ttyretype(struct tty *tp);
>  int	 ttyrub(int c, struct tty *tp);
>  int	 ttysleep(struct tty *tp, void *chan, int pri, char *wmesg);
> +int	 ttysleep_nsec(struct tty *, void *, int, char *, uint64_t);
>  int	 ttywait(struct tty *tp);
>  int	 ttywflush(struct tty *tp);
>  void	 ttytstamp(struct tty *tp, int octs, int ncts, int odcd, int ndcd);
>