[PATCH 1/2] barrier: Remove leftover semaphore remnants

Joel Sherrill joel at rtems.org
Wed Oct 2 22:00:23 UTC 2019 

On Wed, Oct 2, 2019 at 2:52 PM Martin Erik Werner <
martinerikwerner at gmail.com> wrote:

> Thanks for the review! Discussion regarding suggested rewording and
> cleanup are inline below.
>
> One minor thing that I also noticed was in the background section there
> is a "cticket" which looks like a typo in "Similarly, cticket holders
> gather at the gates of arenas [...]". Unless it's some shorthand for
> concert-tickets that I'm unaware of? Unless it's intended I'll add that
> change to this patch set as well.
>
> On Tue, 2019-10-01 at 18:02 -0500, Joel Sherrill wrote:
> >
> >
> > On Tue, Oct 1, 2019, 5:07 PM Gedare Bloom <gedare at rtems.org> wrote:
> > > On Tue, Oct 1, 2019 at 3:53 PM Gedare Bloom <gedare at rtems.org>
> > > wrote:
> > > >
> > > > On Wed, Sep 4, 2019 at 3:36 PM Martin Erik Werner
> > > > <martinerikwerner at gmail.com> wrote:
> > > > >
> > > > > Remove various incorrect references to "lock" and "obtain" and
> > > to an
> > > > > option set which is not part of the barrier interface.
> > > > >
> > > > > It looks like the barrier documentation was started based on a
> > > copy of
> > > > > the semaphore documentation and these things are surviving
> > > remnants.
> > > > >
> > > > > Also remove an unfinished sentence in the barrier wait
> > > description,
> > > > > since the intended information is already provided in the under
> > > the NOTE
> > > > > label.
> > > > Thanks for your contribution!  I have a few minor comments:
> > > >
> > > > > ---
> > > > >  c-user/barrier_manager.rst | 38 +++++++-----------------------
> > > --------
> > > > >  1 file changed, 7 insertions(+), 31 deletions(-)
> > > > >
> > > > > diff --git a/c-user/barrier_manager.rst b/c-
> > > user/barrier_manager.rst
> > > > > index b0bf3bf..e5d69b0 100644
> > > > > --- a/c-user/barrier_manager.rst
> > > > > +++ b/c-user/barrier_manager.rst
> > > > > @@ -324,8 +324,7 @@ NOTES:
> > > > >
> > > > >  .. _rtems_barrier_wait:
> > > > >
> > > > > -.. index:: obtain a barrier
> > > > > -.. index:: lock a barrier
> > > > > +.. index:: wait at a barrier
> > > > >  .. index:: rtems_barrier_wait
> > > > >
> > > > >  BARRIER_WAIT - Wait at a barrier
> > > > > @@ -356,14 +355,11 @@ DIRECTIVE STATUS CODES:
> > > > >
> > > > >  DESCRIPTION:
> > > > >
> > > > > -    This directive acquires the barrier specified by ``id``.
> > > The
> > > > > -    ``RTEMS_WAIT`` and ``RTEMS_NO_WAIT`` components of the
> > > options parameter
> > > > > -    indicate whether the calling task wants to wait for the
> > > barrier to become
> > > > > -    available or return immediately if the barrier is not
> > > currently available.
> > > > > -    With either ``RTEMS_WAIT`` or ``RTEMS_NO_WAIT``, if the
> > > current barrier
> > > > > -    count is positive, then it is decremented by one and the
> > > barrier is
> > > > > -    successfully acquired by returning immediately with a
> > > successful return
> > > > > -    code.
> > > > > +    This directive waits at the barrier specified by ``id``.
> > > The timeout
> > > > > +    parameter specifies the maximum interval the calling task
> > > is willing to be
> > > > > +    blocked waiting for the barrier.  If it is set to
> > > ``RTEMS_NO_TIMEOUT``,
> > > > > +    then the calling task will wait forever.  If the barrier
> > > is available or
> > > > 1. "will wait forever" -> "will wait forever if it doesn't get
> > > > released" or something more precise, even just "may wait forever"
> >
> > Will wait until the barrier is released/opened. Then add there is no
> > option for no wait as found in other APIs.
>
> Ok, so
> "If it is set to ``RTEMS_NO_TIMEOUT``, then the calling task will wait
> until the barrier is released."
> ?
>
> The whole "If the barrier is available or the ``RTEMS_NO_WAIT`` option
> component is set, then the timeout is ignored." wording is another
> semaphore leftover that I missed when moving things around.
>
> I'm thinking I'll remove that whole sentence. replacing it with one of
> the following alternatives (or a mix thereof):
>
>     It is not possible to make the calling task return based on the timeout
>     without wating for at least one clock tick.
>
>     There is no equivalent to the ``RTEMS_NO_WAIT`` option for this
> directive,
>     hence the minimum timeout possible is one clock tick.
>
>     It is not possible to make the calling task return immediately based
> on the
>     timeout, the minimum timeout is 1 clock tick.
>
>     The minimum timeout interval is one clock tick and it is therefore not
>     possible to make the calling task return immediately based on the
> timeout.
>
> I'm still not particularly happy with any of them, but I'm a bit stuck
> for the precise wording.
>

