[PATCH 2/9] cpukit/jffs2: Protect the inode cache

[Kinsey Moore]

On Wed, Dec 13, 2023 at 8:35 AM Sebastian Huber <
sebastian.huber at embedded-brains.de> wrote:

> On 13.12.23 15:27, Kinsey Moore wrote:
> > On Wed, Dec 13, 2023 at 12:26 AM Sebastian Huber
> > <sebastian.huber at embedded-brains.de
> > <mailto:sebastian.huber at embedded-brains.de>> wrote:
> >
> >     On 09.12.23 03:31, Kinsey Moore wrote:
> >      > The inode cache can be altered and queried by multiple threads of
> >      > execution, even before the introduction of delayed write support
> for
> >      > NAND. This provides a new lock to prevent simultaneous
> >     modification of
> >      > the cache.
> >
> >     Under which condition is the inode cache accessed without the file
> >     system instance lock for normal operations (no delayed works stuff)?
> >
> >     Your new code still has no test cases and the configuration option is
> >     not documented (http://devel.rtems.org/ticket/4961
> >     <http://devel.rtems.org/ticket/4961>).
> >
> >     I am still in favour of an alternative locking approach:
> >
> >     1. The delayed work support uses a mutex D and a condition variable C
> >     used with D.
> >
> >     2. Add a queue for the delayed work to the fs information and a node
> to
> >     register the info in the delayed work support.
> >
> >     3. The first delayed work request of a JFFS2 instance registers the
> fs
> >     information in the delayed work support and uses C to signal the
> >     work to
> >     the delayed work task.
> >
> >     4. Further requests just get enqueued and signaled using D and C.
> >
> >     5. When a instance is unmounted, drain the delayed work queue using
> >     D and C.
> >
> >     The delayed work uses the fs info mutex to protect the work. You need
> >     also reference count for the fs info to control the work and the
> drain
> >     during unmount.
> >
> >
> > Using the FS information lock at the level of delayed work callback
> > isn't workable with the current API exposed/consumed by the JFFS2
> > library as this information is not exposed to the thread calling the
> > delayed work without modification of the JFFS2 library itself or abusing
> > macros to pull in information that isn't actually provided to them (and
> > would require that local variable naming be extremely consistent across
> > usages of this abusive macro).All that is available is the callback
> > function pointer and an opaque void pointer argument.
>
> I don't understand the problem. If you need JFFS2 specific details, why
> don't you implement this part to the JFFS2 area?
>

I could, but I prefer to minimize changes to external code where possible.
It's not strictly necessary in this case, so I'm avoiding it.

>
> > Other
> > implementations that use this library achieve safe locking without the
> > FS information lock.
>
> What is "this library"?
>

The JFFS2 library.

>
> Before you started with adding some locks here and some locks there, the
> complete JFFS2 state was protected by a single instance lock. This is
> not great in terms of SMP performance, however, it is very simple and it
> works. I don't know why you can't get the instance lock, do the delayed
> work, and then release the instance lock.
>

The lock is not available to the delayed work caller without modifying the
JFFS2 code and, while I'm sure it would work fine from a data integrity
perspective, it was not intended to operate that way. If I were going to go
this direction to reduce complexity, it might make more sense to disable
delayed write support and force all writes to be immediate such that it
behaves like NOR. The downside to reduced locking granularity or delayed
write removal would be additional wear on the NAND flash.

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