[PATCH 0/5] Add flight recorder to MTDRAM

[PATCH 0/5] Add flight recorder to MTDRAM

[Dirk Behme]

From: Manfred Spraul <manfred@colorfullife.com>

Hi,

The series adds a flight recorder to MTDRAM.
This allows very efficient power fail testing:
>From the flight recorder output, it is possible to recreate every image
that might have existed between the start of the recording and the end.

Obviously, a user space tool is required, it is attached as the last
mail in the series.

Patches:

0001-mtdram-expose-write-size-and-writebuf-size-as-module:
	An initial cleanup: write_size and writebuf_size are
	hardcoded in the source code.
	Convert that to module parameters.

0002-mtdram-Add-flight-recorder.patch:
	Initial flight recorder

0003-mtdram-Allow-to-enable-disable-flight-recorder-mode-.patch:
	For the preparation step, or for evaluating dumps, the
	flight recorder doesn't make sense. Thus allow
	to disable it at runtime by writing to debugfs/mtdram

0004-mtdram-Convert-the-flight-recorder-to-a-ring-buffer.patch:
	The initial flight recorder is very simple, cleanup 1:
	Convert the kernel buffer to a proper ringbuffer.

0005-mtdram-flight-recorder-Add-checksums.patch:
	When using tool to simulate something, there is always the
	risk that the issue is in the tool and not in the production
	code. Thus add checksums, to detect tool issues.

--
	Manfred

[PATCH 1/5] mtdram: expose write size and writebuf size as module parameters

[Dirk Behme]

From: Manfred Spraul <manfred@colorfullife.com>

Right now, mtdram reports itself as a device with a 1 byte write
size, and the writebuf size can only be changed at compile time.

The patch:
- allows to change the write size, both at compile and at
  module load time
- allows to change the writebuf size at module load time.

Increasing the write size e.g. speeds up nandwrite significantly,
and it allows to test how UBI behaves with/without subpage writes.

Signed-off-by: Manfred Spraul <manfred.spraul@de.bosch.com>
Cc: Manfred Spraul <manfred@colorfullife.com>
---
 drivers/mtd/devices/Kconfig  | 19 +++++++++++++++++++
 drivers/mtd/devices/mtdram.c |  7 +++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/devices/Kconfig b/drivers/mtd/devices/Kconfig
index 6def5445e03e..d8b67ba0b5de 100644
--- a/drivers/mtd/devices/Kconfig
+++ b/drivers/mtd/devices/Kconfig
@@ -181,6 +181,25 @@ config MTDRAM_ERASE_SIZE
 	  as a module, it is also possible to specify this as a parameter when
 	  loading the module.
 
+config MTDRAM_WRITE_SIZE
+	int "MTDRAM write size in bytes"
+	depends on MTD_MTDRAM
+	default "1"
+	help
+	  This allows you to configure the minimum write size in the device
+	  emulated by the MTDRAM driver.  If the MTDRAM driver is built
+	  as a module, it is also possible to specify this as a parameter when
+	  loading the module. Common values are 1 (NOR), 512 (NAND with sub-
+	  page writes) or 2048 (NAND without sub-page writes).
+
+config MTDRAM_WRITEBUF_SIZE
+	int "MTDRAM writebuf size in bytes"
+	depends on MTD_MTDRAM
+	default "64"
+	help
+	  This allows you to specify the writebuf size that is reported
+	  by the device emulated by the MTDRAM driver.
+
 config MTD_BLOCK2MTD
 	tristate "MTD using block device"
 	depends on BLOCK
diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
index 0bf4aeaf0cb8..0c8652ac0395 100644
--- a/drivers/mtd/devices/mtdram.c
+++ b/drivers/mtd/devices/mtdram.c
@@ -20,7 +20,8 @@
 
 static unsigned long total_size = CONFIG_MTDRAM_TOTAL_SIZE;
 static unsigned long erase_size = CONFIG_MTDRAM_ERASE_SIZE;
-static unsigned long writebuf_size = 64;
+static unsigned long writebuf_size = CONFIG_MTDRAM_WRITEBUF_SIZE;
+static unsigned long write_size = CONFIG_MTDRAM_WRITE_SIZE;
 #define MTDRAM_TOTAL_SIZE (total_size * 1024)
 #define MTDRAM_ERASE_SIZE (erase_size * 1024)
 
@@ -31,6 +32,8 @@ module_param(erase_size, ulong, 0);
 MODULE_PARM_DESC(erase_size, "Device erase block size in KiB");
 module_param(writebuf_size, ulong, 0);
 MODULE_PARM_DESC(writebuf_size, "Device write buf size in Bytes (Default: 64)");
