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

[Sebastian Huber]

On 13.12.23 19:41, Kinsey Moore wrote:
> On Wed, Dec 13, 2023 at 8:35 AM Sebastian Huber 
> <sebastian.huber at embedded-brains.de 
> <mailto: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>
>      > <mailto: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>
>      >     <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.

In which place in the code do you have problems to get the fs info 
block? I am absolutely not in favour of having the internal locking 
enabled for JFFS2. We use this file system on lower end controllers.

Independent of this, for NAND I would use YAFFS2.

-- 
embedded brains GmbH & Co. KG
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber at embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/