avoid unnnecessary mtd read when read can be satisfied by write buffer

The following small patch modifies wbuf.c::jffs2_flash_read to avoid a call to the mtd read layer in the case where all data returned by the read would be replaced by data from the write buffer anyway. This condition occurs relatively infrequently, but the cost of the test is low and the benefit of avoiding the mtd read is high.

index a7f153f..01ef266 100644
@@ -907,6 +907,20 @@ int jffs2_flash_read(struct jffs2_sb_inf
 
   /* Read flash */
   down_read(&c->wbuf_sem);
+
+    /* Check if the read can be completely satisfied from the write buffer;
+     * if so, we can avoid the mtd call entirely.
+     */
+    if (c->wbuf_pagesize &&
+        (SECTOR_ADDR(ofs) == SECTOR_ADDR(c->wbuf_ofs)) &&
+        (ofs >= c->wbuf_ofs) &&
+        (ofs + len) <= (c->wbuf_ofs + c->wbuf_len)) {
+        memcpy(buf, c->wbuf + (ofs - c->wbuf_ofs), len);
+        *retlen = len;
+        ret = 0;
+        goto exit;
+    }
+
   ret = c->mtd->read(c->mtd, ofs, len, retlen, buf);
 
   if ( (ret == -EBADMSG || ret == -EUCLEAN) && (*retlen == len) ) {

Not sure whether this micro optimization is worth the code. A real solution would be to have a cache for the full device. Possibly read the device through pagecache somehow (writing obviously should be write-through, not write-back). That would have a significantly higher hit rate than your patch.

Jörn

Don't worry about people stealing your ideas. If your ideas are any good,
you'll have to ram them down people's throats.
-- Howard Aiken quoted by Ken Iverson quoted by Jim Horning quoted by
   Raph Levien, 1979

We haven't written to the device yet, at this point -- this is just the case where the data in question exist _only_ in the wbuf, but we're actually reading from the flash first _anyway_, with the current code.

I don't think that caching the compressed nodes from the flash is going to be particularly useful -- we already have the page cache for _uncompressed_ data. For other things we might want to cache, such as dirents and symlinks, we'll do _much_ better to store that in memory for ourselves rather than just caching whole pages of the backing store for such sparse content.

dwmw2

Good point. Ignore my objections then.

My observations have been a bit different. I've had to deal with a semi-broken simulator, where flash accesses were unbearably slow. Booting took about an hour. The simulator has been fixed since, but the broken state made some details fairly visible. :)

One of the observations was that a lot of flash is being read multiple times. Reads on NAND happen in page granularity, so first we have to read pretty much everything during scan, later again when accessing the actual files. There is some room for savings, as RAM is faster than flash even in the real world.

Jörn

A quarrel is quickly settled when deserted by one party; there is
no battle unless there be two.
-- Seneca

My point is that for any given problem, raw caching of flash is not the answer -- something more cunning is going to be better.

The specific cases of dirents and symlinks I already talked about. Any other time we keep reading the same bit of flash over and over again is also a problem. The one you mention is fixed by using eraseblock summary.

I need to finish what I was doing yesterday, which halves the amount of space taken by summaries. We should probably also remove the dependency on EXPERIMENTAL.

dwmw2

Not necessarily. JFFS2, all metadata is not just cached, but cannot even get evicted from cache. Logfs*) currently doesn't cache metadata at all, which is fairly bad. But there is no requirement to pin the metadata into memory, so it can have a dynamic cache with a shrinker. And now the caching should be based on a (device, offset) tupel. Nothing cunning about it, except to decide whether any given offset contains metadata or file data, which is already cached elsewhere.

*) No code released yet, yadda yadda

Jörn

The wise man seeks everything in himself; the ignorant man tries to get
everything from somebody else.
-- unknown