On 06/01/20(Mon) 17:21, Scott Cheloha wrote:
> [...]
> But the risk is there. So I don't want to change the rounding yet.
Which risk? You still haven't explain. Diff is below, where is the
risk?
> So I don't want to change the rounding yet.
That we understood :o) It's a matter of taste, liking, bikescheding...
> I only want to focus on getting finer-grained timeout information to
> the timeout layer. The more detail we can get to the timeout layer
> the better it can make decisions. So in this sweep of conversions I'm
> only focused on getting the math right.
You're mixing unrelated stuff. The conversion is about moving the
hz/tick conversion inside a function such that we don't have to
fix all the callers.
> > Here it is 1 tick, generally ~10ms. So either the code work with a
> > sleep of 10ms less or more. Generally it doesn't matter. Now for
> > userland facing programs you shouldn't wakeup 10ms earlier.
> >
> > I don't understand why the rounding precision is different between the
> > two interfaces. We have an interface that adds one tick and one that
> > doesn't.
>
> It's different because no other callers using tsleep(9) cared about
> the loss of up to 10ms.
My recent diff for uthum(4) shows the contrary:
https://marc.info/?l=openbsd-tech&m=157815193019519&w=2
How do we know no caller care? How do we know there isn't a bug in
current code? Are we willing to take such risk? At which price?
> So tstohz(9)/tvtohz(9) were added as bandaids.
> They actually work pretty well, all things considered.
I already said this is damn too complex and we don't need another API...
Then we'll need to convert those calls at least once more. You even
introduced a bug in that conversion... What else can I do?
If we convert everything to nanoseconds this all goes away. But is our
goal to have nice, simple and working code?
On top of that this bandaid is stupid because it doesn't abstract `hz'.
That creates dependency in the development steps. Do you see it?
> > Both choices are imprecise and should disappear if/when the
> > guts of the sleep are modified to be tickless (whatever that means).
> > In the meantime I'd suggest we keep the same behavior between the two
> > interfaces so we can move forward with the other part of the problem:
> > the conversion.
>
> I agree. I don't think we should change the rounding behavior (yet)
> while we still have many trickier conversions ahead of us.
That's the other way around, we already changed the rounding behavior,
diff below restores it.
> > It is like doing a refactoring with introducing a behavior change that
> > prevent us from finishing the refactoring because the change depends on
> > the internals... Am I going in circle?
>
> You're right, there is a chicken-and-egg problem in this work. Some
> timeouts work currently on the tick-based backend but may break when
> we change to a tickless backend. But they may break if we preemptively
> convert them to use a real unit of time before we change the backend.
Are you saying that two breakages are better than one? This tsleep(9) ->
tsleep_nsec(9) conversion should be breakage free. This should just be a
refactoring. Refactoring should not introduce changes in behavior.
Once that's finish let's change sleep internals to whatever mechanism of
the decade and let's take care of breakages.
> The sys/kern syscall timeouts are one such group. others. The
> trickiest ones in sys/kern are the per-process itimers.
There's no difference. Timeouts are timeouts. The kernel is a single
huge program and everything is related.
> > PS: What about architectures that won't go tickless? How are we going
> > to deal with the conversions if there's more than one API?
>
> If we can get the tickless timeout(9) backend working on the major
> architectures then it will be in use on all architectures. There
> won't be two APIs.
This sentence starts with 'If'. Why not start with a single API then
ask ourself "If we can get ...". Do you see the sense of this?
Index: kern/kern_synch.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_synch.c,v
retrieving revision 1.155
diff -u -p -r1.155 kern_synch.c
--- kern/kern_synch.c 30 Nov 2019 11:19:17 -0000 1.155
+++ kern/kern_synch.c 7 Jan 2020 09:12:59 -0000
@@ -166,11 +166,9 @@ tsleep_nsec(const volatile void *ident,
__func__, wmesg);
}
#endif
- to_ticks = nsecs / (tick * 1000);
+ to_ticks = (nsecs + tick_nsec - 1) / tick_nsec + 1;
if (to_ticks > INT_MAX)
to_ticks = INT_MAX;
- if (to_ticks == 0)
- to_ticks = 1;
return tsleep(ident, priority, wmesg, (int)to_ticks);
}
@@ -271,11 +269,9 @@ msleep_nsec(const volatile void *ident,
__func__, wmesg);
}
#endif
- to_ticks = nsecs / (tick * 1000);
+ to_ticks = (nsecs + tick_nsec - 1) / tick_nsec + 1;
if (to_ticks > INT_MAX)
to_ticks = INT_MAX;
- if (to_ticks == 0)
- to_ticks = 1;
return msleep(ident, mtx, priority, wmesg, (int)to_ticks);
}
@@ -322,11 +318,9 @@ rwsleep_nsec(const volatile void *ident,
__func__, wmesg);
}
#endif
- to_ticks = nsecs / (tick * 1000);
+ to_ticks = (nsecs + tick_nsec - 1) / tick_nsec + 1;
if (to_ticks > INT_MAX)
to_ticks = INT_MAX;
- if (to_ticks == 0)
- to_ticks = 1;
return rwsleep(ident, rwl, priority, wmesg, (int)to_ticks);
}
