NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

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



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:39:40 +0000

 > Date: Sun, 5 Mar 2023 04:29:09 -0800
 > From: Jason Thorpe <thorpej%me.com@localhost>
 >=20
 > > On Mar 4, 2023, at 2:15 PM, Taylor R Campbell <riastradh%NetBSD.org@localhost> wr=
 ote:
 > >=20
 > > is now implemented by sleeping only for the _remainder_ of the 1sec
 > > delay in open(2):
 >=20
 > Yes, as it turns out, there were a couple of bugs in that block of
 > code.  Most notably:
 >=20
 > - In the case if a spurious wake, it would not wait for the full
 >   duration.
 
 That's intentional.  The job of .d_cancel is to make any concurrent
 open or I/O wake and fail immediately, so that:
 
 (a) close can finish promptly, and
 (b) open will redo any permission checks after revoke.
 
 Part (a) is arguable (is a delay of <1sec that bad? -- at least it
 won't deadlock), but part (b) is mandatory -- if open sleeps and the
 file is revoked because permissions have concurrently changed, open
 MUST restart or fail.
 
 > - 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).
 
 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.
 
 > I have a fix now that's been tested for a few dozen rapid
 > open-close-open cycles, and is not crashy in the face of pulling the
 > device (which I did a couple of times).
 
 Share the patch?
 


Home | Main Index | Thread Index | Old Index