Re: kern/57259: ucom serial ports cannot be re-opened "too quickly" with O_NONBLOCK

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

The following reply was made to PR kern/57259; it has been noted by GNATS.

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: Jason Thorpe <thorpej%me.com@localhost>
Cc: gnats-bugs%NetBSD.org@localhost
Subject: Re: kern/57259: ucom serial ports cannot be re-opened "too quickly"
	with O_NONBLOCK
Date: Sun, 5 Mar 2023 12:56:44 +0000

 > Date: Sun, 5 Mar 2023 04:46:26 -0800
 > From: Jason Thorpe <thorpej%me.com@localhost>
 >=20
 > > On Mar 5, 2023, at 4:39 AM, Taylor R Campbell <riastradh%NetBSD.org@localhost> wr=
 ote:
 > >=20
 > >> - Even suppressing EWOULDBLOCK (resulting in an ERESTART), it would
 > >>  simply fail with a spurious EINTR that I couldn't determine where
 > >>  that was coming from (clearly from some higher-up layer).
 > >=20
 > > EINTR should only happen if a signal is delivered.  If you're not sure
 > > where that was coming from it bears further investigation.  I don't
 > > think tty(9) or condvar(9) will fabricate EINTR when there's no signal
 > > pending.
 >=20
 > Nope, you're wrong.
 
 That was a hortative `should', not an informative `should'.  It seems
 my suspicion was right that tty(9) and condvar(9) aren't fabricating
 this EINTR.  Strikes me as a bug that open(2) would fabricate EINTR
 when a signal has not been delivered.
 
 > I found where it's coming from:
 >=20
 >         error =3D vn_open(dvp, pb, TRYEMULROOT, flags, cmode,
 >             &vp, &dupfd_move, &dupfd);
 >         if (error !=3D 0) {
 >                 fd_abort(p, fp, indx);
 >                 if (error =3D=3D ERESTART)
 >                         error =3D EINTR;
 >                 return error;
 >         }
 >=20
 > That's in do_open().  So the logic to return ERESTART here is just
 > wrong, and it could have never worked as written.
 
 I tried chasing down where this ERESTART->EINTR map came from,
 and...it's been buried by the AT&T lawsuit.  It first appeared in
 vfs_syscalls.c 1.31, but all revisions up through 1.30 were expunged
 from history.
 
 Sigh.
 
 It's not obvious to me at all why open might not be restartable.  The
 man page only says:
 
      [EINTR]            The open() operation was interrupted by a signal.
 
 It's obviously risky business to consider changing something like this
 that has lurking been in the system for decades, but on the other
 hand, Chesterton's fences are bad and should be abolished.  So we
 should try to figure out why this is happening.