[PATCH 0/7] UBIFS: fix recovery on CFI NOR

[PATCH 0/7] UBIFS: fix recovery on CFI NOR

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

Hi,

here is the patch-set against the latest Linux kernel (e.g. 2.6.38-rc3)
which should fix CFI NOR flash recovery.

The previous attempt was not entirely successful - it broke backward
compatibility and was reverted:

http://marc.info/?l=linux-kernel&m=129631939419818&w=2

This patch-set goes the following way:
1. Incorporates the notion of 'writebufsize' into UBI and UBIFS as
   'max_write_size', because using term write-buffer would be confusing, as
   UBIFS has its own write-buffers.
2. Changes UBIFS write-buffer and makes it of 'max_write_size', instead of
   'min_io_size'. This presumably leads to better performance because we
   accumulate more data and write them in larger chunks and faster.
   And we do not waste space when synchronizing UBIFS write-buffers because we
   write only the used amount of bytes aligned to 'min_io_size'. So this is an
   improvement.
3. Tweak UBIFS recovery and make it aware of the fact that we can write in
   chunks larger than 'min_io_size'. Namely, we can write in 'max_write_size'
   chunks.

Could you guys please test this WRT power cuts and let me know if it solves the
issues?

Note, obviously this requires patches from Anatolij which introduce
'writebufsize' to MTD, except of the UBI patch which was reverted later.

Artem.

[PATCH 1/7] UBI: incorporate maximum write size

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

Incorporate MTD write buffer size into UBI device information
because UBIFS needs this field. UBI does not use it ATM, just
provides to upper layers in 'struct ubi_device_info'.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 drivers/mtd/ubi/build.c |   13 +++++++++++++
 drivers/mtd/ubi/kapi.c  |    1 +
 drivers/mtd/ubi/ubi.h   |    3 +++
 include/linux/mtd/ubi.h |   15 +++++++++++++++
 4 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 5ebe280..d7fa52e 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -690,6 +690,19 @@ static int io_init(struct ubi_device *ubi)
 	ubi_assert(ubi->hdrs_min_io_size <= ubi->min_io_size);
 	ubi_assert(ubi->min_io_size % ubi->hdrs_min_io_size == 0);
 
+	ubi->max_write_size = ubi->mtd->writebufsize;
+	/*
+	 * Maximum write size has to be greater or equivalent to min. I/O
+	 * size, and be multiple of min. I/O size.
+	 */
+	if (ubi->max_write_size < ubi->min_io_size ||
+	    ubi->max_write_size % ubi->min_io_size ||
+	    !is_power_of_2(ubi->max_write_size)) {
+		ubi_err("bad write buffer size %d for %d min. I/O unit",
+			ubi->max_write_size, ubi->min_io_size);
+		return -EINVAL;
+	}
+
 	/* Calculate default aligned sizes of EC and VID headers */
 	ubi->ec_hdr_alsize = ALIGN(UBI_EC_HDR_SIZE, ubi->hdrs_min_io_size);
 	ubi->vid_hdr_alsize = ALIGN(UBI_VID_HDR_SIZE, ubi->hdrs_min_io_size);
diff --git a/drivers/mtd/ubi/kapi.c b/drivers/mtd/ubi/kapi.c
index 69fa4ef0..701df4f 100644
--- a/drivers/mtd/ubi/kapi.c
+++ b/drivers/mtd/ubi/kapi.c
@@ -41,6 +41,7 @@ void ubi_do_get_device_info(struct ubi_device *ubi, struct ubi_device_info *di)
 	di->ubi_num = ubi->ubi_num;
 	di->leb_size = ubi->leb_size;
 	di->min_io_size = ubi->min_io_size;
+	di->max_write_size = ubi->max_write_size;
 	di->ro_mode = ubi->ro_mode;
 	di->cdev = ubi->cdev.dev;
 }
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 0b0149c..b789943 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -381,6 +381,8 @@ struct ubi_wl_entry;
  * @bad_allowed: whether the MTD device admits of bad physical eraseblocks or
  *               not
  * @nor_flash: non-zero if working on top of NOR flash
+ * @max_write_size: maximum amount of bytes the underlying flash can write at a
+ *                  time (MTD write buffer size)
  * @mtd: MTD device descriptor
  *
  * @peb_buf1: a buffer of PEB size used for different purposes
