<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Oct 2, 2019 at 2:52 PM Martin Erik Werner <<a href="mailto:martinerikwerner@gmail.com">martinerikwerner@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Thanks for the review! Discussion regarding suggested rewording and<br>
cleanup are inline below.<br>
<br>
One minor thing that I also noticed was in the background section there<br>
is a "cticket" which looks like a typo in "Similarly, cticket holders<br>
gather at the gates of arenas [...]". Unless it's some shorthand for<br>
concert-tickets that I'm unaware of? Unless it's intended I'll add that<br>
change to this patch set as well.<br>
<br>
On Tue, 2019-10-01 at 18:02 -0500, Joel Sherrill wrote:<br>
> <br>
> <br>
> On Tue, Oct 1, 2019, 5:07 PM Gedare Bloom <<a href="mailto:gedare@rtems.org" target="_blank">gedare@rtems.org</a>> wrote:<br>
> > On Tue, Oct 1, 2019 at 3:53 PM Gedare Bloom <<a href="mailto:gedare@rtems.org" target="_blank">gedare@rtems.org</a>><br>
> > wrote:<br>
> > ><br>
> > > On Wed, Sep 4, 2019 at 3:36 PM Martin Erik Werner<br>
> > > <<a href="mailto:martinerikwerner@gmail.com" target="_blank">martinerikwerner@gmail.com</a>> wrote:<br>
> > > ><br>
> > > > Remove various incorrect references to "lock" and "obtain" and<br>
> > to an<br>
> > > > option set which is not part of the barrier interface.<br>
> > > ><br>
> > > > It looks like the barrier documentation was started based on a<br>
> > copy of<br>
> > > > the semaphore documentation and these things are surviving<br>
> > remnants.<br>
> > > ><br>
> > > > Also remove an unfinished sentence in the barrier wait<br>
> > description,<br>
> > > > since the intended information is already provided in the under<br>
> > the NOTE<br>
> > > > label.<br>
> > > Thanks for your contribution! I have a few minor comments:<br>
> > ><br>
> > > > ---<br>
> > > > c-user/barrier_manager.rst | 38 +++++++-----------------------<br>
> > --------<br>
> > > > 1 file changed, 7 insertions(+), 31 deletions(-)<br>
> > > ><br>
> > > > diff --git a/c-user/barrier_manager.rst b/c-<br>
> > user/barrier_manager.rst<br>
> > > > index b0bf3bf..e5d69b0 100644<br>
> > > > --- a/c-user/barrier_manager.rst<br>
> > > > +++ b/c-user/barrier_manager.rst<br>
> > > > @@ -324,8 +324,7 @@ NOTES:<br>
> > > ><br>
> > > > .. _rtems_barrier_wait:<br>
> > > ><br>
> > > > -.. index:: obtain a barrier<br>
> > > > -.. index:: lock a barrier<br>
> > > > +.. index:: wait at a barrier<br>
> > > > .. index:: rtems_barrier_wait<br>
> > > ><br>
> > > > BARRIER_WAIT - Wait at a barrier<br>
> > > > @@ -356,14 +355,11 @@ DIRECTIVE STATUS CODES:<br>
> > > ><br>
> > > > DESCRIPTION:<br>
> > > ><br>
> > > > - This directive acquires the barrier specified by ``id``. <br>
> > The<br>
> > > > - ``RTEMS_WAIT`` and ``RTEMS_NO_WAIT`` components of the<br>
> > options parameter<br>
> > > > - indicate whether the calling task wants to wait for the<br>
> > barrier to become<br>
> > > > - available or return immediately if the barrier is not<br>
> > currently available.<br>
> > > > - With either ``RTEMS_WAIT`` or ``RTEMS_NO_WAIT``, if the<br>
> > current barrier<br>
> > > > - count is positive, then it is decremented by one and the<br>
> > barrier is<br>
> > > > - successfully acquired by returning immediately with a<br>
> > successful return<br>
> > > > - code.<br>
> > > > + This directive waits at the barrier specified by ``id``. <br>
> > The timeout<br>
> > > > + parameter specifies the maximum interval the calling task<br>
> > is willing to be<br>
> > > > + blocked waiting for the barrier. If it is set to<br>
> > ``RTEMS_NO_TIMEOUT``,<br>
> > > > + then the calling task will wait forever. If the barrier<br>
> > is available or<br>
> > > 1. "will wait forever" -> "will wait forever if it doesn't get<br>
> > > released" or something more precise, even just "may wait forever"<br>
> <br>
> Will wait until the barrier is released/opened. Then add there is no<br>
> option for no wait as found in other APIs. <br>
<br>
Ok, so<br>
"If it is set to ``RTEMS_NO_TIMEOUT``, then the calling task will wait<br>
until the barrier is released."<br>
?<br>
<br>
The whole "If the barrier is available or the ``RTEMS_NO_WAIT`` option<br>
component is set, then the timeout is ignored." wording is another<br>
semaphore leftover that I missed when moving things around.<br>
<br>
I'm thinking I'll remove that whole sentence. replacing it with one of<br>
the following alternatives (or a mix thereof):<br>
<br>
It is not possible to make the calling task return based on the timeout<br>
without wating for at least one clock tick.<br>
<br>
There is no equivalent to the ``RTEMS_NO_WAIT`` option for this directive,<br>
hence the minimum timeout possible is one clock tick.<br>
<br>
It is not possible to make the calling task return immediately based on the<br>
timeout, the minimum timeout is 1 clock tick.<br>
<br>
The minimum timeout interval is one clock tick and it is therefore not<br>
possible to make the calling task return immediately based on the timeout.<br>
<br>
I'm still not particularly happy with any of them, but I'm a bit stuck<br>
for the precise wording.<br></blockquote><div><br></div><div>How about starting with something like, "a barrier is always considered</div><div>to be closed (e.g. unavailable) and, consequently, polling does not make</div><div>sense. The barrier wait operation always blocks with the option of timeout."</div><div><br></div><div>When teaching the RTEMS class, I always highlight that Classic API </div><div>blocking operations are poll, block forever, or block with timeout. This</div><div>is the sole exception. It is because the barrier is always closed when you</div><div>arrive at it. If this can come across somehow in the description, that</div><div>would be good.</div><div><br></div><div>Plus you editing gives me more time to install Sphinx in my new VM. Mine</div><div>crashed badly earlier this week. :(</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> > > 2. I'm not quite sure what "available" means in this context.<br>
> > Maybe<br>
> > > "open" is a better word?<br>
> > ><br>
> > I guess "available" is the wording that has been used throughout.<br>
> > Again, probably due to inheriting from the semaphore documentation.<br>
> > I<br>
> > think "open/closed" is more precise when describing a barrier, but<br>
> > available is OK if it is consistently applied.<br>
> <br>
> Open/closed is better. I think of a barrier as the starting gates at<br>
> a horse race. Horses arrive and wait while the gates are closed. When<br>
> the race starts, all gates are opened and the horses are released. <br>
> > I would still like the fix on #1, even though that was not<br>
> > introduced<br>
> > by you just copied from elsewhere, it is good to improve it at the<br>
> > same time.<br>
> > <br>
> > > > + the ``RTEMS_NO_WAIT`` option component is set, then<br>
> > timeout is ignored.<br>
> > > ><br>
> > > > Conceptually, the calling task should always be thought of<br>
> > as blocking when<br>
> > > > it makes this call and being unblocked when the barrier is<br>
> > released. If<br>
> > > > @@ -372,24 +368,7 @@ DESCRIPTION:<br>
> > > > callers will block except for the one which is the Nth<br>
> > task which trips the<br>
> > > > automatic release condition.<br>
> > > ><br>
> > > > - The timeout parameter specifies the maximum interval the<br>
> > calling task is<br>
> > > > - willing to be blocked waiting for the barrier. If it is<br>
> > set to<br>
> > > > - ``RTEMS_NO_TIMEOUT``, then the calling task will wait<br>
> > forever. If the<br>
> > > > - barrier is available or the ``RTEMS_NO_WAIT`` option<br>
> > component is set, then<br>
> > > > - timeout is ignored.<br>
> > > > -<br>
> > > > NOTES:<br>
> > > > -<br>
> > > > - The following barrier acquisition option constants are<br>
> > defined by RTEMS:<br>
> > > > -<br>
> > > > - .. list-table::<br>
> > > > - :class: rtems-table<br>
> > > > -<br>
> > > > - * - ``RTEMS_WAIT``<br>
> > > > - - task will wait for barrier (default)<br>
> > > > - * - ``RTEMS_NO_WAIT``<br>
> > > > - - task should not wait<br>
> > > > -<br>
> > > > A clock tick is required to support the timeout<br>
> > functionality of this<br>
> > > > directive.<br>
> > > ><br>
> > > > @@ -399,7 +378,6 @@ NOTES:<br>
> > > ><br>
> > > > .. _rtems_barrier_release:<br>
> > > ><br>
> > > > -.. index:: wait at a barrier<br>
> > > > .. index:: release a barrier<br>
> > > > .. index:: rtems_barrier_release<br>
> > > ><br>
> > > > @@ -425,9 +403,7 @@ DIRECTIVE STATUS CODES:<br>
> > > ><br>
> > > > DESCRIPTION:<br>
> > > > This directive releases the barrier specified by id. All<br>
> > tasks waiting at<br>
> > > > - the barrier will be unblocked. If the running task's<br>
> > preemption mode is<br>
> > > > - enabled and one of the unblocked tasks has a higher<br>
> > priority than the<br>
> > > > - running task.<br>
> > > > + the barrier will be unblocked.<br>
> > > ><br>
> > > > NOTES:<br>
> > > > The calling task may be preempted if it causes a higher<br>
> > priority task to be<br>
> > > > --<br>
> > > > 2.11.0<br>
> > > ><br>
> > > > _______________________________________________<br>
> > > > devel mailing list<br>
> > > > <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
> > > > <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
> > _______________________________________________<br>
> > devel mailing list<br>
> > <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
> > <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
-- <br>
Martin Erik Werner <<a href="mailto:martinerikwerner@gmail.com" target="_blank">martinerikwerner@gmail.com</a>><br>
<br>
</blockquote></div></div>