<meta http-equiv="Content-Type" content="text/html; charset=utf-8"><div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Dec 13, 2023 at 8:35 AM Sebastian Huber <<a href="mailto:sebastian.huber@embedded-brains.de">sebastian.huber@embedded-brains.de</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">On 13.12.23 15:27, Kinsey Moore wrote:<br>
> On Wed, Dec 13, 2023 at 12:26 AM Sebastian Huber <br>
> <<a href="mailto:sebastian.huber@embedded-brains.de" target="_blank">sebastian.huber@embedded-brains.de</a> <br>
> <mailto:<a href="mailto:sebastian.huber@embedded-brains.de" target="_blank">sebastian.huber@embedded-brains.de</a>>> wrote:<br>
> <br>
>     On 09.12.23 03:31, Kinsey Moore wrote:<br>
>      > The inode cache can be altered and queried by multiple threads of<br>
>      > execution, even before the introduction of delayed write support for<br>
>      > NAND. This provides a new lock to prevent simultaneous<br>
>     modification of<br>
>      > the cache.<br>
> <br>
>     Under which condition is the inode cache accessed without the file<br>
>     system instance lock for normal operations (no delayed works stuff)?<br>
> <br>
>     Your new code still has no test cases and the configuration option is<br>
>     not documented (<a href="http://devel.rtems.org/ticket/4961" rel="noreferrer" target="_blank">http://devel.rtems.org/ticket/4961</a><br>
>     <<a href="http://devel.rtems.org/ticket/4961" rel="noreferrer" target="_blank">http://devel.rtems.org/ticket/4961</a>>).<br>
> <br>
>     I am still in favour of an alternative locking approach:<br>
> <br>
>     1. The delayed work support uses a mutex D and a condition variable C<br>
>     used with D.<br>
> <br>
>     2. Add a queue for the delayed work to the fs information and a node to<br>
>     register the info in the delayed work support.<br>
> <br>
>     3. The first delayed work request of a JFFS2 instance registers the fs<br>
>     information in the delayed work support and uses C to signal the<br>
>     work to<br>
>     the delayed work task.<br>
> <br>
>     4. Further requests just get enqueued and signaled using D and C.<br>
> <br>
>     5. When a instance is unmounted, drain the delayed work queue using<br>
>     D and C.<br>
> <br>
>     The delayed work uses the fs info mutex to protect the work. You need<br>
>     also reference count for the fs info to control the work and the drain<br>
>     during unmount.<br>
> <br>
> <br>
> Using the FS information lock at the level of delayed work callback <br>
> isn't workable with the current API exposed/consumed by the JFFS2 <br>
> library as this information is not exposed to the thread calling the <br>
> delayed work without modification of the JFFS2 library itself or abusing <br>
> macros to pull in information that isn't actually provided to them (and <br>
> would require that local variable naming be extremely consistent across <br>
> usages of this abusive macro).All that is available is the callback <br>
> function pointer and an opaque void pointer argument. <br>
<br>
I don't understand the problem. If you need JFFS2 specific details, why <br>
don't you implement this part to the JFFS2 area?<br></blockquote><div><br></div><div>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.<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> Other <br>
> implementations that use this library achieve safe locking without the <br>
> FS information lock.<br>
<br>
What is "this library"?<br></blockquote><div><br></div><div>The JFFS2 library. <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Before you started with adding some locks here and some locks there, the <br>
complete JFFS2 state was protected by a single instance lock. This is <br>
not great in terms of SMP performance, however, it is very simple and it <br>
works. I don't know why you can't get the instance lock, do the delayed <br>
work, and then release the instance lock.<br></blockquote><div><br></div><div>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.</div><div><br></div><div>Kinsey<br></div></div></div>