Date: Tue, 17 Feb 2015 23:28:34 +0000 From: "wblock (Warren Block)" <phabric-noreply@FreeBSD.org> To: freebsd-net@freebsd.org Subject: [Differential] [Changed Subscribers] D1438: FreeBSD callout rewrite and cleanup Message-ID: <55424bb6afde62db71124c0bd96403f3@localhost.localdomain> In-Reply-To: <differential-rev-PHID-DREV-o4lr3ts4fgj46hyp6sg3-req@FreeBSD.org> References: <differential-rev-PHID-DREV-o4lr3ts4fgj46hyp6sg3-req@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
wblock added a subscriber: wblock. wblock added a comment. This is as much as I have time for at the moment. I'll add doc to the reviewers list INLINE COMMENTS share/man/man9/timeout.9:73 That is kind of a difficult sentence to parse. Does this retain the meaning? API is used to schedule a one-time call to an arbitrary function at a specific future time. share/man/man9/timeout.9:76 The original is an "aside": Consumers of this API are required to allocate a callout structure (struct callout) for each pending function invocation. The parentheses are probably the wrong markup for the new form, and "struct callout structure" is redundant. Maybe this: Consumers of this API are required to allocate a .Ft struct callout for each pending function invocation. share/man/man9/timeout.9:79 As above, use .Ft and the word "structure" is redundant. share/man/man9/timeout.9:80 "should be" is a recommendation. Does it mean I ought to call those functions before freeing, but will work fine without it? Otherwise, consider "must be drained by a call to". share/man/man9/timeout.9:85 The rule is to use American English spellings unless the entire document was written with British spellings. See https://www.freebsd.org/doc/en_US.ISO8859-1/books/fdp-primer/writing-style-guidelines.html share/man/man9/timeout.9:88 It would be better to say .Pq Deprecated, see blahblah for current usage. This function is used to prepare a share/man/man9/timeout.9:90 Use .Ft and remove the redundant word "structure". share/man/man9/timeout.9:94 "Was" is a little odd here, but "were" is not quite right either. How about: function will return as if no timeout was pending. share/man/man9/timeout.9:97 Again, better to say the function is deprecated and give a pointer to the replacement in a separate sentence. Then explain what this one does in the next sentence. share/man/man9/timeout.9:101 .Ft and s/structure// share/man/man9/timeout.9:107 .Ft and s/structure// share/man/man9/timeout.9:114 Possessive: application's share/man/man9/timeout.9:116 Comma: functions, including the share/man/man9/timeout.9:121 s/Else/Otherwise,/ share/man/man9/timeout.9:122 Avoid simultaneous access by obtaining an exclusive lock share/man/man9/timeout.9:132 Split this long sentence: function is called. It is also expected, but currently not share/man/man9/timeout.9:135 This last part needs to be split out into a separate sentence, starting with ", except when the". share/man/man9/timeout.9:144 .Ft and s/structure// share/man/man9/timeout.9:149 Passive "should be" and should, and "specify a pointer"... How about: argument is a non-zero pointer to a valid share/man/man9/timeout.9:153 The specified mutex is expected to be locked when calling any share/man/man9/timeout.9:155 functions other than the share/man/man9/timeout.9:159 functions. share/man/man9/timeout.9:165 The callout function is assumed to have released the specified share/man/man9/timeout.9:167 s/Else/Otherwise,/ share/man/man9/timeout.9:173 This function is similar to the share/man/man9/timeout.9:175 function, but accepts a read-mostly type of lock. share/man/man9/timeout.9:176 s/initialised/initialized/ share/man/man9/timeout.9:182 This function is similar to the share/man/man9/timeout.9:184 function, but accepts a reader-writer type of lock. Should that be "read/write" instead? share/man/man9/timeout.9:188 As above, separate sentence to say it is deprecated and point to the replacement. share/man/man9/timeout.9:199 argument is a valid pointer to a function that takes a single share/man/man9/timeout.9:223 "canceled" is spelled "cancelled" elsewhere in this man page. The first is modern American. share/man/man9/timeout.9:243 s/Else/Otherwise,/ share/man/man9/timeout.9:246 s/called/called,/ share/man/man9/timeout.9:249 s/words/words,/ share/man/man9/timeout.9:250 s/Else/Otherwise,/ share/man/man9/timeout.9:273 s/Else/Otherwise,/ share/man/man9/timeout.9:295 s/Else/Otherwise,/ share/man/man9/timeout.9:309 .Fa pr argument. is redundant, and can be just .Fa pr . (There are lots of these.) share/man/man9/timeout.9:310 Avoid the informal use of "you": This function is used when high precision timeouts are needed. share/man/man9/timeout.9:313 Again, redundant. Can be just If .Fa sbt specifies... share/man/man9/timeout.9:324 "the" is not needed here. share/man/man9/timeout.9:334 Again, can just be By default, .Fa sbt is treated... Also, "is treated *as*". share/man/man9/timeout.9:346 Needs a serial comma before "and": ...1/4, and so on. share/man/man9/timeout.9:362 This function works like share/man/man9/timeout.9:363 .Fn callout_reset_sbt , share/man/man9/timeout.9:364 except the callback function given by share/man/man9/timeout.9:366 will be executed on the CPU which called this function. share/man/man9/timeout.9:375 Otherwise, the callback function given by share/man/man9/timeout.9:377 will be executed on the same CPU... The rest of that line is unclear. As shown previously in the man page? As run previously on the CPU? share/man/man9/timeout.9:382 This function works like .Fn callout_reset_sbt , except the callback function given by .Fa func will be executed on the CPU given by .Fa cpu . share/man/man9/timeout.9:392 Refer to .Fa callout_init . REVISION DETAIL https://reviews.freebsd.org/D1438 To: hselasky, jhb, adrian, markj, emaste, sbruno, imp, lstewart, rwatson, gnn, rrs, kostikbel, delphij, neel, erj Cc: wblock, freebsd-net
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?55424bb6afde62db71124c0bd96403f3>