+module_param(write_size, ulong, 0);
+MODULE_PARM_DESC(write_size, "Device write size in Bytes (Default: 1)");
 #endif
 
 // We could store these in the mtd structure, but we only support 1 device..
@@ -134,7 +137,7 @@ int mtdram_init_device(struct mtd_info *mtd, void *mapped_address,
 	mtd->type = MTD_RAM;
 	mtd->flags = MTD_CAP_RAM;
 	mtd->size = size;
-	mtd->writesize = 1;
+	mtd->writesize = write_size;
 	mtd->writebufsize = writebuf_size;
 	mtd->erasesize = MTDRAM_ERASE_SIZE;
 	mtd->priv = mapped_address;
-- 
2.14.1

[PATCH 2/5] mtdram: Add flight recorder

[Dirk Behme]

From: Manfred Spraul <manfred@colorfullife.com>

The patch adds the option to enable a flight recorder for the
mtdram test device: All ERASE and WRITE commands are logged, e.g.
to perform targeted power fail testing.

Signed-off-by: Manfred Spraul <manfred.spraul@de.bosch.com>
Cc: Manfred Spraul <manfred@colorfullife.com>
---
 drivers/mtd/devices/Kconfig  |  25 +++++
 drivers/mtd/devices/mtdram.c | 239 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 252 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/devices/Kconfig b/drivers/mtd/devices/Kconfig
index d8b67ba0b5de..8647214089c9 100644
--- a/drivers/mtd/devices/Kconfig
+++ b/drivers/mtd/devices/Kconfig
@@ -200,6 +200,31 @@ config MTDRAM_WRITEBUF_SIZE
 	  This allows you to specify the writebuf size that is reported
 	  by the device emulated by the MTDRAM driver.
 
+config MTDRAM_FLIGHTRECORDER
+	bool "Flight recorder for MTDRAM test device"
+	depends on (MTD_MTDRAM && DEBUG_FS)
+	default n
+	help
+	  This allows you to enable flight recorder mode for the MTDRAM test
+	  device: All ERASE and WRITE commands are logged and exposed in
+	  debugfs.
+
+config MTDRAM_FLIGHTRECORDER_BUFFER_SIZE
+	int "MTDRAM flightrecorder buffer size in KiB"
+	depends on MTDRAM_FLIGHTRECORDER
+	default "131072"
+	help
+	  This allows you to set the size of the flight recorder buffer for
+	  the MTDRAM test device.
+
+config MTDRAM_FLIGHTRECORDER_ENABLED
+	bool "Flight recorder for MTDRAM test device"
+	default y
+	depends on MTDRAM_FLIGHTRECORDER
+	help
+	  This flag sets the initial setting of the flight recorder. If enabled,
+	  the recording starts immediate at module start.
+
 config MTD_BLOCK2MTD
 	tristate "MTD using block device"
 	depends on BLOCK
diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
index 0c8652ac0395..1dc0a5ce0f07 100644
--- a/drivers/mtd/devices/mtdram.c
+++ b/drivers/mtd/devices/mtdram.c
@@ -18,6 +18,190 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/mtdram.h>
 
+#ifdef CONFIG_MTDRAM_FLIGHTRECORDER
+
+#include <linux/debugfs.h>
+#include <linux/sched.h>
+
+static unsigned long fr_buffersize = CONFIG_MTDRAM_FLIGHTRECORDER_BUFFER_SIZE;
+
+#define FR_BUFFER_TOTAL_SIZE	(fr_buffersize * 1024)
+#define FR_BUFFER_MARGIN	512
+
+static bool fr_enabled = CONFIG_MTDRAM_FLIGHTRECORDER_ENABLED;
+
+#ifdef MODULE
+module_param(fr_buffersize, ulong, 0);
+MODULE_PARM_DESC(fr_buffersize, "Flight recorder buffer size KiB");
+module_param(fr_enabled, bool, 0);
+MODULE_PARM_DESC(fr_enabled, "Set the initial enabled/disabled status");
+#endif
+
+static char *fr_buffer;
+static int fr_pos;
+static struct dentry *fr_dentry;
+
+static DEFINE_MUTEX(fr_mutex);
+static DECLARE_WAIT_QUEUE_HEAD(fr_wait);
+
+static ssize_t nandrec_read_file(struct file *file, char __user *user_buf,
+				 size_t count, loff_t *ppos)
+{
+	ssize_t r;
+
+	mutex_lock(&fr_mutex);
+
+	/* Every read must read all available data */
+	if (count < fr_pos) {
+		r = -EINVAL;
+		goto out_unlock;
+	}
+
+	if (fr_pos == 0) {
+		r = 0;
+		goto out_unlock;
+	}
+
+	*ppos = 0;
+
+	r = debugfs_file_get(fr_dentry);
+	count = fr_pos;
+	if (likely(!r)) {
+		r = simple_read_from_buffer(user_buf, count, ppos, fr_buffer,
+					    FR_BUFFER_TOTAL_SIZE);
+		debugfs_file_put(fr_dentry);
+	}
+
+	/* Every read must read all available data */
+	WARN_ON(fr_pos != r);
+
+	/* Every read clears the kernel buffer */
+	fr_pos = 0;
+
+	/* if someone waits, wake him up */
+	if (waitqueue_active(&fr_wait))
+		wake_up(&fr_wait);
+
+out_unlock:
+	mutex_unlock(&fr_mutex);
+	return r;
+}
+
+static const struct file_operations fr_fops = {
+	.read =		nandrec_read_file,
+	.open =		simple_open,
+};
+
+#define MAGIC_START	0x12345678UL
+#define MAGIC_END	0x87654321UL
+
+#define FUNC_WRITE	1UL
+#define FUNC_ERASE	2UL
+
+static void write_u32(u32 data)
+{
+	u32 *target = (u32 *)(fr_buffer + fr_pos);
+
+	if (fr_enabled) {
+		*target = cpu_to_le32(data);
+		fr_pos += round_up(sizeof(u32), 8);
+	}
+}
+
+static void write_u64(u64 data)
+{
+	u64 *target = (u64 *)(fr_buffer + fr_pos);
+
+	if (fr_enabled) {
+		*target = cpu_to_le64(data);
+		fr_pos += round_up(sizeof(u64), 8);
+	}
+}
+
+static void write_blob(const u_char *data, int len)
+{
+	u32 *target = (u32 *)(fr_buffer + fr_pos);
+
+	if (fr_enabled) {
+		memcpy(target, data, len);
+		fr_pos += round_up(len, 8);
+	}
+}
+
+static void start_write(int size)
+{
+	mutex_lock(&fr_mutex);
+
+	if (fr_enabled) {
+		while (fr_pos + size + 2*8 >=
+				FR_BUFFER_TOTAL_SIZE - FR_BUFFER_MARGIN) {
+			DEFINE_WAIT(wait);
+
+			pr_info("%p: Waiting - write count %d, current %d.\n",
+				current, size, fr_pos);
+
+			prepare_to_wait(&fr_wait, &wait, TASK_UNINTERRUPTIBLE);
+
+			mutex_unlock(&fr_mutex);
+
+			schedule();
+			mutex_lock(&fr_mutex);
+			finish_wait(&fr_wait, &wait);
+		}
+		write_u32(MAGIC_START);
+	}
+}
+
+static void end_write(void)
+{
+	if (fr_enabled) {
+		write_u32(MAGIC_END);
+		WARN_ON(fr_pos > FR_BUFFER_TOTAL_SIZE - FR_BUFFER_MARGIN);
+	}
+	mutex_unlock(&fr_mutex);
+}
+
+static int fr_init(void)
+{
+	fr_buffer = vmalloc(FR_BUFFER_TOTAL_SIZE);
+	if (!fr_buffer)
+		return -ENOMEM;
+
+	memset(fr_buffer, 0xfa, FR_BUFFER_TOTAL_SIZE);
+
+	fr_dentry = debugfs_create_file("mtdram", 0600, NULL, NULL,
+					&fr_fops);
+
+	if (IS_ERR(fr_dentry)) {
+		vfree(fr_buffer);
+		fr_buffer = NULL;
+		return -ENOMEM;
+	}
+	return 0;
+}
+
+static void fr_exit(void)
+{
+	if (fr_buffer) {
+		debugfs_remove(fr_dentry);
+		vfree(fr_buffer);
+		fr_buffer = NULL;
+	}
+}
+
+#else /* CONFIG_MTDRAM_FLIGHTRECORDER */
+
+static int fr_init(void)
+{
+	return 0;
+}
+
+static void fr_exit(void)
+{
+}
+
+#endif /* CONFIG_MTDRAM_FLIGHTRECORDER */
+
 static unsigned long total_size = CONFIG_MTDRAM_TOTAL_SIZE;
 static unsigned long erase_size = CONFIG_MTDRAM_ERASE_SIZE;
 static unsigned long writebuf_size = CONFIG_MTDRAM_WRITEBUF_SIZE;
@@ -63,6 +247,15 @@ static int ram_erase(struct mtd_info *mtd, struct erase_info *instr)
 	if (check_offs_len(mtd, instr->addr, instr->len))
 		return -EINVAL;
 	memset((char *)mtd->priv + instr->addr, 0xff, instr->len);
+
+#ifdef CONFIG_MTDRAM_FLIGHTRECORDER
+	start_write(3*8);
+	write_u32(FUNC_ERASE);
+	write_u64(instr->addr);
+	write_u64(instr->len);
+	end_write();
+#endif
+
 	instr->state = MTD_ERASE_DONE;
 	mtd_erase_callback(instr);
 	return 0;
@@ -114,12 +307,23 @@ static int ram_write(struct mtd_info *mtd, loff_t to, size_t len,
 		size_t *retlen, const u_char *buf)
 {
 	memcpy((char *)mtd->priv + to, buf, len);
+
+#ifdef CONFIG_MTDRAM_FLIGHTRECORDER
+	start_write(3*8 + len);
+	write_u32(FUNC_WRITE);
+	write_u64(to);
+	write_u64(len);
+	write_blob(buf, len);
+	end_write();
+#endif
+
 	*retlen = len;
 	return 0;
 }
 
 static void __exit cleanup_mtdram(void)
 {
+	fr_exit();
 	if (mtd_info) {
 		mtd_device_unregister(mtd_info);
 		vfree(mtd_info->priv);
@@ -163,25 +367,36 @@ static int __init init_mtdram(void)
 	if (!total_size)
 		return -EINVAL;
 
+	err = -ENOMEM;
+
 	/* Allocate some memory */
 	mtd_info = kmalloc(sizeof(struct mtd_info), GFP_KERNEL);
 	if (!mtd_info)
-		return -ENOMEM;
+		goto fail_mtdinfo;
 
 	addr = vmalloc(MTDRAM_TOTAL_SIZE);
-	if (!addr) {
-		kfree(mtd_info);
-		mtd_info = NULL;
-		return -ENOMEM;
-	}
+	if (!addr)
+		goto fail_ramdisk;
+
 	err = mtdram_init_device(mtd_info, addr, MTDRAM_TOTAL_SIZE, "mtdram test device");
-	if (err) {
-		vfree(addr);
-		kfree(mtd_info);
-		mtd_info = NULL;
-		return err;
-	}
+	if (err)
+		goto fail_init_device;
+
 	memset(mtd_info->priv, 0xff, MTDRAM_TOTAL_SIZE);
+	err = fr_init();
+	if (err)
+		goto fail_fr_init;
+
+	return 0;
+
+fail_fr_init:
+	mtd_device_unregister(mtd_info);
+fail_init_device:
+	vfree(addr);
+fail_ramdisk:
+	kfree(mtd_info);
+	mtd_info = NULL;
+fail_mtdinfo:
 	return err;
 }
 
-- 
2.14.1

[PATCH 3/5] mtdram: Allow to enable/disable flight recorder mode at runtime.

[Dirk Behme]

From: Manfred Spraul <manfred@colorfullife.com>

The patch allows to enable/disable the flight recorder mode by
writing to the debugfs file.

Signed-off-by: Manfred Spraul <manfred.spraul@de.bosch.com>
Cc: Manfred Spraul <manfred@colorfullife.com>
---
 drivers/mtd/devices/mtdram.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
index 1dc0a5ce0f07..a6f5a656eb94 100644
--- a/drivers/mtd/devices/mtdram.c
+++ b/drivers/mtd/devices/mtdram.c
@@ -87,8 +87,45 @@ static ssize_t nandrec_read_file(struct file *file, char __user *user_buf,
 	return r;
 }
 
+static ssize_t nandrec_write_file(struct file *file, const char __user *buf,
+				  size_t count, loff_t *ppos)
+{
+	ssize_t r;
+	char kbuf[8];
+
+	mutex_lock(&fr_mutex);
+
+	*ppos = 0;
+	memset(kbuf, 0, sizeof(kbuf));
+
+	r = debugfs_file_get(fr_dentry);
+	if (likely(!r)) {
+		r = simple_write_to_buffer(kbuf, sizeof(kbuf) - 1,
+					   ppos, buf, count);
+		debugfs_file_put(fr_dentry);
+	}
+
+	if (r > 0) {
+		unsigned long newval;
+		int ret;
+
+		ret = kstrtoul(kbuf, 0, &newval);
+		if (ret == 0)
+			fr_enabled = newval;
+		else
+			r = ret;
+	}
+	if (fr_enabled)
+		pr_info("MTDRAM flight recorder enabled.\n");
+	else
+		pr_info("MTDRAM flight recorder disabled.\n");
+	mutex_unlock(&fr_mutex);
+	return r;
+}
+
 static const struct file_operations fr_fops = {
 	.read =		nandrec_read_file,
+	.write =	nandrec_write_file,
 	.open =		simple_open,
 };
 
-- 
2.14.1

[PATCH 4/5] mtdram: Convert the flight recorder to a ring buffer.

[Dirk Behme]

From: Manfred Spraul <manfred@colorfullife.com>

In flight recorder mode, all erase and write operations are logged.
The patch converts the internal buffer to a ring buffer, i.e.
it is not necessary anymore to read the complete buffer in one
syscall.

Signed-off-by: Manfred Spraul <manfred.spraul@de.bosch.com>
Cc: Manfred Spraul <manfred@colorfullife.com>
---
 drivers/mtd/devices/mtdram.c | 119 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 90 insertions(+), 29 deletions(-)

diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
index a6f5a656eb94..202696bc92ef 100644
--- a/drivers/mtd/devices/mtdram.c
+++ b/drivers/mtd/devices/mtdram.c
@@ -38,45 +38,101 @@ MODULE_PARM_DESC(fr_enabled, "Set the initial enabled/disabled status");
 #endif
 
 static char *fr_buffer;
-static int fr_pos;
+static int fr_head;
+static int fr_tail;
 static struct dentry *fr_dentry;
 
 static DEFINE_MUTEX(fr_mutex);
 static DECLARE_WAIT_QUEUE_HEAD(fr_wait);
 
+static int nandrec_available_read(void)
+{
+	if (fr_head >= fr_tail)
+		return fr_head - fr_tail;
+
+	return fr_head + FR_BUFFER_TOTAL_SIZE - fr_tail;
+}
+
+static int nandrec_available_write(void)
+{
+	return FR_BUFFER_TOTAL_SIZE
+		- nandrec_available_read() - FR_BUFFER_MARGIN;
+}
+
+static void nandrec_advance_head(int count)
+{
+	WARN_ON(count > nandrec_available_write());
+
+	fr_head += count;
+	if (fr_head > FR_BUFFER_TOTAL_SIZE)
+		fr_head -= FR_BUFFER_TOTAL_SIZE;
+}
+
+static void nandrec_advance_tail(int count)
+{
+	WARN_ON(count > nandrec_available_read());
+
+	fr_tail += count;
+	if (fr_tail > FR_BUFFER_TOTAL_SIZE)
+		fr_tail -= FR_BUFFER_TOTAL_SIZE;
+
+	if (fr_tail == fr_head) {
+		fr_tail = 0;
+		fr_head = 0;
+	}
+}
+
 static ssize_t nandrec_read_file(struct file *file, char __user *user_buf,
 				 size_t count, loff_t *ppos)
 {
 	ssize_t r;
+	int max_data;
 
 	mutex_lock(&fr_mutex);
 
-	/* Every read must read all available data */
-	if (count < fr_pos) {
-		r = -EINVAL;
-		goto out_unlock;
-	}
+	max_data = nandrec_available_read();
 
-	if (fr_pos == 0) {
+	if (max_data == 0) {
 		r = 0;
 		goto out_unlock;
 	}
 
+	if (count > max_data)
+		count = max_data;
+
 	*ppos = 0;
 
 	r = debugfs_file_get(fr_dentry);
-	count = fr_pos;
 	if (likely(!r)) {
-		r = simple_read_from_buffer(user_buf, count, ppos, fr_buffer,
-					    FR_BUFFER_TOTAL_SIZE);
+		loff_t rpos;
+
+		rpos = fr_tail;
+		if (fr_tail + count > FR_BUFFER_TOTAL_SIZE) {
+			ssize_t tmp;
+
+			tmp = FR_BUFFER_TOTAL_SIZE - fr_tail;
+			r = simple_read_from_buffer(user_buf, tmp,
+						    &rpos, fr_buffer,
+						    FR_BUFFER_TOTAL_SIZE);
+			if (r == tmp) {
+				rpos = 0;
+				tmp = simple_read_from_buffer(user_buf + tmp,
+							count - tmp,
+							&rpos, fr_buffer,
+							FR_BUFFER_TOTAL_SIZE);
+				if (tmp > 0)
+					r += tmp;
+			}
+		} else {
+			r = simple_read_from_buffer(user_buf, count,
+						    &rpos, fr_buffer,
+						    FR_BUFFER_TOTAL_SIZE);
+		}
 		debugfs_file_put(fr_dentry);
 	}
 
-	/* Every read must read all available data */
-	WARN_ON(fr_pos != r);
-
-	/* Every read clears the kernel buffer */
-	fr_pos = 0;
+	if (r > 0)
+		nandrec_advance_tail(r);
 
 	/* if someone waits, wake him up */
 	if (waitqueue_active(&fr_wait))
@@ -137,31 +193,39 @@ static const struct file_operations fr_fops = {
 
 static void write_u32(u32 data)
 {
-	u32 *target = (u32 *)(fr_buffer + fr_pos);
+	u32 *target = (u32 *)(fr_buffer + fr_head);
 
 	if (fr_enabled) {
 		*target = cpu_to_le32(data);
-		fr_pos += round_up(sizeof(u32), 8);
+		nandrec_advance_head(round_up(sizeof(u32), 8));
 	}
 }
 
 static void write_u64(u64 data)
 {
-	u64 *target = (u64 *)(fr_buffer + fr_pos);
+	u64 *target = (u64 *)(fr_buffer + fr_head);
 
 	if (fr_enabled) {
 		*target = cpu_to_le64(data);
-		fr_pos += round_up(sizeof(u64), 8);
+		nandrec_advance_head(round_up(sizeof(u64), 8));
 	}
 }
 
 static void write_blob(const u_char *data, int len)
 {
-	u32 *target = (u32 *)(fr_buffer + fr_pos);
+	u32 *target = (u32 *)(fr_buffer + fr_head);
 
 	if (fr_enabled) {
-		memcpy(target, data, len);
-		fr_pos += round_up(len, 8);
+		if (fr_head + len > FR_BUFFER_TOTAL_SIZE) {
+			int p1;
+
+			p1 = FR_BUFFER_TOTAL_SIZE - fr_head;
+			memcpy(target, data, p1);
+			memcpy(fr_buffer, data + p1, len - p1);
+		} else {
+			memcpy(target, data, len);
+		}
+		nandrec_advance_head(round_up(len, 8));
 	}
 }
 
@@ -170,12 +234,11 @@ static void start_write(int size)
 	mutex_lock(&fr_mutex);
 
 	if (fr_enabled) {
-		while (fr_pos + size + 2*8 >=
-				FR_BUFFER_TOTAL_SIZE - FR_BUFFER_MARGIN) {
+		while (nandrec_available_write() < size + 2*8) {
 			DEFINE_WAIT(wait);
 
-			pr_info("%p: Waiting - write count %d, current %d.\n",
-				current, size, fr_pos);
+			pr_info("%p: Waiting - write count %d, current %d/%d.\n",
+				current, size, fr_tail, fr_head);
 
 			prepare_to_wait(&fr_wait, &wait, TASK_UNINTERRUPTIBLE);
 
@@ -191,10 +254,8 @@ static void start_write(int size)
 
 static void end_write(void)
 {
-	if (fr_enabled) {
+	if (fr_enabled)
 		write_u32(MAGIC_END);
-		WARN_ON(fr_pos > FR_BUFFER_TOTAL_SIZE - FR_BUFFER_MARGIN);
-	}
 	mutex_unlock(&fr_mutex);
 }
 
-- 
2.14.1

[PATCH 5/5] mtdram flight recorder: Add checksums.

[Dirk Behme]

From: Manfred Spraul <manfred@colorfullife.com>

Add checksums, to ensure that corruptions can be detected.
To allow userspace to detect the new fields, use new IDs
for WRITE/ERASE commands.

Signed-off-by: Manfred Spraul <manfred.spraul@de.bosch.com>
Cc: Manfred Spraul <manfred@colorfullife.com>
---
 drivers/mtd/devices/mtdram.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
index 202696bc92ef..cdf5ae90943b 100644
--- a/drivers/mtd/devices/mtdram.c
+++ b/drivers/mtd/devices/mtdram.c
@@ -190,6 +190,11 @@ static const struct file_operations fr_fops = {
 
 #define FUNC_WRITE	1UL
 #define FUNC_ERASE	2UL
+#define FUNC_WRITE_CHK	3UL
+#define FUNC_ERASE_CHK	4UL
+#define CHECK_VAL1	7ULL
+#define CHECK_VAL2	(2*76777ULL)
+#define CHECK_VAL3	104677ULL
 
 static void write_u32(u32 data)
 {
@@ -348,9 +353,10 @@ static int ram_erase(struct mtd_info *mtd, struct erase_info *instr)
 
 #ifdef CONFIG_MTDRAM_FLIGHTRECORDER
 	start_write(3*8);
-	write_u32(FUNC_ERASE);
+	write_u32(FUNC_ERASE_CHK);
 	write_u64(instr->addr);
 	write_u64(instr->len);
+	write_u64(CHECK_VAL1*(u64)instr->len + CHECK_VAL2*(u64)instr->addr);
 	end_write();
 #endif
 
@@ -407,12 +413,23 @@ static int ram_write(struct mtd_info *mtd, loff_t to, size_t len,
 	memcpy((char *)mtd->priv + to, buf, len);
 
 #ifdef CONFIG_MTDRAM_FLIGHTRECORDER
-	start_write(3*8 + len);
-	write_u32(FUNC_WRITE);
-	write_u64(to);
-	write_u64(len);
-	write_blob(buf, len);
-	end_write();
+	start_write(4*8 + len);
+	{
+		u64 i;
+		u64 chk;
+
+		write_u32(FUNC_WRITE_CHK);
+
+		chk = CHECK_VAL1*(u64)to + CHECK_VAL2*(u64)len;
+		for (i = 0; i < len; i++)
+			chk += (i + buf[i])*CHECK_VAL3;
+
+		write_u64(to);
+		write_u64(len);
+		write_blob(buf, len);
+		write_u64(chk);
+		end_write();
+	}
 #endif
 
 	*retlen = len;
-- 
2.14.1

Re: [PATCH 0/5] Add flight recorder to MTDRAM

[Richard Weinberger]

Dirk, Manfred,

Am Mittwoch, 6. Dezember 2017, 09:50:34 CET schrieb Dirk Behme:
> From: Manfred Spraul <manfred@colorfullife.com>
> 
> Hi,
> 
> The series adds a flight recorder to MTDRAM.

Thanks a lot for sharing your tool, this is highly appreciated.

> This allows very efficient power fail testing:
> From the flight recorder output, it is possible to recreate every image
> that might have existed between the start of the recording and the end.
> 
> Obviously, a user space tool is required, it is attached as the last
> mail in the series.

So, to understand this approach better I need to recap.
The "flight recorder" logs every single MTD operation (READ, ERASE, PROGRAM) 
to a file while the MTD is under load, right?

Then you take the log, replay it to a _file_ but instead of replaying
all N MTD operations only N - X operations are replayed?
The output file is later written back to MTDRAM to check how much UBIFS likes 
it?

While having such a tool would be awesome, we have to be very sure that it 
behaves correctly.
Yesterday I spent almost the whole night with staring at some of Manfred's 
images and I'm not sure whether what I see makes sense or can actually happen
on a real NAND or NOR flash. But I'm still investigating.

> Patches:
> 
> 0001-mtdram-expose-write-size-and-writebuf-size-as-module:
> 	An initial cleanup: write_size and writebuf_size are
> 	hardcoded in the source code.
> 	Convert that to module parameters.

MTDRAM is a special purpose MTD simulator, I'm not so sure whether it is a 
good idea to turn it into a NAND-alike zombie.

I said this already some time before, Boris and I are in general unhappy with 
the current MTD simulator zoo in MTD.
But fixing this is not your job, we have to. :-)

Thanks,
//richard

Re: [PATCH 0/5] Add flight recorder to MTDRAM

[Manfred Spraul]

Hi Richard,

On 12/06/2017 11:41 AM, Richard Weinberger wrote:
> Dirk, Manfred,
>
> Am Mittwoch, 6. Dezember 2017, 09:50:34 CET schrieb Dirk Behme:
>> From: Manfred Spraul <manfred@colorfullife.com>
>>
>> Hi,
>>
>> The series adds a flight recorder to MTDRAM.
> Thanks a lot for sharing your tool, this is highly appreciated.
>
>> This allows very efficient power fail testing:
>>  From the flight recorder output, it is possible to recreate every image
>> that might have existed between the start of the recording and the end.
>>
>> Obviously, a user space tool is required, it is attached as the last
>> mail in the series.
> So, to understand this approach better I need to recap.
> The "flight recorder" logs every single MTD operation (READ, ERASE, PROGRAM)
> to a file while the MTD is under load, right?
Only ERASE and PROGRAM. READ is not logged.
Would it help if READ is logged as well? (memory is cheap, ...)

What would be fairly simple is to add a backtrace for every ERASE and 
PROGRAM. I'll try to add that.

> Then you take the log, replay it to a _file_ but instead of replaying
> all N MTD operations only N - X operations are replayed?
Exactly.  Instead of replaying all N operations, only X operations are 
replayed.

image-168167.bin is after replaying 168167 operations.
image-168168.bin is after replaying one additional operation.
> The output file is later written back to MTDRAM to check how much UBIFS likes
> it?
Exactly.
> While having such a tool would be awesome, we have to be very sure that it
> behaves correctly.
> Yesterday I spent almost the whole night with staring at some of Manfred's
> images and I'm not sure whether what I see makes sense or can actually happen
> on a real NAND or NOR flash. But I'm still investigating.
 From my understand, the tool result is exactly identical to a powerfail 
immediately after PROGRAM.
What differs from realistic embedded systems is obviously performance:
RAM disk+2-core I3 is probably much faster & much more parallelism happens.

I have uploaded the initial image, the final image and the flight recording.

https://sourceforge.net/projects/calculix-rpm/files/ubifs/xattr/

--
     Manfred

Re: [PATCH 0/5] Add flight recorder to MTDRAM

[Richard Weinberger]

Manfred,

Am Mittwoch, 6. Dezember 2017, 20:44:55 CET schrieb Manfred Spraul:
> Hi Richard,
> 
> On 12/06/2017 11:41 AM, Richard Weinberger wrote:
> > Dirk, Manfred,
> > 
> > Am Mittwoch, 6. Dezember 2017, 09:50:34 CET schrieb Dirk Behme:
> >> From: Manfred Spraul <manfred@colorfullife.com>
> >> 
> >> Hi,
> >> 
> >> The series adds a flight recorder to MTDRAM.
> > 
> > Thanks a lot for sharing your tool, this is highly appreciated.
> > 
> >> This allows very efficient power fail testing:
> >>  From the flight recorder output, it is possible to recreate every image
> >> 
> >> that might have existed between the start of the recording and the end.
> >> 
> >> Obviously, a user space tool is required, it is attached as the last
> >> mail in the series.
> > 
> > So, to understand this approach better I need to recap.
> > The "flight recorder" logs every single MTD operation (READ, ERASE,
> > PROGRAM) to a file while the MTD is under load, right?
> 
> Only ERASE and PROGRAM. READ is not logged.
> Would it help if READ is logged as well? (memory is cheap, ...)
> 
> What would be fairly simple is to add a backtrace for every ERASE and
> PROGRAM. I'll try to add that.

Given a second thought, for power-cut testing READ is not important.
So no need to hurry.

> > Then you take the log, replay it to a _file_ but instead of replaying
> > all N MTD operations only N - X operations are replayed?
> 
> Exactly.  Instead of replaying all N operations, only X operations are
> replayed.
> 
> image-168167.bin is after replaying 168167 operations.
> image-168168.bin is after replaying one additional operation.

This is what I thought.
It worries me a bit that image-168167.bin shows a corrupted LPT (LEB property 
tree). The current logical operation of UBIFS is writing the index tree.

> > The output file is later written back to MTDRAM to check how much UBIFS
> > likes it?
> 
> Exactly.
> 
> > While having such a tool would be awesome, we have to be very sure that it
> > behaves correctly.
> > Yesterday I spent almost the whole night with staring at some of Manfred's
> > images and I'm not sure whether what I see makes sense or can actually
> > happen on a real NAND or NOR flash. But I'm still investigating.
> 
>  From my understand, the tool result is exactly identical to a powerfail
> immediately after PROGRAM.

Yes.

> What differs from realistic embedded systems is obviously performance:
> RAM disk+2-core I3 is probably much faster & much more parallelism happens.

Yep. I found also some UBI and UBIFS on my x86 system with nandsim and 
powercuts over the last years.

> I have uploaded the initial image, the final image and the flight recording.
> 
> https://sourceforge.net/projects/calculix-rpm/files/ubifs/xattr/

Cool! I'll give it a try.

Thanks,
//richard

Re: [PATCH 0/5] Add flight recorder to MTDRAM

[Richard Weinberger]

Manfred,

Am Mittwoch, 6. Dezember 2017, 20:59:43 CET schrieb Richard Weinberger:
> > I have uploaded the initial image, the final image and the flight
> > recording.
> > 
> > https://sourceforge.net/projects/calculix-rpm/files/ubifs/xattr/
> 
> Cool! I'll give it a try.

Hmm, I did:
./replay -e 131072 -w 2048 dump-before.bin record.bin 168167
 
This applies 168167 to dump-before.bin, right?
So I expect dump-before.bin to match the sha1sum of image-168167.bin.
But it doesn't.

Thanks,
//richard

Re: [PATCH 0/5] Add flight recorder to MTDRAM

[Manfred Spraul]

Hi Ricard,

On 12/06/2017 09:57 PM, Richard Weinberger wrote:
> Manfred,
>
> Am Mittwoch, 6. Dezember 2017, 20:59:43 CET schrieb Richard Weinberger:
>>> I have uploaded the initial image, the final image and the flight
>>> recording.
>>>
>>> https://sourceforge.net/projects/calculix-rpm/files/ubifs/xattr/
>> Cool! I'll give it a try.
> Hmm, I did:
> ./replay -e 131072 -w 2048 dump-before.bin record.bin 168167
>   
> This applies 168167 to dump-before.bin, right?
> So I expect dump-before.bin to match the sha1sum of image-168167.bin.
> But it doesn't.
./replay dump-before.bin record.bin 168167

If you specify "-w 2048", then replay.cpp splits large writes into 
multiple operations.
e.g.: there are some 126 kB writes.
> Thanks,
> //richard