<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 1:32 PM 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 19:41, Kinsey Moore wrote:<br>
> On Wed, Dec 13, 2023 at 8:35 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 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>><br>
> > <mailto:<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<br>
> threads of<br>
> > > execution, even before the introduction of delayed write<br>
> 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<br>
> file<br>
> > system instance lock for normal operations (no delayed works<br>
> stuff)?<br>
> ><br>
> > Your new code still has no test cases and the configuration<br>
> 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>
> > <<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<br>
> variable C<br>
> > used with D.<br>
> ><br>
> > 2. Add a queue for the delayed work to the fs information and<br>
> a node to<br>
> > register the info in the delayed work support.<br>
> ><br>
> > 3. The first delayed work request of a JFFS2 instance<br>
> 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<br>
> using<br>
> > D and C.<br>
> ><br>
> > The delayed work uses the fs info mutex to protect the work.<br>
> You need<br>
> > also reference count for the fs info to control the work and<br>
> 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<br>
> abusing<br>
> > macros to pull in information that isn't actually provided to<br>
> them (and<br>
> > would require that local variable naming be extremely consistent<br>
> 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>
> <br>
> <br>
> I could, but I prefer to minimize changes to external code where <br>
> possible. It's not strictly necessary in this case, so I'm avoiding it.<br>
> <br>
> <br>
> > Other<br>
> > implementations that use this library achieve safe locking<br>
> without the<br>
> > FS information lock.<br>
> <br>
> What is "this library"?<br>
> <br>
> <br>
> The JFFS2 library.<br>
> <br>
> <br>
> Before you started with adding some locks here and some locks there,<br>
> 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<br>
> 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>
> <br>
> <br>
> The lock is not available to the delayed work caller without modifying <br>
> the JFFS2 code and, while I'm sure it would work fine from a data <br>
> integrity perspective, it was not intended to operate that way. If I <br>
> were going to go this direction to reduce complexity, it might make more <br>
> sense to disable delayed write support and force all writes to be <br>
> immediate such that it behaves like NOR. The downside to reduced locking <br>
> granularity or delayed write removal would be additional wear on the <br>
> NAND flash.<br>
<br>
In which place in the code do you have problems to get the fs info <br>
block? I am absolutely not in favour of having the internal locking <br>
enabled for JFFS2. We use this file system on lower end controllers.<br>
<br></blockquote><div>The call to submit delayed work does not provide the FS info as a parameter:<br></div><div>void jffs2_queue_delayed_work(struct delayed_work *work, int delay_ms);</div><div>This is wrapped as:</div><div>#define queue_delayed_work(workqueue, delayed_work, delay_ms) ...</div><div><br></div><div>The initialization call for delayed work does not have direct access to the FS info, either:</div><div>#define INIT_DELAYED_WORK(delayed_work, delayed_workqueue_callback) ...<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Independent of this, for NAND I would use YAFFS2.<br></blockquote><div><br></div><div>Unfortunately, YAFFS2 is licensed GPL2 and not suitable for inclusion in an RTEMS application without commercial licensing, dynamic loading via libdl, or writing an implementation of that file system from scratch. I would agree that JFFS2 isn't ideal for modern NAND given the 4GB file system limit.</div><div><br></div><div>Kinsey<br></div></div></div>