How about starting with something like, "a barrier is always considered
to be closed (e.g. unavailable) and, consequently, polling does not make
sense. The barrier wait operation always blocks with the option of timeout."

When teaching the RTEMS class, I always highlight that Classic API
blocking operations are poll, block forever, or block with timeout. This
is the sole exception. It is because the barrier is always closed when you
arrive at it. If this can come across somehow in the description, that
would be good.

Plus you editing gives me more time to install Sphinx in my new VM. Mine
crashed badly earlier this week. :(


>
> > > > 2. I'm not quite sure what "available" means in this context.
> > > Maybe
> > > > "open" is a better word?
> > > >
> > > I guess "available" is the wording that has been used throughout.
> > > Again, probably due to inheriting from the semaphore documentation.
> > > I
> > > think "open/closed" is more precise when describing a barrier, but
> > > available is OK if it is consistently applied.
> >
> > Open/closed is better. I think of a barrier as the starting gates at
> > a horse race. Horses arrive and wait while the gates are closed. When
> > the race starts, all gates are opened and the horses are released.
> > > I would still like the fix on #1, even though that was not
> > > introduced
> > > by you just copied from elsewhere, it is good to improve it at the
> > > same time.
> > >
> > > > > +    the ``RTEMS_NO_WAIT`` option component is set, then
> > > timeout is ignored.
> > > > >
> > > > >      Conceptually, the calling task should always be thought of
> > > as blocking when
> > > > >      it makes this call and being unblocked when the barrier is
> > > released.  If
> > > > > @@ -372,24 +368,7 @@ DESCRIPTION:
> > > > >      callers will block except for the one which is the Nth
> > > task which trips the
> > > > >      automatic release condition.
> > > > >
> > > > > -    The timeout parameter specifies the maximum interval the
> > > calling task is
> > > > > -    willing to be blocked waiting for the barrier.  If it is
> > > set to
> > > > > -    ``RTEMS_NO_TIMEOUT``, then the calling task will wait
> > > forever.  If the
> > > > > -    barrier is available or the ``RTEMS_NO_WAIT`` option
> > > component is set, then
> > > > > -    timeout is ignored.
> > > > > -
> > > > >  NOTES:
> > > > > -
> > > > > -    The following barrier acquisition option constants are
> > > defined by RTEMS:
> > > > > -
> > > > > -    .. list-table::
> > > > > -     :class: rtems-table
> > > > > -
> > > > > -     * - ``RTEMS_WAIT``
> > > > > -       - task will wait for barrier (default)
> > > > > -     * - ``RTEMS_NO_WAIT``
> > > > > -       - task should not wait
> > > > > -
> > > > >      A clock tick is required to support the timeout
> > > functionality of this
> > > > >      directive.
> > > > >
> > > > > @@ -399,7 +378,6 @@ NOTES:
> > > > >
> > > > >  .. _rtems_barrier_release:
> > > > >
> > > > > -.. index:: wait at a barrier
> > > > >  .. index:: release a barrier
> > > > >  .. index:: rtems_barrier_release
> > > > >
> > > > > @@ -425,9 +403,7 @@ DIRECTIVE STATUS CODES:
> > > > >
> > > > >  DESCRIPTION:
> > > > >      This directive releases the barrier specified by id.  All
> > > tasks waiting at
> > > > > -    the barrier will be unblocked.  If the running task's
> > > preemption mode is
> > > > > -    enabled and one of the unblocked tasks has a higher
> > > priority than the
> > > > > -    running task.
> > > > > +    the barrier will be unblocked.
> > > > >
> > > > >  NOTES:
> > > > >      The calling task may be preempted if it causes a higher
> > > priority task to be
> > > > > --
> > > > > 2.11.0
> > > > >
> > > > > _______________________________________________
> > > > > devel mailing list
> > > > > devel at rtems.org
> > > > > http://lists.rtems.org/mailman/listinfo/devel
> > > _______________________________________________
> > > devel mailing list
> > > devel at rtems.org
> > > http://lists.rtems.org/mailman/listinfo/devel
> --
> Martin Erik Werner <martinerikwerner at gmail.com>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20191002/65ba5fbf/attachment-0001.html>

More information about the devel mailing list