@@ -464,6 +466,7 @@ struct ubi_device {
 	int vid_hdr_shift;
 	unsigned int bad_allowed:1;
 	unsigned int nor_flash:1;
+	int max_write_size;
 	struct mtd_info *mtd;
 
 	void *peb_buf1;
diff --git a/include/linux/mtd/ubi.h b/include/linux/mtd/ubi.h
index b31bd9e..238b32c 100644
--- a/include/linux/mtd/ubi.h
+++ b/include/linux/mtd/ubi.h
@@ -117,17 +117,32 @@ struct ubi_volume_info {
  * @ubi_num: ubi device number
  * @leb_size: logical eraseblock size on this UBI device
  * @min_io_size: minimal I/O unit size
+ * @max_write_size: maximum amount of bytes the underlying flash can write at a
+ *                  time (MTD write buffer size)
  * @ro_mode: if this device is in read-only mode
  * @cdev: UBI character device major and minor numbers
  *
  * Note, @leb_size is the logical eraseblock size offered by the UBI device.
  * Volumes of this UBI device may have smaller logical eraseblock size if their
  * alignment is not equivalent to %1.
+ *
+ * The @max_write_size field describes flash write maximum write unit. For
+ * example, NOR flash allows for changing individual bytes, so @min_io_size is
+ * %1. However, it does not mean than NOR flash has to write data byte-by-byte.
+ * Instead, CFI NOR flashes have a write-buffer of, e.g., 64 bytes, and when
+ * writing large chunks of data, they write 64-bytes at a time. Obviously, this
+ * improves write throughput.
+ *
+ * The @max_write_size field is always greater or equivalent to @min_io_size.
+ * E.g., some NOR flashes may have (@min_io_size = 1, @max_write_size = 64). In
+ * contrast, NAND flashes usually have @min_io_size = @max_write_size = NAND
+ * page size.
  */
 struct ubi_device_info {
 	int ubi_num;
 	int leb_size;
 	int min_io_size;
+	int max_write_size;
 	int ro_mode;
 	dev_t cdev;
 };
-- 
1.7.2.3

[PATCH 2/7] UBIFS: incorporate maximum write size

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

Incorporate maximum write size into the UBIFS description data
structure. This patch just introduces new 'c->max_write_size'
and 'c->max_write_shift' fields as a preparation for the following
patches.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 fs/ubifs/super.c |   19 +++++++++++++++++++
 fs/ubifs/ubifs.h |    5 +++++
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index d203c99..6747b59 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -515,6 +515,8 @@ static int init_constants_early(struct ubifs_info *c)
 	c->half_leb_size = c->leb_size / 2;
 	c->min_io_size = c->di.min_io_size;
 	c->min_io_shift = fls(c->min_io_size) - 1;
+	c->max_write_size = c->di.max_write_size;
+	c->max_write_shift = fls(c->max_write_size) - 1;
 
 	if (c->leb_size < UBIFS_MIN_LEB_SZ) {
 		ubifs_err("too small LEBs (%d bytes), min. is %d bytes",
@@ -533,6 +535,18 @@ static int init_constants_early(struct ubifs_info *c)
 		return -EINVAL;
 	}
 
+ 	/*
+	 * Maximum write size has to be greater or equivalent to min. I/O
+	 * size, and be multiple of min. I/O size.
+	 */
+	if (c->max_write_size < c->min_io_size ||
+	    c->max_write_size % c->min_io_size ||
+	    !is_power_of_2(c->max_write_size)) {
+		ubifs_err("bad write buffer size %d for %d min. I/O unit",
+			  c->max_write_size, c->min_io_size);
+		return -EINVAL;
+	}
+
 	/*
 	 * UBIFS aligns all node to 8-byte boundary, so to make function in
 	 * io.c simpler, assume minimum I/O unit size to be 8 bytes if it is
@@ -541,6 +555,10 @@ static int init_constants_early(struct ubifs_info *c)
 	if (c->min_io_size < 8) {
 		c->min_io_size = 8;
 		c->min_io_shift = 3;
+		if (c->max_write_size < c->min_io_size) {
+			c->max_write_size = c->min_io_size;
+			c->max_write_shift = c->min_io_shift;
+		}
 	}
 
 	c->ref_node_alsz = ALIGN(UBIFS_REF_NODE_SZ, c->min_io_size);
@@ -1399,6 +1417,7 @@ static int mount_ubifs(struct ubifs_info *c)
 
 	dbg_msg("compiled on:         " __DATE__ " at " __TIME__);
 	dbg_msg("min. I/O unit size:  %d bytes", c->min_io_size);
+	dbg_msg("max. write size:     %d bytes", c->max_write_size);
 	dbg_msg("LEB size:            %d bytes (%d KiB)",
 		c->leb_size, c->leb_size >> 10);
 	dbg_msg("data journal heads:  %d",
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index d182354..8b51949 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -1024,6 +1024,9 @@ struct ubifs_debug_info;
  *
  * @min_io_size: minimal input/output unit size
  * @min_io_shift: number of bits in @min_io_size minus one
+ * @max_write_size: maximum amount of bytes the underlying flash can write at a
+ *                  time (MTD write buffer size)
+ * @max_write_shift: number of bits in @max_write_size minus one
  * @leb_size: logical eraseblock size in bytes
  * @half_leb_size: half LEB size
  * @idx_leb_size: how many bytes of an LEB are effectively available when it is
@@ -1270,6 +1273,8 @@ struct ubifs_info {
 
 	int min_io_size;
 	int min_io_shift;
+	int max_write_size;
+	int max_write_shift;
 	int leb_size;
 	int half_leb_size;
 	int idx_leb_size;
-- 
1.7.2.3

[PATCH 3/7] UBIFS: introduce write-buffer size field

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

Currently we assume write-buffer size is always min_io_size. But
this is about to change and write-buffers may be of variable size.
Namely, they will be of max_write_size at the beginning, but will
get smaller when we are approaching the end of LEB.

This is a preparation patch which introduces 'size' field in
the write-buffer structure which carries the current write-buffer
size.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 fs/ubifs/io.c    |   28 +++++++++++++++++++---------
 fs/ubifs/ubifs.h |    2 ++
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index d1fe562..7c2a014 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -361,7 +361,10 @@ int ubifs_wbuf_sync_nolock(struct ubifs_wbuf *wbuf)
 	dbg_io("LEB %d:%d, %d bytes, jhead %s",
 	       wbuf->lnum, wbuf->offs, wbuf->used, dbg_jhead(wbuf->jhead));
 	ubifs_assert(!(wbuf->avail & 7));
-	ubifs_assert(wbuf->offs + c->min_io_size <= c->leb_size);
+	ubifs_assert(wbuf->offs + wbuf->size <= c->leb_size);
+	ubifs_assert(wbuf->size >= c->min_io_size);
+	ubifs_assert(wbuf->size <= c->max_write_size);
+	ubifs_assert(wbuf->size % c->min_io_size == 0);
 	ubifs_assert(!c->ro_media && !c->ro_mount);
 
 	if (c->ro_error)
@@ -369,10 +372,10 @@ int ubifs_wbuf_sync_nolock(struct ubifs_wbuf *wbuf)
 
 	ubifs_pad(c, wbuf->buf + wbuf->used, wbuf->avail);
 	err = ubi_leb_write(c->ubi, wbuf->lnum, wbuf->buf, wbuf->offs,
-			    c->min_io_size, wbuf->dtype);
+			    wbuf->size, wbuf->dtype);
 	if (err) {
 		ubifs_err("cannot write %d bytes to LEB %d:%d",
-			  c->min_io_size, wbuf->lnum, wbuf->offs);
+			  wbuf->size, wbuf->lnum, wbuf->offs);
 		dbg_dump_stack();
 		return err;
 	}
@@ -380,8 +383,9 @@ int ubifs_wbuf_sync_nolock(struct ubifs_wbuf *wbuf)
 	dirt = wbuf->avail;
 
 	spin_lock(&wbuf->lock);
-	wbuf->offs += c->min_io_size;
+	wbuf->offs += wbuf->size;
 	wbuf->avail = c->min_io_size;
+	wbuf->size = c->min_io_size;
 	wbuf->used = 0;
 	wbuf->next_ino = 0;
 	spin_unlock(&wbuf->lock);
@@ -425,6 +429,7 @@ int ubifs_wbuf_seek_nolock(struct ubifs_wbuf *wbuf, int lnum, int offs,
 	wbuf->lnum = lnum;
 	wbuf->offs = offs;
 	wbuf->avail = c->min_io_size;
+	wbuf->size = c->min_io_size;
 	wbuf->used = 0;
 	spin_unlock(&wbuf->lock);
 	wbuf->dtype = dtype;
@@ -522,7 +527,10 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len)
 	ubifs_assert(len > 0 && wbuf->lnum >= 0 && wbuf->lnum < c->leb_cnt);
 	ubifs_assert(wbuf->offs >= 0 && wbuf->offs % c->min_io_size == 0);
 	ubifs_assert(!(wbuf->offs & 7) && wbuf->offs <= c->leb_size);
-	ubifs_assert(wbuf->avail > 0 && wbuf->avail <= c->min_io_size);
+	ubifs_assert(wbuf->avail > 0 && wbuf->avail <= wbuf->size);
+	ubifs_assert(wbuf->size >= c->min_io_size);
+	ubifs_assert(wbuf->size <= c->max_write_size);
+	ubifs_assert(wbuf->size % c->min_io_size == 0);
 	ubifs_assert(mutex_is_locked(&wbuf->io_mutex));
 	ubifs_assert(!c->ro_media && !c->ro_mount);
 
@@ -547,7 +555,7 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len)
 			dbg_io("flush jhead %s wbuf to LEB %d:%d",
 			       dbg_jhead(wbuf->jhead), wbuf->lnum, wbuf->offs);
 			err = ubi_leb_write(c->ubi, wbuf->lnum, wbuf->buf,
-					    wbuf->offs, c->min_io_size,
+					    wbuf->offs, wbuf->size,
 					    wbuf->dtype);
 			if (err)
 				goto out;
@@ -555,6 +563,7 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len)
 			spin_lock(&wbuf->lock);
 			wbuf->offs += c->min_io_size;
 			wbuf->avail = c->min_io_size;
+			wbuf->size = c->min_io_size;
 			wbuf->used = 0;
 			wbuf->next_ino = 0;
 			spin_unlock(&wbuf->lock);
@@ -577,11 +586,11 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len)
 	       dbg_jhead(wbuf->jhead), wbuf->lnum, wbuf->offs);
 	memcpy(wbuf->buf + wbuf->used, buf, wbuf->avail);
 	err = ubi_leb_write(c->ubi, wbuf->lnum, wbuf->buf, wbuf->offs,
-			    c->min_io_size, wbuf->dtype);
+			    wbuf->size, wbuf->dtype);
 	if (err)
 		goto out;
 
-	offs = wbuf->offs + c->min_io_size;
+	offs = wbuf->offs + wbuf->size;
 	len -= wbuf->avail;
 	aligned_len -= wbuf->avail;
 	written = wbuf->avail;
@@ -618,6 +627,7 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len)
 	wbuf->offs = offs;
 	wbuf->used = aligned_len;
 	wbuf->avail = c->min_io_size - aligned_len;
+	wbuf->size = c->min_io_size;
 	wbuf->next_ino = 0;
 	spin_unlock(&wbuf->lock);
 
@@ -855,7 +865,7 @@ int ubifs_wbuf_init(struct ubifs_info *c, struct ubifs_wbuf *wbuf)
 
 	wbuf->used = 0;
 	wbuf->lnum = wbuf->offs = -1;
-	wbuf->avail = c->min_io_size;
+	wbuf->avail = wbuf->size = c->min_io_size;
 	wbuf->dtype = UBI_UNKNOWN;
 	wbuf->sync_callback = NULL;
 	mutex_init(&wbuf->io_mutex);
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 8b51949..293ea97 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -646,6 +646,7 @@ typedef int (*ubifs_lpt_scan_callback)(struct ubifs_info *c,
  * @offs: write-buffer offset in this logical eraseblock
  * @avail: number of bytes available in the write-buffer
  * @used:  number of used bytes in the write-buffer
+ * @size: write-buffer size (in [@c->min_io_size, @c->max_write_size] range)
  * @dtype: type of data stored in this LEB (%UBI_LONGTERM, %UBI_SHORTTERM,
  * %UBI_UNKNOWN)
  * @jhead: journal head the mutex belongs to (note, needed only to shut lockdep
@@ -680,6 +681,7 @@ struct ubifs_wbuf {
 	int offs;
 	int avail;
 	int used;
+	int size;
 	int dtype;
 	int jhead;
 	int (*sync_callback)(struct ubifs_info *c, int lnum, int free, int pad);
-- 
1.7.2.3

[PATCH 4/7] UBIFS: use max_write_size for write-buffers

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

Switch write-buffers from 'c->min_io_size' to 'c->max_write_size' which
presumably has to be more write speed-efficient. However, when write-buffer
is synchronized, write only the the min. I/O units which contain the
data, do not write whole write-buffer. This is more space-efficient.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 fs/ubifs/io.c |   55 ++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index 7c2a014..37866b6 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -351,7 +351,7 @@ static void cancel_wbuf_timer_nolock(struct ubifs_wbuf *wbuf)
 int ubifs_wbuf_sync_nolock(struct ubifs_wbuf *wbuf)
 {
 	struct ubifs_info *c = wbuf->c;
-	int err, dirt;
+	int err, dirt, sync_len;
 
 	cancel_wbuf_timer_nolock(wbuf);
 	if (!wbuf->used || wbuf->lnum == -1)
@@ -370,22 +370,29 @@ int ubifs_wbuf_sync_nolock(struct ubifs_wbuf *wbuf)
 	if (c->ro_error)
 		return -EROFS;
 
-	ubifs_pad(c, wbuf->buf + wbuf->used, wbuf->avail);
+	/*
+	 * Do not write whole write buffer but write only the minimum necessary
+	 * amount of min. I/O units.
+	 */
+	sync_len = ALIGN(wbuf->used, c->min_io_size);
+	dirt = sync_len - wbuf->used;
+	if (dirt)
+		ubifs_pad(c, wbuf->buf + wbuf->used, dirt);
 	err = ubi_leb_write(c->ubi, wbuf->lnum, wbuf->buf, wbuf->offs,
-			    wbuf->size, wbuf->dtype);
+			    sync_len, wbuf->dtype);
 	if (err) {
 		ubifs_err("cannot write %d bytes to LEB %d:%d",
-			  wbuf->size, wbuf->lnum, wbuf->offs);
+			  sync_len, wbuf->lnum, wbuf->offs);
 		dbg_dump_stack();
 		return err;
 	}
 
-	dirt = wbuf->avail;
-
 	spin_lock(&wbuf->lock);
-	wbuf->offs += wbuf->size;
-	wbuf->avail = c->min_io_size;
-	wbuf->size = c->min_io_size;
+	wbuf->offs += sync_len;
+	wbuf->size = c->leb_size - wbuf->offs;
+	if (wbuf->size > c->max_write_size)
+		wbuf->size = c->max_write_size;
+	wbuf->avail = wbuf->size;
 	wbuf->used = 0;
 	wbuf->next_ino = 0;
 	spin_unlock(&wbuf->lock);
@@ -428,8 +435,10 @@ int ubifs_wbuf_seek_nolock(struct ubifs_wbuf *wbuf, int lnum, int offs,
 	spin_lock(&wbuf->lock);
 	wbuf->lnum = lnum;
 	wbuf->offs = offs;
-	wbuf->avail = c->min_io_size;
-	wbuf->size = c->min_io_size;
+	wbuf->size = c->leb_size - wbuf->offs;
+	if (wbuf->size > c->max_write_size)
+		wbuf->size = c->max_write_size;
+	wbuf->avail = wbuf->size;
 	wbuf->used = 0;
 	spin_unlock(&wbuf->lock);
 	wbuf->dtype = dtype;
@@ -561,9 +570,11 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len)
 				goto out;
 
 			spin_lock(&wbuf->lock);
-			wbuf->offs += c->min_io_size;
-			wbuf->avail = c->min_io_size;
-			wbuf->size = c->min_io_size;
+			wbuf->offs += wbuf->size;
+			wbuf->size = c->leb_size - wbuf->offs;
+			if (wbuf->size > c->max_write_size)
+				wbuf->size = c->max_write_size;
+			wbuf->avail = wbuf->size;
 			wbuf->used = 0;
 			wbuf->next_ino = 0;
 			spin_unlock(&wbuf->lock);
@@ -601,9 +612,9 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len)
 	 * We align node length to 8-byte boundary because we anyway flash wbuf
 	 * if the remaining space is less than 8 bytes.
 	 */
