[PATCH 1/2] cpukit/jffs2: Set extracted nodes off-chain

[Kinsey Moore]

On Thu, Sep 21, 2023 at 10:01 AM Sebastian Huber <
sebastian.huber at embedded-brains.de> wrote:

> On 21.09.23 16:52, Kinsey Moore wrote:
> > On Thu, Sep 21, 2023 at 9:47 AM Sebastian Huber
> > <sebastian.huber at embedded-brains.de
> > <mailto:sebastian.huber at embedded-brains.de>> wrote:
> >
> >     On 20.09.23 20:35, Kinsey Moore wrote:
> >     [...]
> >      > @@ -1306,8 +1307,22 @@ static void process_delayed_work(void)
> >      >       while (!rtems_chain_is_tail(&process_work_chain, node)) {
> >      >               work = (struct delayed_work*) node;
> >      >               rtems_chain_node* next_node =
> rtems_chain_next(node);
> >      > +
> >      > +             /*
> >      > +              * Don't leave extracted node exposed to other
> >     operations
> >      > +              * under RTEMS_DEBUG
> >      > +              */
> >      > +#ifdef RTEMS_DEBUG
> >      > +             mutex_lock(&delayed_work_mutex);
> >      > +#endif
> >      >               rtems_chain_extract(node);
> >      > +#ifdef RTEMS_DEBUG
> >      > +             node->next = node;
> >      > +             mutex_unlock(&delayed_work_mutex);
> >      > +#endif
> >      > +
> >      >               work->callback(&work->work);
> >      > +             rtems_chain_set_off_chain(node);
> >      >               node = next_node;
> >      >       }
> >      >   }
> >
> >     This change makes no sense to me. The code should work regardless if
> >     RTEMS_DEBUG is defined or not.
> >
> >
> > RTEMS_DEBUG introduces a behavioral change in rtems_chain_extract() such
> > that extracted nodes are automatically set off-chain. When RTEMS_DEBUG
> > is not set, node->next is left untouched. This has to be managed because
> > this code needs the node to remain on-chain so that it is not re-queued
> > during the operation.
>
> Yes, but while a node is on a chain you must not call
> rtems_chain_set_off_chain(). If you want to use the off-chain state,
> then you have to use this:
>
> rtems_chain_extract(node);
> rtems_chain_set_off_chain(node);
>
> or
>
> rtems_chain_extract_unprotected(node);
> rtems_chain_set_off_chain(node);
>
> The automatic set off-chain in RTEMS_DEBUG is just to ensure that basic
> chain operations are used in the right state.
>

Yes, there is no behavior here where rtems_chain_set_off_chain() is being
called on a node that is still in a chain. This section of code is entirely
managing the side-effect of RTEMS_DEBUG that sets the node in the off-chain
state post-extraction. In this case, that side-effect is undesirable and so
a lock is in place to prevent that temporary side-effect from leaking to
other parts of the system since all other off-chain checks are behind the
same lock.

Kinsey
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20230921/b39cb0e1/attachment.htm>