Re: [v3] timeout.9: document missing interfaces, miscellaneous rewrites
On Sat, Dec 31, 2022 at 11:31:21PM +0100, Ingo Schwarze wrote: > Hi Scott, > > in the NAME section, please put timeout_add_nsec after timeout_add_usec > to agree with the order in the SYNOPSIS. Whoops, done. > In any case, please go ahead. It appears jmc@ is developing a sore > elbow from more than a year of medicine ball ping pong. ;-) These rewrites tend to gather a lot of moss, yeah :) > The following are merely suggestions / observations / questions, > not conditions. > > Scott Cheloha wrote on Sat, Dec 31, 2022 at 11:22:22AM -0500: > > > mvs@ is nudging me to realign the timeout.9 page with the state of the > > kernel. > > > > Here is my rewrite (again). > > > > There are some bits that I want to rework. The opening paragraph is > > especially clickety-clackety. > > The opening paragraph seems fine to me. The second paragraph might > be considered a bit awkward. If you rename the struct timeout > argument from *to to *timeout in all prototypes in the SYNOPSIS > and everywhere in the text, you might be able to join the second and > third paragraphs, for example as follows: > > All state is encapsulated in a > .Vt struct timeout > allocated by the caller. > The > .Fn timeout_set > function initializes the > .Fa timeout > before it can be passed to any of the other functions. > When the timeout ... > > That way, you get rid of the word "caller-allocated" and the > parenthetic remark, and some of the later text may also simplify > all by itself in a natural way. I need to rethink this. I'm going to go ahead as-is. Maybe we can improve it later. > > Still, I think this is an improvement over what's in-tree. And the > > technical content should be complete and accurate, which is the most > > important thing. > > > > ok? > > > > Index: timeout.9 > > === > > RCS file: /cvs/src/share/man/man9/timeout.9,v > > retrieving revision 1.55 > > diff -u -p -r1.55 timeout.9 > > --- timeout.9 22 Jun 2022 14:10:49 - 1.55 > > +++ timeout.9 31 Dec 2022 16:19:27 - > [...] > > @@ -44,281 +46,370 @@ > [...] > > -Once initialized, the > > -.Fa to > > -structure can be used repeatedly in > > -.Fn timeout_add > > -and > > -.Fn timeout_del > > -and does not need to be reinitialized unless > > -the function called and/or its argument must change. > > Are you deleting this information on purpose? I think timeout reinitialization is a terrible idea. A bug magnet. An accident waiting to happen. ... I suppose we ought to include the information, though. I have put it back. > [...] > > -timeout in at least > > Are you deleting the words "at least" on purpose, or should they be > reinserted into this sentence: > > KCLOCK_NONE timeouts may be scheduled with the function timeout_add(), > which schedules the given timeout to execute after nticks hardclock(9) > ticks have elapsed. Whoops, I have put it back, thanks. > [...] > > +The > > .Fn timeout_del_barrier > > -is like > > -.Fn timeout_del > > -but it will wait until any current execution of the timeout has completed. > > +function is similar to > > +.Fn timeout_del , > > +except that it may block until any current execution of the given timeout > > +has completed. > > This appears to change the meaning. > > The old text provides a guarantee that timeout_del_barrier(9) > does not return before the timeout completes, if it is currently > running. The new wording no longer provides that guarantee. > It that intentional? > > Otherwise, s/may block/blocks/ ? Or, if you think it should be > more explicit, maybe something like: > > except that, if the timeout is currently executing, > .Fn timeout_del > blocks until execution of the timeout has completed. I have tweaked it to indicate that it does not return until any ongoing execution is completed. > [...] > > -Otherwise, the system will deadlock. > > +Otherwise, > > +the system will deadlock. > > No need to change that. The rule "break the line after a comma" > is specific to the Linux man pages project, and i consider it excessive. > We only follow "new sentence, new line". Oh, sure. That's useful. I will keep that in mind in the future. I assumed it was a rule, but I can't remember where I got it from. -- Gonna go with the attached. Index: timeout.9 === RCS file: /cvs/src/share/man/man9/timeout.9,v retrieving revision 1.55 diff -u -p -r1.55 timeout.9 --- timeout.9 22 Jun 2022 14:10:49 - 1.55 +++ timeout.9 1 Jan 2023 01:01:23 - @@ -1,6 +1,7 @@ .\"$OpenBSD: timeout.9,v 1.55 2022/06/22 14:10:49 visa Exp $ .\" .\" Copyright (c) 2000 Artur Grabowski +.\" Copyright (c) 2021, 2022 Scott Cheloha .\" All rights reserved. .\" .\" Redistribution and use in source and binary forms, with or without @@ -33,9 +34,10 @@ .Nm timeout_add , .Nm timeout_add_sec , .Nm timeout_add_msec , -.Nm timeout_add_nsec , .Nm
Re: [v3] timeout.9: document missing interfaces, miscellaneous rewrites
Hi Scott, in the NAME section, please put timeout_add_nsec after timeout_add_usec to agree with the order in the SYNOPSIS. In any case, please go ahead. It appears jmc@ is developing a sore elbow from more than a year of medicine ball ping pong. ;-) The following are merely suggestions / observations / questions, not conditions. Scott Cheloha wrote on Sat, Dec 31, 2022 at 11:22:22AM -0500: > mvs@ is nudging me to realign the timeout.9 page with the state of the > kernel. > > Here is my rewrite (again). > > There are some bits that I want to rework. The opening paragraph is > especially clickety-clackety. The opening paragraph seems fine to me. The second paragraph might be considered a bit awkward. If you rename the struct timeout argument from *to to *timeout in all prototypes in the SYNOPSIS and everywhere in the text, you might be able to join the second and third paragraphs, for example as follows: All state is encapsulated in a .Vt struct timeout allocated by the caller. The .Fn timeout_set function initializes the .Fa timeout before it can be passed to any of the other functions. When the timeout ... That way, you get rid of the word "caller-allocated" and the parenthetic remark, and some of the later text may also simplify all by itself in a natural way. > Still, I think this is an improvement over what's in-tree. And the > technical content should be complete and accurate, which is the most > important thing. > > ok? > > Index: timeout.9 > === > RCS file: /cvs/src/share/man/man9/timeout.9,v > retrieving revision 1.55 > diff -u -p -r1.55 timeout.9 > --- timeout.9 22 Jun 2022 14:10:49 - 1.55 > +++ timeout.9 31 Dec 2022 16:19:27 - [...] > @@ -44,281 +46,370 @@ [...] > -Once initialized, the > -.Fa to > -structure can be used repeatedly in > -.Fn timeout_add > -and > -.Fn timeout_del > -and does not need to be reinitialized unless > -the function called and/or its argument must change. Are you deleting this information on purpose? [...] > -timeout in at least Are you deleting the words "at least" on purpose, or should they be reinserted into this sentence: KCLOCK_NONE timeouts may be scheduled with the function timeout_add(), which schedules the given timeout to execute after nticks hardclock(9) ticks have elapsed. [...] > +The > .Fn timeout_del_barrier > -is like > -.Fn timeout_del > -but it will wait until any current execution of the timeout has completed. > +function is similar to > +.Fn timeout_del , > +except that it may block until any current execution of the given timeout > +has completed. This appears to change the meaning. The old text provides a guarantee that timeout_del_barrier(9) does not return before the timeout completes, if it is currently running. The new wording no longer provides that guarantee. It that intentional? Otherwise, s/may block/blocks/ ? Or, if you think it should be more explicit, maybe something like: except that, if the timeout is currently executing, .Fn timeout_del blocks until execution of the timeout has completed. [...] > -Otherwise, the system will deadlock. > +Otherwise, > +the system will deadlock. No need to change that. The rule "break the line after a comma" is specific to the Linux man pages project, and i consider it excessive. We only follow "new sentence, new line". Yours, Ingo
Re: [v3] timeout.9: document missing interfaces, miscellaneous rewrites
Thanks! ok mvs@ > On 31 Dec 2022, at 19:22, Scott Cheloha wrote: > > mvs@ is nudging me to realign the timeout.9 page with the state of the > kernel. > > Here is my rewrite (again). > > There are some bits that I want to rework. The opening paragraph is > especially clickety-clackety. > > Still, I think this is an improvement over what's in-tree. And the > technical content should be complete and accurate, which is the most > important thing. > > ok? > > Index: timeout.9 > === > RCS file: /cvs/src/share/man/man9/timeout.9,v > retrieving revision 1.55 > diff -u -p -r1.55 timeout.9 > --- timeout.9 22 Jun 2022 14:10:49 - 1.55 > +++ timeout.9 31 Dec 2022 16:19:27 - > @@ -1,6 +1,7 @@ > .\" $OpenBSD: timeout.9,v 1.55 2022/06/22 14:10:49 visa Exp $ > .\" > .\" Copyright (c) 2000 Artur Grabowski > +.\" Copyright (c) 2021, 2022 Scott Cheloha > .\" All rights reserved. > .\" > .\" Redistribution and use in source and binary forms, with or without > @@ -36,6 +37,7 @@ > .Nm timeout_add_nsec , > .Nm timeout_add_usec , > .Nm timeout_add_tv , > +.Nm timeout_abs_ts , > .Nm timeout_del , > .Nm timeout_del_barrier , > .Nm timeout_barrier , > @@ -44,281 +46,370 @@ > .Nm timeout_triggered , > .Nm TIMEOUT_INITIALIZER , > .Nm TIMEOUT_INITIALIZER_FLAGS > -.Nd execute a function after a specified period of time > +.Nd execute a function in the future > .Sh SYNOPSIS > .In sys/types.h > .In sys/timeout.h > .Ft void > -.Fn timeout_set "struct timeout *to" "void (*fn)(void *)" "void *arg" > +.Fo timeout_set > +.Fa "struct timeout *to" > +.Fa "void (*fn)(void *)" > +.Fa "void *arg" > +.Fc > .Ft void > .Fo timeout_set_flags > .Fa "struct timeout *to" > .Fa "void (*fn)(void *)" > .Fa "void *arg" > +.Fa "int kclock" > .Fa "int flags" > .Fc > .Ft void > -.Fn timeout_set_proc "struct timeout *to" "void (*fn)(void *)" "void *arg" > +.Fo timeout_set_proc > +.Fa "struct timeout *to" > +.Fa "void (*fn)(void *)" > +.Fa "void *arg" > +.Fc > .Ft int > -.Fn timeout_add "struct timeout *to" "int ticks" > +.Fo timeout_add > +.Fa "struct timeout *to" > +.Fa "int nticks" > +.Fc > .Ft int > -.Fn timeout_del "struct timeout *to" > +.Fo timeout_add_sec > +.Fa "struct timeout *to" > +.Fa "int secs" > +.Fc > .Ft int > -.Fn timeout_del_barrier "struct timeout *to" > -.Ft void > -.Fn timeout_barrier "struct timeout *to" > +.Fo timeout_add_msec > +.Fa "struct timeout *to" > +.Fa "int msecs" > +.Fc > .Ft int > -.Fn timeout_pending "struct timeout *to" > +.Fo timeout_add_usec > +.Fa "struct timeout *to" > +.Fa "int usecs" > +.Fc > .Ft int > -.Fn timeout_initialized "struct timeout *to" > +.Fo timeout_add_nsec > +.Fa "struct timeout *to" > +.Fa "int nsecs" > +.Fc > .Ft int > -.Fn timeout_triggered "struct timeout *to" > +.Fo timeout_add_tv > +.Fa "struct timeout *to" > +.Fa "struct timeval *tv" > +.Fc > .Ft int > -.Fn timeout_add_tv "struct timeout *to" "struct timeval *" > +.Fo timeout_abs_ts > +.Fa "struct timeout *to" > +.Fa "const struct timespec *abs" > +.Fc > .Ft int > -.Fn timeout_add_sec "struct timeout *to" "int sec" > +.Fo timeout_del > +.Fa "struct timeout *to" > +.Fc > .Ft int > -.Fn timeout_add_msec "struct timeout *to" "int msec" > +.Fo timeout_del_barrier > +.Fa "struct timeout *to" > +.Fc > +.Ft void > +.Fo timeout_barrier > +.Fa "struct timeout *to" > +.Fc > +.Ft int > +.Fo timeout_pending > +.Fa "struct timeout *to" > +.Fc > .Ft int > -.Fn timeout_add_usec "struct timeout *to" "int usec" > +.Fo timeout_initialized > +.Fa "struct timeout *to" > +.Fc > .Ft int > -.Fn timeout_add_nsec "struct timeout *to" "int nsec" > -.Fn TIMEOUT_INITIALIZER "void (*fn)(void *)" "void *arg" > -.Fn TIMEOUT_INITIALIZER_FLAGS "void (*fn)(void *)" "void *arg" "int flags" > +.Fo timeout_triggered > +.Fa "struct timeout *to" > +.Fc > +.Fo TIMEOUT_INITIALIZER > +.Fa "void (*fn)(void *)" > +.Fa "void *arg" > +.Fc > +.Fo TIMEOUT_INITIALIZER_FLAGS > +.Fa "void (*fn)(void *)" > +.Fa "void *arg" > +.Fa "int kclock" > +.Fa "int flags" > +.Fc > .Sh DESCRIPTION > The > .Nm timeout > -API provides a mechanism to execute a function at a given time. > -The granularity of the time is limited by the granularity of the > -.Xr hardclock 9 > -timer which executes > -.Xr hz 9 > -times a second. > +API provides a mechanism to schedule a function for asynchronous > +execution in the future. > .Pp > -It is the responsibility of the caller to provide these functions with > -pre-allocated timeout structures. > +All state is encapsulated in a caller-allocated > +.Vt struct timeout > +.Po > +hereafter, > +a > +.Qq timeout > +.Pc . > +A timeout must be initialized before it may be used as input to other > +functions in the API. > .Pp > The > .Fn timeout_set > -function prepares the timeout structure > -.Fa to > -to be used in future calls to > -.Fn timeout_add > -and > -.Fn timeout_del . > -The timeout will be prepared to call the function specified by the > +function initializes the timeout > +.Fa to .
Re: [v3] timeout.9: document missing interfaces, miscellaneous rewrites
On Sat, Dec 31, 2022 at 11:22:22AM -0500, Scott Cheloha wrote: > mvs@ is nudging me to realign the timeout.9 page with the state of the > kernel. > > Here is my rewrite (again). > i'm losing track of what i'm reviewing! maybe just put it in and we can adjust it as needed, if needed. ok jmc
[v3] timeout.9: document missing interfaces, miscellaneous rewrites
mvs@ is nudging me to realign the timeout.9 page with the state of the kernel. Here is my rewrite (again). There are some bits that I want to rework. The opening paragraph is especially clickety-clackety. Still, I think this is an improvement over what's in-tree. And the technical content should be complete and accurate, which is the most important thing. ok? Index: timeout.9 === RCS file: /cvs/src/share/man/man9/timeout.9,v retrieving revision 1.55 diff -u -p -r1.55 timeout.9 --- timeout.9 22 Jun 2022 14:10:49 - 1.55 +++ timeout.9 31 Dec 2022 16:19:27 - @@ -1,6 +1,7 @@ .\"$OpenBSD: timeout.9,v 1.55 2022/06/22 14:10:49 visa Exp $ .\" .\" Copyright (c) 2000 Artur Grabowski +.\" Copyright (c) 2021, 2022 Scott Cheloha .\" All rights reserved. .\" .\" Redistribution and use in source and binary forms, with or without @@ -36,6 +37,7 @@ .Nm timeout_add_nsec , .Nm timeout_add_usec , .Nm timeout_add_tv , +.Nm timeout_abs_ts , .Nm timeout_del , .Nm timeout_del_barrier , .Nm timeout_barrier , @@ -44,281 +46,370 @@ .Nm timeout_triggered , .Nm TIMEOUT_INITIALIZER , .Nm TIMEOUT_INITIALIZER_FLAGS -.Nd execute a function after a specified period of time +.Nd execute a function in the future .Sh SYNOPSIS .In sys/types.h .In sys/timeout.h .Ft void -.Fn timeout_set "struct timeout *to" "void (*fn)(void *)" "void *arg" +.Fo timeout_set +.Fa "struct timeout *to" +.Fa "void (*fn)(void *)" +.Fa "void *arg" +.Fc .Ft void .Fo timeout_set_flags .Fa "struct timeout *to" .Fa "void (*fn)(void *)" .Fa "void *arg" +.Fa "int kclock" .Fa "int flags" .Fc .Ft void -.Fn timeout_set_proc "struct timeout *to" "void (*fn)(void *)" "void *arg" +.Fo timeout_set_proc +.Fa "struct timeout *to" +.Fa "void (*fn)(void *)" +.Fa "void *arg" +.Fc .Ft int -.Fn timeout_add "struct timeout *to" "int ticks" +.Fo timeout_add +.Fa "struct timeout *to" +.Fa "int nticks" +.Fc .Ft int -.Fn timeout_del "struct timeout *to" +.Fo timeout_add_sec +.Fa "struct timeout *to" +.Fa "int secs" +.Fc .Ft int -.Fn timeout_del_barrier "struct timeout *to" -.Ft void -.Fn timeout_barrier "struct timeout *to" +.Fo timeout_add_msec +.Fa "struct timeout *to" +.Fa "int msecs" +.Fc .Ft int -.Fn timeout_pending "struct timeout *to" +.Fo timeout_add_usec +.Fa "struct timeout *to" +.Fa "int usecs" +.Fc .Ft int -.Fn timeout_initialized "struct timeout *to" +.Fo timeout_add_nsec +.Fa "struct timeout *to" +.Fa "int nsecs" +.Fc .Ft int -.Fn timeout_triggered "struct timeout *to" +.Fo timeout_add_tv +.Fa "struct timeout *to" +.Fa "struct timeval *tv" +.Fc .Ft int -.Fn timeout_add_tv "struct timeout *to" "struct timeval *" +.Fo timeout_abs_ts +.Fa "struct timeout *to" +.Fa "const struct timespec *abs" +.Fc .Ft int -.Fn timeout_add_sec "struct timeout *to" "int sec" +.Fo timeout_del +.Fa "struct timeout *to" +.Fc .Ft int -.Fn timeout_add_msec "struct timeout *to" "int msec" +.Fo timeout_del_barrier +.Fa "struct timeout *to" +.Fc +.Ft void +.Fo timeout_barrier +.Fa "struct timeout *to" +.Fc +.Ft int +.Fo timeout_pending +.Fa "struct timeout *to" +.Fc .Ft int -.Fn timeout_add_usec "struct timeout *to" "int usec" +.Fo timeout_initialized +.Fa "struct timeout *to" +.Fc .Ft int -.Fn timeout_add_nsec "struct timeout *to" "int nsec" -.Fn TIMEOUT_INITIALIZER "void (*fn)(void *)" "void *arg" -.Fn TIMEOUT_INITIALIZER_FLAGS "void (*fn)(void *)" "void *arg" "int flags" +.Fo timeout_triggered +.Fa "struct timeout *to" +.Fc +.Fo TIMEOUT_INITIALIZER +.Fa "void (*fn)(void *)" +.Fa "void *arg" +.Fc +.Fo TIMEOUT_INITIALIZER_FLAGS +.Fa "void (*fn)(void *)" +.Fa "void *arg" +.Fa "int kclock" +.Fa "int flags" +.Fc .Sh DESCRIPTION The .Nm timeout -API provides a mechanism to execute a function at a given time. -The granularity of the time is limited by the granularity of the -.Xr hardclock 9 -timer which executes -.Xr hz 9 -times a second. +API provides a mechanism to schedule a function for asynchronous +execution in the future. .Pp -It is the responsibility of the caller to provide these functions with -pre-allocated timeout structures. +All state is encapsulated in a caller-allocated +.Vt struct timeout +.Po +hereafter, +a +.Qq timeout +.Pc . +A timeout must be initialized before it may be used as input to other +functions in the API. .Pp The .Fn timeout_set -function prepares the timeout structure -.Fa to -to be used in future calls to -.Fn timeout_add -and -.Fn timeout_del . -The timeout will be prepared to call the function specified by the +function initializes the timeout +.Fa to . +When the timeout is executed, +the function .Fa fn -argument with a -.Fa void * -argument given in the +will be called with .Fa arg -argument. -Once initialized, the -.Fa to -structure can be used repeatedly in -.Fn timeout_add -and -.Fn timeout_del -and does not need to be reinitialized unless -the function called and/or its argument must change. +as its sole parameter. +The timeout is