-	n = aligned_len >> c->min_io_shift;
+	n = aligned_len >> c->max_write_shift;
 	if (n) {
-		n <<= c->min_io_shift;
+		n <<= c->max_write_shift;
 		dbg_io("write %d bytes to LEB %d:%d", n, wbuf->lnum, offs);
 		err = ubi_leb_write(c->ubi, wbuf->lnum, buf + written, offs, n,
 				    wbuf->dtype);
@@ -626,8 +637,10 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len)
 
 	wbuf->offs = offs;
 	wbuf->used = aligned_len;
-	wbuf->avail = c->min_io_size - aligned_len;
-	wbuf->size = c->min_io_size;
+	wbuf->size = c->leb_size - wbuf->offs;
+	if (wbuf->size > c->max_write_size)
+		wbuf->size = c->max_write_size;
+	wbuf->avail = wbuf->size - aligned_len;
 	wbuf->next_ino = 0;
 	spin_unlock(&wbuf->lock);
 
@@ -851,11 +864,11 @@ int ubifs_wbuf_init(struct ubifs_info *c, struct ubifs_wbuf *wbuf)
 {
 	size_t size;
 
-	wbuf->buf = kmalloc(c->min_io_size, GFP_KERNEL);
+	wbuf->buf = kmalloc(c->max_write_size, GFP_KERNEL);
 	if (!wbuf->buf)
 		return -ENOMEM;
 
-	size = (c->min_io_size / UBIFS_CH_SZ + 1) * sizeof(ino_t);
+	size = (c->max_write_size / UBIFS_CH_SZ + 1) * sizeof(ino_t);
 	wbuf->inodes = kmalloc(size, GFP_KERNEL);
 	if (!wbuf->inodes) {
 		kfree(wbuf->buf);
@@ -865,7 +878,7 @@ int ubifs_wbuf_init(struct ubifs_info *c, struct ubifs_wbuf *wbuf)
 
 	wbuf->used = 0;
 	wbuf->lnum = wbuf->offs = -1;
-	wbuf->avail = wbuf->size = c->min_io_size;
+	wbuf->avail = wbuf->size = c->max_write_size;
 	wbuf->dtype = UBI_UNKNOWN;
 	wbuf->sync_callback = NULL;
 	mutex_init(&wbuf->io_mutex);
-- 
1.7.2.3

[PATCH 6/7] UBIFS: amend commentaries in io.c to match new situation

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

Now write-buffer size is 'c->max_write_size', and some comments
are out-of-date. This patch fixes them as well as adds few new
comments.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 fs/ubifs/io.c |   35 ++++++++++++++++++++++++++---------
 1 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index 37866b6..c122bc5 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -31,6 +31,16 @@
  * buffer is full or when it is not used for some time (by timer). This is
  * similar to the mechanism is used by JFFS2.
  *
+ * UBIFS distinguishes between minimum write size (@c->min_io_size) and maximum
+ * write size (@c->max_write_size). The latter is the maximum amount of bytes
+ * the underlying flash is able to program at a time, and writing in
+ * @c->max_write_size units should presumably be faster. Obviously,
+ * @c->min_io_size <= @c->max_write_size. Write-buffers are of
+ * @c->max_write_size bytes in size for maximum performance. However, when a
+ * write-buffer is flushed, only the portion of it (aligned to @c->min_io_size
+ * boundary) which contains data is written, not the whole write-buffer,
+ * because this is more space-efficient.
+ *
  * Write-buffers are defined by 'struct ubifs_wbuf' objects and protected by
  * mutexes defined inside these objects. Since sometimes upper-level code
  * has to lock the write-buffer (e.g. journal space reservation code), many
@@ -46,8 +56,8 @@
  * UBIFS uses padding when it pads to the next min. I/O unit. In this case it
  * uses padding nodes or padding bytes, if the padding node does not fit.
  *
- * All UBIFS nodes are protected by CRC checksums and UBIFS checks all nodes
- * every time they are read from the flash media.
+ * All UBIFS nodes are protected by CRC checksums and UBIFS checks CRC when
+ * they are read from the flash media.
  */
 
 #include <linux/crc32.h>
@@ -347,6 +357,12 @@ static void cancel_wbuf_timer_nolock(struct ubifs_wbuf *wbuf)
  *
  * This function synchronizes write-buffer @buf and returns zero in case of
  * success or a negative error code in case of failure.
+ *
+ * Note, although write-buffers are of @c->max_write_size, this function does
+ * not necessarily writes all @c->max_write_size bytes to the flash. Instead,
+ * if the write-buffer is only partially filled with data, only the used part
+ * of the write-buffer (aligned on @c->min_io_size boundary) is synchronized.
+ * This way we waste less space.
  */
 int ubifs_wbuf_sync_nolock(struct ubifs_wbuf *wbuf)
 {
@@ -518,8 +534,9 @@ out_timers:
  *
  * This function writes data to flash via write-buffer @wbuf. This means that
  * the last piece of the node won't reach the flash media immediately if it
- * does not take whole minimal I/O unit. Instead, the node will sit in RAM
- * until the write-buffer is synchronized (e.g., by timer).
+ * does not take whole max. write unit (@c->max_write_size). Instead, the node
+ * will sit in RAM until the write-buffer is synchronized (e.g., by timer, or
+ * because more data are appended to the write-buffer).
  *
  * This function returns zero in case of success and a negative error code in
  * case of failure. If the node cannot be written because there is no more
@@ -590,8 +607,8 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len)
 
 	/*
 	 * The node is large enough and does not fit entirely within current
-	 * minimal I/O unit. We have to fill and flush write-buffer and switch
-	 * to the next min. I/O unit.
+	 * available space. We have to fill and flush write-buffer and switch
+	 * to the next max. write unit.
 	 */
 	dbg_io("flush jhead %s wbuf to LEB %d:%d",
 	       dbg_jhead(wbuf->jhead), wbuf->lnum, wbuf->offs);
@@ -607,8 +624,8 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len)
 	written = wbuf->avail;
 
 	/*
-	 * The remaining data may take more whole min. I/O units, so write the
-	 * remains multiple to min. I/O unit size directly to the flash media.
+	 * The remaining data may take more whole max. write units, so write the
+	 * remains multiple to max. write unit size directly to the flash media.
 	 * We align node length to 8-byte boundary because we anyway flash wbuf
 	 * if the remaining space is less than 8 bytes.
 	 */
@@ -630,7 +647,7 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len)
 	if (aligned_len)
 		/*
 		 * And now we have what's left and what does not take whole
-		 * min. I/O unit, so write it to the write-buffer and we are
+		 * max. write unit, so write it to the write-buffer and we are
 		 * done.
 		 */
 		memcpy(wbuf->buf, buf + written, len);
