Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
> Date: Mon, 6 Jan 2020 17:21:01 -0600
> From: Scott Cheloha
>
> Sorry for the delay. I needed to think on this.
>
> On Fri, Jan 03, 2020 at 11:15:20AM +0100, Martin Pieuchot wrote:
> > On 02/01/20(Thu) 14:29, Scott Cheloha wrote:
> > > > On Jan 2, 2020, at 9:02 AM, Martin Pieuchot wrote:
> > > >
> > > > On 01/01/20(Wed) 22:13, Scott Cheloha wrote:
> > > >>> On Dec 31, 2019, at 9:35 AM, Martin Pieuchot wrote:
> > > >>>
> > > >>> I'd like to stop converting the given timespec to ticks and instead
> > > >>> use
> > > >>> nanoseconds. This is part of the ongoing effort to reduce the use of
> > > >>> `hz' through the kernel.
> > > >>>
> > > >>> Since I don't know C I'd appreciate any pointer about the checks that
> > > >>> should be added to TIMESPEC_TO_NSEC().
> > > >>>
> > > >>> Then the conversions to {t,rw}sleep_nsec(9) become trivial, diff
> > > >>> below.
> > > >>
> > > >> We can't do this until timeouts have a tickless interface. Otherwise
> > > >> your timeouts will return early. That's why I was saving the sys/kern
> > > >> conversions until after resolving that issue.
> > > >
> > > > I don't understand, can you elaborate?
> > >
> > > Timeout are scheduled against the current value of "ticks". Any time that
> > > has elapsed since the current tick began is unaccounted for. You need to
> > > add a tick to your sleep to account for it. tstohz(9) does this. We
> > > don't
> > > do it automatically for the *sleep_nsec(9) interfaces because that would
> > > have complicated the conversions we're doing and probably broken callers
> > > before we were ready to break them.
> >
> > I question the argument that would complicate the conversions. Isn't it
> > just a margin of error that is given by the precision of the conversion?
>
> It depends on the sensitivity of the subsystem. Some timeouts are
> "fat" and nobody will notice if they are ~10ms "late". Others are
> razor thin and the system will be palpably slower for an interactive
> user. Most are somewhere in between.
Examples? I'd expect that almost all cases where you care about
sleeping as short as possible will use a wakeup.
> But the risk is there. So I don't want to change the rounding yet.
> 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.
>
> > 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. So tstohz(9)/tvtohz(9) were added as bandaids.
> They actually work pretty well, all things considered.
>
> > 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.
>
> In this context, tickless timeouts are timeouts that preserve the
> granularity of the timeout in a constant quantum, e.g. nanoseconds.
> Said another way, timeouts are tickless if they expire at an absolute
> time on one of the system clocks, e.g. "ten seconds and 400
> nanoseconds after boot" or "Jan 10 2020 16:30 UTC".
>
> > 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.
>
> The sys/kern syscall timeouts are one such group. others. The
> trickiest ones in sys/kern are the per-process itimers.
>
> There might be other groups.
>
> > 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.
>
> If you're asking about architectures that we fail to implement a
> dynamic hardclock(9) for, I don't know. I think they'll just continue
> to have a static hardclock(9).
>
>
Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
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=157815193019519=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 - 1.155 +++ kern/kern_synch.c 7 Jan 2020 09:12:59 - @@ -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 =
Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
Sorry for the delay. I needed to think on this.
On Fri, Jan 03, 2020 at 11:15:20AM +0100, Martin Pieuchot wrote:
> On 02/01/20(Thu) 14:29, Scott Cheloha wrote:
> > > On Jan 2, 2020, at 9:02 AM, Martin Pieuchot wrote:
> > >
> > > On 01/01/20(Wed) 22:13, Scott Cheloha wrote:
> > >>> On Dec 31, 2019, at 9:35 AM, Martin Pieuchot wrote:
> > >>>
> > >>> I'd like to stop converting the given timespec to ticks and instead use
> > >>> nanoseconds. This is part of the ongoing effort to reduce the use of
> > >>> `hz' through the kernel.
> > >>>
> > >>> Since I don't know C I'd appreciate any pointer about the checks that
> > >>> should be added to TIMESPEC_TO_NSEC().
> > >>>
> > >>> Then the conversions to {t,rw}sleep_nsec(9) become trivial, diff below.
> > >>
> > >> We can't do this until timeouts have a tickless interface. Otherwise
> > >> your timeouts will return early. That's why I was saving the sys/kern
> > >> conversions until after resolving that issue.
> > >
> > > I don't understand, can you elaborate?
> >
> > Timeout are scheduled against the current value of "ticks". Any time that
> > has elapsed since the current tick began is unaccounted for. You need to
> > add a tick to your sleep to account for it. tstohz(9) does this. We don't
> > do it automatically for the *sleep_nsec(9) interfaces because that would
> > have complicated the conversions we're doing and probably broken callers
> > before we were ready to break them.
>
> I question the argument that would complicate the conversions. Isn't it
> just a margin of error that is given by the precision of the conversion?
It depends on the sensitivity of the subsystem. Some timeouts are
"fat" and nobody will notice if they are ~10ms "late". Others are
razor thin and the system will be palpably slower for an interactive
user. Most are somewhere in between.
But the risk is there. So I don't want to change the rounding yet.
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.
> 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. So tstohz(9)/tvtohz(9) were added as bandaids.
They actually work pretty well, all things considered.
> 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.
In this context, tickless timeouts are timeouts that preserve the
granularity of the timeout in a constant quantum, e.g. nanoseconds.
Said another way, timeouts are tickless if they expire at an absolute
time on one of the system clocks, e.g. "ten seconds and 400
nanoseconds after boot" or "Jan 10 2020 16:30 UTC".
> 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.
The sys/kern syscall timeouts are one such group. others. The
trickiest ones in sys/kern are the per-process itimers.
There might be other groups.
> 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.
If you're asking about architectures that we fail to implement a
dynamic hardclock(9) for, I don't know. I think they'll just continue
to have a static hardclock(9).
Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
On 02/01/20(Thu) 14:29, Scott Cheloha wrote:
> > On Jan 2, 2020, at 9:02 AM, Martin Pieuchot wrote:
> >
> > On 01/01/20(Wed) 22:13, Scott Cheloha wrote:
> >>> On Dec 31, 2019, at 9:35 AM, Martin Pieuchot wrote:
> >>>
> >>> I'd like to stop converting the given timespec to ticks and instead use
> >>> nanoseconds. This is part of the ongoing effort to reduce the use of
> >>> `hz' through the kernel.
> >>>
> >>> Since I don't know C I'd appreciate any pointer about the checks that
> >>> should be added to TIMESPEC_TO_NSEC().
> >>>
> >>> Then the conversions to {t,rw}sleep_nsec(9) become trivial, diff below.
> >>
> >> We can't do this until timeouts have a tickless interface. Otherwise
> >> your timeouts will return early. That's why I was saving the sys/kern
> >> conversions until after resolving that issue.
> >
> > I don't understand, can you elaborate?
>
> Timeout are scheduled against the current value of "ticks". Any time that
> has elapsed since the current tick began is unaccounted for. You need to
> add a tick to your sleep to account for it. tstohz(9) does this. We don't
> do it automatically for the *sleep_nsec(9) interfaces because that would
> have complicated the conversions we're doing and probably broken callers
> before we were ready to break them.
I question the argument that would complicate the conversions. Isn't it
just a margin of error that is given by the precision of the conversion?
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. 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.
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?
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?
Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
> On Jan 2, 2020, at 9:02 AM, Martin Pieuchot wrote:
>
> On 01/01/20(Wed) 22:13, Scott Cheloha wrote:
>>> On Dec 31, 2019, at 9:35 AM, Martin Pieuchot wrote:
>>>
>>> I'd like to stop converting the given timespec to ticks and instead use
>>> nanoseconds. This is part of the ongoing effort to reduce the use of
>>> `hz' through the kernel.
>>>
>>> Since I don't know C I'd appreciate any pointer about the checks that
>>> should be added to TIMESPEC_TO_NSEC().
>>>
>>> Then the conversions to {t,rw}sleep_nsec(9) become trivial, diff below.
>>
>> We can't do this until timeouts have a tickless interface. Otherwise
>> your timeouts will return early. That's why I was saving the sys/kern
>> conversions until after resolving that issue.
>
> I don't understand, can you elaborate?
Timeout are scheduled against the current value of "ticks". Any time that
has elapsed since the current tick began is unaccounted for. You need to
add a tick to your sleep to account for it. tstohz(9) does this. We don't
do it automatically for the *sleep_nsec(9) interfaces because that would
have complicated the conversions we're doing and probably broken callers
before we were ready to break them.
When we have a tickless interface for the timeout(9) layer you will be
able to set a timeout to expire at a given absolute uptime with nanosecond
precision. Then you won't need to work around ticks. It'll be based on
the actual system uptime, not "ticks", which is too coarse an approximation.
With the aforementioned interface we'll be able to write a *sleep(9)
interface that takes advantage of it. And then we can convert the
callers in sys/kern.
Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
On 01/01/20(Wed) 22:13, Scott Cheloha wrote:
> > On Dec 31, 2019, at 9:35 AM, Martin Pieuchot wrote:
> >
> > I'd like to stop converting the given timespec to ticks and instead use
> > nanoseconds. This is part of the ongoing effort to reduce the use of
> > `hz' through the kernel.
> >
> > Since I don't know C I'd appreciate any pointer about the checks that
> > should be added to TIMESPEC_TO_NSEC().
> >
> > Then the conversions to {t,rw}sleep_nsec(9) become trivial, diff below.
>
> We can't do this until timeouts have a tickless interface. Otherwise
> your timeouts will return early. That's why I was saving the sys/kern
> conversions until after resolving that issue.
I don't understand, can you elaborate?
Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
> On Dec 31, 2019, at 9:35 AM, Martin Pieuchot wrote:
>
> I'd like to stop converting the given timespec to ticks and instead use
> nanoseconds. This is part of the ongoing effort to reduce the use of
> `hz' through the kernel.
>
> Since I don't know C I'd appreciate any pointer about the checks that
> should be added to TIMESPEC_TO_NSEC().
>
> Then the conversions to {t,rw}sleep_nsec(9) become trivial, diff below.
We can't do this until timeouts have a tickless interface. Otherwise
your timeouts will return early. That's why I was saving the sys/kern
conversions until after resolving that issue.
You could convert the conversion code to use tstohz(9) to minimize
copy-paste in the kernel, but that's about all you can do until
timeouts have a tickless interface.
Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
On 31/12/19(Tue) 15:00, Todd C. Miller wrote: > On Tue, 31 Dec 2019 16:35:59 +0100, Martin Pieuchot wrote: > > > I'd like to stop converting the given timespec to ticks and instead use > > nanoseconds. This is part of the ongoing effort to reduce the use of > > `hz' through the kernel. > > > > Since I don't know C I'd appreciate any pointer about the checks that > > should be added to TIMESPEC_TO_NSEC(). > > Is it possible for tv_sec or tv_nsec to be negative? Not in those two syscalls.
Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
On 31/12/19(Tue) 17:44, Alexander Bluhm wrote:
> On Tue, Dec 31, 2019 at 04:35:59PM +0100, Martin Pieuchot wrote:
> > Since I don't know C I'd appreciate any pointer about the checks that
> > should be added to TIMESPEC_TO_NSEC().
>
> Perhaps this way?
Thanks.
> static inline uint64_t
> TIMESPEC_TO_NSEC(const struct timespec *ts)
> {
> if (ts->tv_sec > (UINT64_MAX - ts->tv_nsec) / 10ULL)
> return UINT64_MAX;
> return ts->tv_sec * 10ULL + ts->tv_nsec;
> }
>
> Should we use inline instead of __inline for new code?
ok mpi@
> Index: sys/time.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/time.h,v
> retrieving revision 1.47
> diff -u -p -r1.47 time.h
> --- sys/time.h22 Oct 2019 20:19:41 - 1.47
> +++ sys/time.h31 Dec 2019 16:38:20 -
> @@ -177,7 +177,7 @@ struct bintime {
> (btp)->frac cmp (ctp)->frac : \
> (btp)->sec cmp (ctp)->sec)
>
> -static __inline void
> +static inline void
> bintimeaddfrac(const struct bintime *bt, uint64_t x, struct bintime *ct)
> {
> ct->sec = bt->sec;
> @@ -186,7 +186,7 @@ bintimeaddfrac(const struct bintime *bt,
> ct->frac = bt->frac + x;
> }
>
> -static __inline void
> +static inline void
> bintimeadd(const struct bintime *bt, const struct bintime *ct,
> struct bintime *dt)
> {
> @@ -196,7 +196,7 @@ bintimeadd(const struct bintime *bt, con
> dt->frac = bt->frac + ct->frac;
> }
>
> -static __inline void
> +static inline void
> bintimesub(const struct bintime *bt, const struct bintime *ct,
> struct bintime *dt)
> {
> @@ -220,14 +220,14 @@ bintimesub(const struct bintime *bt, con
> * time_second ticks after N.9 not after N.49
> */
>
> -static __inline void
> +static inline void
> BINTIME_TO_TIMESPEC(const struct bintime *bt, struct timespec *ts)
> {
> ts->tv_sec = bt->sec;
> ts->tv_nsec = (long)(((uint64_t)10 * (uint32_t)(bt->frac >>
> 32)) >> 32);
> }
>
> -static __inline void
> +static inline void
> TIMESPEC_TO_BINTIME(const struct timespec *ts, struct bintime *bt)
> {
> bt->sec = ts->tv_sec;
> @@ -235,14 +235,14 @@ TIMESPEC_TO_BINTIME(const struct timespe
> bt->frac = (uint64_t)ts->tv_nsec * (uint64_t)18446744073ULL;
> }
>
> -static __inline void
> +static inline void
> BINTIME_TO_TIMEVAL(const struct bintime *bt, struct timeval *tv)
> {
> tv->tv_sec = bt->sec;
> tv->tv_usec = (long)(((uint64_t)100 * (uint32_t)(bt->frac >> 32))
> >> 32);
> }
>
> -static __inline void
> +static inline void
> TIMEVAL_TO_BINTIME(const struct timeval *tv, struct bintime *bt)
> {
> bt->sec = (time_t)tv->tv_sec;
> @@ -332,14 +332,14 @@ void clock_secs_to_ymdhms(time_t, struct
> /* Traditional POSIX base year */
> #define POSIX_BASE_YEAR 1970
>
> -static __inline void
> +static inline void
> NSEC_TO_TIMEVAL(uint64_t ns, struct timeval *tv)
> {
> tv->tv_sec = ns / 10L;
> tv->tv_usec = (ns % 10L) / 1000;
> }
>
> -static __inline void
> +static inline void
> NSEC_TO_TIMESPEC(uint64_t ns, struct timespec *ts)
> {
> ts->tv_sec = ns / 10L;
> @@ -348,7 +348,7 @@ NSEC_TO_TIMESPEC(uint64_t ns, struct tim
>
> #include
>
> -static __inline uint64_t
> +static inline uint64_t
> SEC_TO_NSEC(uint64_t seconds)
> {
> if (seconds > UINT64_MAX / 10ULL)
> @@ -356,7 +356,7 @@ SEC_TO_NSEC(uint64_t seconds)
> return seconds * 10ULL;
> }
>
> -static __inline uint64_t
> +static inline uint64_t
> MSEC_TO_NSEC(uint64_t milliseconds)
> {
> if (milliseconds > UINT64_MAX / 100ULL)
> @@ -364,7 +364,7 @@ MSEC_TO_NSEC(uint64_t milliseconds)
> return milliseconds * 100ULL;
> }
>
> -static __inline uint64_t
> +static inline uint64_t
> USEC_TO_NSEC(uint64_t microseconds)
> {
> if (microseconds > UINT64_MAX / 1000ULL)
>
Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
On Tue, 31 Dec 2019 16:35:59 +0100, Martin Pieuchot wrote: > I'd like to stop converting the given timespec to ticks and instead use > nanoseconds. This is part of the ongoing effort to reduce the use of > `hz' through the kernel. > > Since I don't know C I'd appreciate any pointer about the checks that > should be added to TIMESPEC_TO_NSEC(). Is it possible for tv_sec or tv_nsec to be negative? - todd
Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
> Should we use inline instead of __inline for new code? inline is a C99 keyword, C99 is 20 years old now, and the kernel source is using C99 named initializers, therefore it is safe to assume C99 support from the compiler. It's time to stop defining __const, __inline, __signed and __volatile in and use underscore-free, lo-carb, keywords.
Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
On Tue, Dec 31, 2019 at 04:35:59PM +0100, Martin Pieuchot wrote:
> Since I don't know C I'd appreciate any pointer about the checks that
> should be added to TIMESPEC_TO_NSEC().
Perhaps this way?
static inline uint64_t
TIMESPEC_TO_NSEC(const struct timespec *ts)
{
if (ts->tv_sec > (UINT64_MAX - ts->tv_nsec) / 10ULL)
return UINT64_MAX;
return ts->tv_sec * 10ULL + ts->tv_nsec;
}
Should we use inline instead of __inline for new code?
bluhm
Index: sys/time.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/time.h,v
retrieving revision 1.47
diff -u -p -r1.47 time.h
--- sys/time.h 22 Oct 2019 20:19:41 - 1.47
+++ sys/time.h 31 Dec 2019 16:38:20 -
@@ -177,7 +177,7 @@ struct bintime {
(btp)->frac cmp (ctp)->frac : \
(btp)->sec cmp (ctp)->sec)
-static __inline void
+static inline void
bintimeaddfrac(const struct bintime *bt, uint64_t x, struct bintime *ct)
{
ct->sec = bt->sec;
@@ -186,7 +186,7 @@ bintimeaddfrac(const struct bintime *bt,
ct->frac = bt->frac + x;
}
-static __inline void
+static inline void
bintimeadd(const struct bintime *bt, const struct bintime *ct,
struct bintime *dt)
{
@@ -196,7 +196,7 @@ bintimeadd(const struct bintime *bt, con
dt->frac = bt->frac + ct->frac;
}
-static __inline void
+static inline void
bintimesub(const struct bintime *bt, const struct bintime *ct,
struct bintime *dt)
{
@@ -220,14 +220,14 @@ bintimesub(const struct bintime *bt, con
* time_second ticks after N.9 not after N.49
*/
-static __inline void
+static inline void
BINTIME_TO_TIMESPEC(const struct bintime *bt, struct timespec *ts)
{
ts->tv_sec = bt->sec;
ts->tv_nsec = (long)(((uint64_t)10 * (uint32_t)(bt->frac >>
32)) >> 32);
}
-static __inline void
+static inline void
TIMESPEC_TO_BINTIME(const struct timespec *ts, struct bintime *bt)
{
bt->sec = ts->tv_sec;
@@ -235,14 +235,14 @@ TIMESPEC_TO_BINTIME(const struct timespe
bt->frac = (uint64_t)ts->tv_nsec * (uint64_t)18446744073ULL;
}
-static __inline void
+static inline void
BINTIME_TO_TIMEVAL(const struct bintime *bt, struct timeval *tv)
{
tv->tv_sec = bt->sec;
tv->tv_usec = (long)(((uint64_t)100 * (uint32_t)(bt->frac >> 32))
>> 32);
}
-static __inline void
+static inline void
TIMEVAL_TO_BINTIME(const struct timeval *tv, struct bintime *bt)
{
bt->sec = (time_t)tv->tv_sec;
@@ -332,14 +332,14 @@ void clock_secs_to_ymdhms(time_t, struct
/* Traditional POSIX base year */
#define POSIX_BASE_YEAR 1970
-static __inline void
+static inline void
NSEC_TO_TIMEVAL(uint64_t ns, struct timeval *tv)
{
tv->tv_sec = ns / 10L;
tv->tv_usec = (ns % 10L) / 1000;
}
-static __inline void
+static inline void
NSEC_TO_TIMESPEC(uint64_t ns, struct timespec *ts)
{
ts->tv_sec = ns / 10L;
@@ -348,7 +348,7 @@ NSEC_TO_TIMESPEC(uint64_t ns, struct tim
#include
-static __inline uint64_t
+static inline uint64_t
SEC_TO_NSEC(uint64_t seconds)
{
if (seconds > UINT64_MAX / 10ULL)
@@ -356,7 +356,7 @@ SEC_TO_NSEC(uint64_t seconds)
return seconds * 10ULL;
}
-static __inline uint64_t
+static inline uint64_t
MSEC_TO_NSEC(uint64_t milliseconds)
{
if (milliseconds > UINT64_MAX / 100ULL)
@@ -364,7 +364,7 @@ MSEC_TO_NSEC(uint64_t milliseconds)
return milliseconds * 100ULL;
}
-static __inline uint64_t
+static inline uint64_t
USEC_TO_NSEC(uint64_t microseconds)
{
if (microseconds > UINT64_MAX / 1000ULL)
TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
I'd like to stop converting the given timespec to ticks and instead use
nanoseconds. This is part of the ongoing effort to reduce the use of
`hz' through the kernel.
Since I don't know C I'd appreciate any pointer about the checks that
should be added to TIMESPEC_TO_NSEC().
Then the conversions to {t,rw}sleep_nsec(9) become trivial, diff below.
Index: sys/time.h
===
RCS file: /cvs/src/sys/sys/time.h,v
retrieving revision 1.47
diff -u -p -r1.47 time.h
--- sys/time.h 22 Oct 2019 20:19:41 - 1.47
+++ sys/time.h 31 Dec 2019 10:20:26 -
@@ -372,6 +372,13 @@ USEC_TO_NSEC(uint64_t microseconds)
return microseconds * 1000ULL;
}
+static __inline uint64_t
+TIMESPEC_TO_NSEC(const struct timespec *ts)
+{
+ /* XXX overflow? */
+ return SEC_TO_NSEC(ts->tv_sec) + ts->tv_nsec;
+}
+
#else /* !_KERNEL */
#include
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 - 1.155
+++ kern/kern_synch.c 31 Dec 2019 10:19:25 -
@@ -624,7 +624,7 @@ thrsleep(struct proc *p, struct sys___th
long ident = (long)SCARG(uap, ident);
struct timespec *tsp = (struct timespec *)SCARG(uap, tp);
void *lock = SCARG(uap, lock);
- uint64_t to_ticks = 0;
+ uint64_t nsecs = INFSLP;
int abort, error;
clockid_t clock_id = SCARG(uap, clock_id);
@@ -648,10 +648,7 @@ thrsleep(struct proc *p, struct sys___th
}
timespecsub(tsp, , tsp);
- to_ticks = (uint64_t)hz * tsp->tv_sec +
- (tsp->tv_nsec + tick * 1000 - 1) / (tick * 1000) + 1;
- if (to_ticks > INT_MAX)
- to_ticks = INT_MAX;
+ nsecs = TIMESPEC_TO_NSEC(tsp);
}
p->p_thrslpid = ident;
@@ -675,8 +672,7 @@ thrsleep(struct proc *p, struct sys___th
void *sleepaddr = >p_thrslpid;
if (ident == -1)
sleepaddr =
- error = tsleep(sleepaddr, PWAIT|PCATCH, "thrsleep",
- (int)to_ticks);
+ error = tsleep_nsec(sleepaddr, PWAIT|PCATCH, "thrsleep", nsecs);
}
out:
Index: kern/sys_futex.c
===
RCS file: /cvs/src/sys/kern/sys_futex.c,v
retrieving revision 1.13
diff -u -p -r1.13 sys_futex.c
--- kern/sys_futex.c10 Jul 2019 15:52:17 - 1.13
+++ kern/sys_futex.c31 Dec 2019 10:19:49 -
@@ -213,7 +213,7 @@ futex_wait(uint32_t *uaddr, uint32_t val
{
struct proc *p = curproc;
struct futex *f;
- uint64_t to_ticks = 0;
+ uint64_t nsecs = INFSLP;
uint32_t cval;
int error;
@@ -244,17 +244,14 @@ futex_wait(uint32_t *uaddr, uint32_t val
#endif
if (ts.tv_sec < 0 || !timespecisvalid())
return EINVAL;
- to_ticks = (uint64_t)hz * ts.tv_sec +
- (ts.tv_nsec + tick * 1000 - 1) / (tick * 1000) + 1;
- if (to_ticks > INT_MAX)
- to_ticks = INT_MAX;
+ nsecs = TIMESPEC_TO_NSEC();
}
f = futex_get(uaddr, flags | FT_CREATE);
TAILQ_INSERT_TAIL(>ft_threads, p, p_fut_link);
p->p_futex = f;
- error = rwsleep(p, , PWAIT|PCATCH, "fsleep", (int)to_ticks);
+ error = rwsleep_nsec(p, , PWAIT|PCATCH, "fsleep", nsecs);
if (error == ERESTART)
error = ECANCELED;
else if (error == EWOULDBLOCK) {
