On Tue, Dec 25, 2018 at 11:24:54PM +0100, Mark Kettenis wrote:
> > Date: Tue, 25 Dec 2018 09:31:25 -0500
> > From: Scott Cheloha <scottchel...@gmail.com>
> >
> > On Tue, Dec 25, 2018 at 02:19:46PM +0100, Mark Kettenis wrote:
> > > > Date: Mon, 24 Dec 2018 16:28:20 -0500
> > > > From: Scott Cheloha <scottchel...@gmail.com>
> > > >
> > > > ok?
> > >
> > > How can this happen? Only if the user asks to sleep for exactly 0ns.
> > > That is a bit nonsensical. Currently we punish such a process by
> > > making it sleep. Which means that other processes can run. Why do
> > > you want to change this?
> >
> > Punishment is relative. I'm coming at this from a bughunting angle.
> >
> > If I get my math wrong and my timeout is truncated to zero I don't
> > want the code to appear as if it is behaving correctly by sleeping
> > for a tick or two. This could go unnoticed. However, if the same
> > code polls *hard* in a loop it's very obvious that something is
> > wrong.
> >
> > You could argue that it's not our problem... but to my mind doing what
> > the caller asked, i.e. "don't sleep," helps the programmer more than
> > it might help other processes by yielding.
>
> Time to beat you over the head with POSIX ;)
Oh joy!
All I *really* wanted for Christmas was an interpretation of a standards
document :)
> The nanosleep() function shall cause the current thread to be
> suspended from execution until...
>
> Which can be interpreted such that nanosleep() should always suspend
> the thread (unless an error is returned) even if te interval is 0ns.
Hmm... I guess that depends on whether the "current thread of execution"
includes stuff running in the kernel. As far as the application is
concerned, all bets are off when we leave the application code, no? POSIX
says in other places that execution is "suspended" during a system call,
but in many of those cases I'm sure the typical implementation is still
executing code.
This interpretation of "suspends" would also exclude (perhaps naive) wakeup
latency optimizations like busy-waiting with delay(9) (or an equivalent)
in order to time out as close to the caller's request as possible. I'm
pretty sure this is not unheard of, though I'd have to dig a little deeper
to find examples on other systems.
> I think you need a more convincing argument to change this (actual
> code that misbehaves with the current implementation) especially since
> your diff adds complexity.
That's fair, I will keep an ear to the ground for spinning applications.
I might have a winner already, though. Earlier today I saw iridium busy
sleeping: repeat nanosleeps with zero'd timespec structs. I have a ktrace
here, but I have yet to reproduce. Need to fuss with it a bit more.
fwiw, here's an exerpt:
90240/241378 iridium 1545768212.972668 CALL nanosleep(0x84928d42a78,0)
90240/123819 iridium 1545768212.972670 CALL nanosleep(0x84928d42a78,0)
90240/241378 iridium 1545768212.972703 STRU struct timespec { 0 }
90240/123819 iridium 1545768212.972706 STRU struct timespec { 0 }
90240/169304 iridium 1545768212.983015 CALL nanosleep(0x84928d42a78,0)
90240/169304 iridium 1545768212.983095 STRU struct timespec { 0 }
90240/525129 iridium 1545768212.983100 CALL nanosleep(0x84928d42a78,0)
90240/525129 iridium 1545768212.984049 STRU struct timespec { 0 }
90240/241378 iridium 1545768212.993270 CALL nanosleep(0x84928d42a78,0)
90240/241378 iridium 1545768212.993351 STRU struct timespec { 0 }
90240/123819 iridium 1545768212.997352 CALL nanosleep(0x84928d42a78,0)
90240/123819 iridium 1545768212.997431 STRU struct timespec { 0 }
90240/558012 iridium 1545768212.998511 CALL nanosleep(0x84928d42a78,0)
90240/558012 iridium 1545768212.998516 STRU struct timespec { 0 }
... in my 13 second trace there are ~3000 zero'd nanosleep calls across
several threads. It went like that, eating up 1.5 CPUs worth of time until
I killed it... ugh, chrome...
> > > > Index: sys/kern/kern_time.c
> > > > ===================================================================
> > > > RCS file: /cvs/src/sys/kern/kern_time.c,v
> > > > retrieving revision 1.103
> > > > diff -u -p -r1.103 kern_time.c
> > > > --- sys/kern/kern_time.c 28 May 2018 18:05:42 -0000 1.103
> > > > +++ sys/kern/kern_time.c 24 Dec 2018 21:21:40 -0000
> > > > @@ -288,13 +288,18 @@ sys_nanosleep(struct proc *p, void *v, r
> > > > if (rmtp)
> > > > getnanouptime(&sts);
> > > >
> > > > + if (!timespecisset(&rqt)) {
> > > > + error = 0;
> > > > + goto out;
> > > > + }
> > > > +
> > > > error = tsleep(&nanowait, PWAIT | PCATCH, "nanosleep",
> > > > MAX(1, tstohz(&rqt)));
> > > > if (error == ERESTART)
> > > > error = EINTR;
> > > > if (error == EWOULDBLOCK)
> > > > error = 0;
> > > > -
> > > > +out:
> > > > if (rmtp) {
> > > > getnanouptime(&ets);
> > > >
> > > >
> > > >
> >