-- 
1.7.2.3

[PATCH 7/7] UBIFS: use max_write_size during recovery

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

When recovering from unclean reboots UBIFS scans the journal and checks nodes.
If a corrupted node is found, UBIFS tries to check if this is the last node
in the LEB or not. This is is done by checking if there only 0xFF bytes
starting from the next min. I/O unit. However, since now we write in
c->max_write_size, we should actually check for 0xFFs starting from the
next max. write unit.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 fs/ubifs/recovery.c |   17 +++++++----------
 1 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/fs/ubifs/recovery.c b/fs/ubifs/recovery.c
index 77e9b87..4278ae7 100644
--- a/fs/ubifs/recovery.c
+++ b/fs/ubifs/recovery.c
@@ -362,8 +362,9 @@ int ubifs_write_rcvrd_mst_node(struct ubifs_info *c)
  * @offs: offset to check
  *
  * This function returns %1 if @offs was in the last write to the LEB whose data
- * is in @buf, otherwise %0 is returned.  The determination is made by checking
- * for subsequent empty space starting from the next @c->min_io_size boundary.
+ * is in @buf, otherwise %0 is returned. The determination is made by checking
+ * for subsequent empty space starting from the next @c->max_write_size
+ * boundary.
  */
 static int is_last_write(const struct ubifs_info *c, void *buf, int offs)
 {
@@ -371,10 +372,10 @@ static int is_last_write(const struct ubifs_info *c, void *buf, int offs)
 	uint8_t *p;
 
 	/*
-	 * Round up to the next @c->min_io_size boundary i.e. @offs is in the
-	 * last wbuf written. After that should be empty space.
+	 * Round up to the next @c->max_write_size boundary i.e. @offs is in
+	 * the last wbuf written. After that should be empty space.
 	 */
-	empty_offs = ALIGN(offs + 1, c->min_io_size);
+	empty_offs = ALIGN(offs + 1, c->max_write_size);
 	check_len = c->leb_size - empty_offs;
 	p = buf + empty_offs - offs;
 	return is_empty(p, check_len);
@@ -836,12 +837,8 @@ struct ubifs_scan_leb *ubifs_recover_log_leb(struct ubifs_info *c, int lnum,
 static int recover_head(const struct ubifs_info *c, int lnum, int offs,
 			void *sbuf)
 {
-	int len, err;
+	int len = c->max_write_size, err;
 
-	if (c->min_io_size > 1)
-		len = c->min_io_size;
-	else
-		len = 512;
 	if (offs + len > c->leb_size)
 		len = c->leb_size - offs;
 
-- 
1.7.2.3

Re: [PATCH 0/7] UBIFS: fix recovery on CFI NOR

[Anatolij Gustschin]

Hi,

On Wed,  2 Feb 2011 10:21:51 +0200
Artem Bityutskiy <dedekind1@gmail.com> wrote:

> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> 
> Hi,
> 
> here is the patch-set against the latest Linux kernel (e.g. 2.6.38-rc3)
> which should fix CFI NOR flash recovery.

Great! Thanks!

> The previous attempt was not entirely successful - it broke backward
> compatibility and was reverted:
> 
> http://marc.info/?l=linux-kernel&m=129631939419818&w=2
> 
> This patch-set goes the following way:
> 1. Incorporates the notion of 'writebufsize' into UBI and UBIFS as
>    'max_write_size', because using term write-buffer would be confusing, as
>    UBIFS has its own write-buffers.
> 2. Changes UBIFS write-buffer and makes it of 'max_write_size', instead of
>    'min_io_size'. This presumably leads to better performance because we
>    accumulate more data and write them in larger chunks and faster.
>    And we do not waste space when synchronizing UBIFS write-buffers because we
>    write only the used amount of bytes aligned to 'min_io_size'. So this is an
>    improvement.
> 3. Tweak UBIFS recovery and make it aware of the fact that we can write in
>    chunks larger than 'min_io_size'. Namely, we can write in 'max_write_size'
>    chunks.
> 
> Could you guys please test this WRT power cuts and let me know if it solves the
> issues?

I'm going to test these patches next days. When using write buffer
size as min_io_size for UBI we still have some issues with UBIFS
recovery while running power cut tests on S29GL512P NOR flash.
Mounting an UBIFS partition fails as there are CRC errors in the
UBIFS data node. A few data bytes following the UBIFS data node
header are corrupted. Sometimes the data node header itself is
corrupted:
...
UBIFS DBG (pid 1416): ubifs_scan_a_node: scanning data node
UBIFS DBG (pid 1416): ubifs_recover_leb: look at LEB 139:111944 (150072 bytes left)
UBIFS DBG (pid 1416): ubifs_scan_a_node: scanning data node
UBIFS DBG (pid 1416): ubifs_recover_leb: look at LEB 139:116088 (145928 bytes left)
UBIFS DBG (pid 1416): ubifs_scan_a_node: scanning unknown node
UBIFS DBG (pid 1416): no_more_nodes: unexpected bad common header at 139:116088
UBIFS DBG (pid 1416): ubifs_recover_leb: look at LEB 139:116088 (145928 bytes left)
UBIFS DBG (pid 1416): ubifs_scan_a_node: scanning unknown node
UBIFS error (pid 1416): ubifs_check_node: bad node type 255
UBIFS error (pid 1416): ubifs_check_node: bad node at LEB 139:116088
        magic          0x6101831
        crc            0x46a2df3b
        node_type      255 (unknown node)
        group_type     255 (unknown)
        sqnum          18446744073709551615
        len            4294967295
node type 255 was not recognized
Call Trace:
[df7f3b80] [c0007ecc] show_stack+0x4c/0x16c (unreliable)
[df7f3bc0] [c013e4c8] ubifs_check_node+0x17c/0x308
[df7f3be0] [c0147934] ubifs_scan_a_node+0x15c/0x2d8
[df7f3c10] [c015cd08] ubifs_recover_leb+0x3f0/0x940
[df7f3c80] [c0148244] replay_buds+0xb4/0xb4c
[df7f3d20] [c01493dc] ubifs_replay_journal+0x700/0xf48
[df7f3da0] [c013aba8] ubifs_fill_super+0xd9c/0x15fc
[df7f3e00] [c013c6fc] ubifs_get_sb+0x10c/0x344
[df7f3e80] [c007b4d0] vfs_kern_mount+0x60/0x150
[df7f3ea0] [c007b610] do_kern_mount+0x40/0x100
[df7f3ec0] [c0092210] do_mount+0x40c/0x718
[df7f3f10] [c00925ac] sys_mount+0x90/0xd8
[df7f3f40] [c0010b44] ret_from_syscall+0x0/0x38
--- Exception: c01 at 0xfe93d18
    LR = 0x1000347c
UBIFS DBG (pid 1416): no_more_nodes: unexpected bad common header at 139:116088
UBIFS error (pid 1416): ubifs_recover_leb: bad node
UBIFS error (pid 1416): ubifs_scanned_corruption: corruption at LEB 139:116088
UBIFS error (pid 1416): ubifs_scanned_corruption: first 8192 bytes from LEB 139:116088
00000000: 31181006 3bdfa246 ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff  1...;..F........................
00000020: ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff  ................................
00000040: ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff  ................................
00000060: ffffffff ffffffff 4647fdeb ee4bdded 4e4f50d1 5253f675 56577d59 5a5b7d5f  ........FG...K..NOP.RS.uVW}YZ[}_
00000080: 5e5f6061 62636465 ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff  ^_`abcde........................
...

I'm still looking for a reason for this behaviour and can't explain
it yet. Maybe we have a problem in the CFI driver.

Thanks,
Anatolij

--
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de

Re: [PATCH 0/7] UBIFS: fix recovery on CFI NOR

[Holger Brunck]

Hi Artem,

Artem Bityutskiy wrote:
> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> 
> Hi,
> 
> here is the patch-set against the latest Linux kernel (e.g. 2.6.38-rc3)
> which should fix CFI NOR flash recovery.
> 
> The previous attempt was not entirely successful - it broke backward
> compatibility and was reverted:
> 
> http://marc.info/?l=linux-kernel&m=129631939419818&w=2
> 
> This patch-set goes the following way:
> 1. Incorporates the notion of 'writebufsize' into UBI and UBIFS as
>    'max_write_size', because using term write-buffer would be confusing, as
>    UBIFS has its own write-buffers.
> 2. Changes UBIFS write-buffer and makes it of 'max_write_size', instead of
>    'min_io_size'. This presumably leads to better performance because we
>    accumulate more data and write them in larger chunks and faster.
>    And we do not waste space when synchronizing UBIFS write-buffers because we
>    write only the used amount of bytes aligned to 'min_io_size'. So this is an
>    improvement.
> 3. Tweak UBIFS recovery and make it aware of the fact that we can write in
>    chunks larger than 'min_io_size'. Namely, we can write in 'max_write_size'
>    chunks.
> 
> Could you guys please test this WRT power cuts and let me know if it solves the
> issues?
> 

I have tested this patches on an ppc82xx and ppc83xx boards with different NOR
flashes with different writebuffers (64 and 1024 bytes) and check wether I am
able to mount previous created UBIFS partitions and this works without any
problems. So the incompatbility seems to be solved. Additionaly I tried it on a
NAND based system and this runs also without problems.

But I am not able to check for power cut problems. We see some problems in the
combination with UBI and NOR flashes with large writebuffers (1024). But
currently we suspect some CFI driver problems see ML entry from an colleague:
http://lists.infradead.org/pipermail/linux-mtd/2011-February/033849.html

Another question related to the writebuffer adaptions for UBI. What should be
done during creation of ubi images on a host system with ubinize if your patches
find their way in the "standard" UBI/UBIFS code. In the past we had "only" NOR
flashes with a writebuffer of 64 bytes and we create our ubi images without the
-m parameter during executing ubinize for the esw image. Now we got a new flash
with writebuffer = 1024. Whats the way forward in the future? Is it ok to omit
the "-m" parameter or do we have to create the images with "-m 64" or "-m 1024"?

Best regards
Holger

Re: [PATCH 0/7] UBIFS: fix recovery on CFI NOR

[Artem Bityutskiy]

Hi,

On Wed, 2011-02-02 at 16:21 +0100, Holger Brunck wrote:
> I have tested this patches on an ppc82xx and ppc83xx boards with different NOR
> flashes with different writebuffers (64 and 1024 bytes) and check wether I am
> able to mount previous created UBIFS partitions and this works without any
> problems. So the incompatbility seems to be solved. Additionaly I tried it on a
> NAND based system and this runs also without problems.

OK, thanks!

> Another question related to the writebuffer adaptions for UBI. What should be
> done during creation of ubi images on a host system with ubinize if your patches
> find their way in the "standard" UBI/UBIFS code. 

Nothing, when creating images you specify min. I/O size, which is 1 in
case of NOR.

> In the past we had "only" NOR
> flashes with a writebuffer of 64 bytes and we create our ubi images without the
> -m parameter during executing ubinize for the esw image.

No, you always specify 1. Your flash still allows writing 1 byte at a
time, and this is the minimum, so you set -m 1.

64 is the internal detail, the "optimal" write size. UBIFS will
automatically pick it up and will try to write in 64-byte chunks at a
time, but not always, only when it is possible.

>  Now we got a new flash
> with writebuffer = 1024. Whats the way forward in the future? Is it ok to omit
> the "-m" parameter or do we have to create the images with "-m 64" or "-m 1024"?

Similarly, just use -m 1

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH 0/7] UBIFS: fix recovery on CFI NOR

[Holger Brunck]

Hi,

On 02/02/2011 06:16 PM, Artem Bityutskiy wrote:
> Hi,
> 
> On Wed, 2011-02-02 at 16:21 +0100, Holger Brunck wrote:
>> I have tested this patches on an ppc82xx and ppc83xx boards with different NOR
>> flashes with different writebuffers (64 and 1024 bytes) and check wether I am
>> able to mount previous created UBIFS partitions and this works without any
>> problems. So the incompatbility seems to be solved. Additionaly I tried it on a
>> NAND based system and this runs also without problems.
> 
> OK, thanks!
> 
>> Another question related to the writebuffer adaptions for UBI. What should be
>> done during creation of ubi images on a host system with ubinize if your patches
>> find their way in the "standard" UBI/UBIFS code. 
> 
> Nothing, when creating images you specify min. I/O size, which is 1 in
> case of NOR.
> 
>> In the past we had "only" NOR
>> flashes with a writebuffer of 64 bytes and we create our ubi images without the
>> -m parameter during executing ubinize for the esw image.
> 
> No, you always specify 1. Your flash still allows writing 1 byte at a
> time, and this is the minimum, so you set -m 1.
> 
> 64 is the internal detail, the "optimal" write size. UBIFS will
> automatically pick it up and will try to write in 64-byte chunks at a
> time, but not always, only when it is possible.
> 
>>  Now we got a new flash
>> with writebuffer = 1024. Whats the way forward in the future? Is it ok to omit
>> the "-m" parameter or do we have to create the images with "-m 64" or "-m 1024"?
> 
> Similarly, just use -m 1
> 

ah ok. This was exactly what I figure out from the UBI/UBIFS documentation, but
due to the discussions and patches for the min I/O sizes adaptions I was a
little bit unsure what to do. Thanks for the clarification.

Regards
Holger