[mtd-utils,v3] nandmarkbad: new util to mark blocks as bad

Message ID 20170830113146.26929-1-u.kleine-koenig@pengutronix.de
State New
Delegated to: David Oberhollenzer
Headers show
Series
  • [mtd-utils,v3] nandmarkbad: new util to mark blocks as bad
Related show

Commit Message

Uwe Kleine-König Aug. 30, 2017, 11:31 a.m.
---
changes since v2, sent with Message-Id: 20170830112145.26586-1-u.kleine-koenig@pengutronix.de:
 - actually committed the changes advertised for v2 (sigh)

changes since (implicit) v1 sent with Message-Id: 20170824152227.30394-1-ukleinek@debian.org:

 - fix printf format resulting in crash
   (suggested by gcc and David Oberhollenzer)
 - add options --help, --version, -V (suggested by David Oberhollenzer)
 - add options --i-know-what-i-do, -y (suggested by Richard Weinberger)
 - add an error message if no device was given
 - make usage static
 - use right author email address

 .gitignore               |   1 +
 nand-utils/Makemodule.am |   5 +-
 nand-utils/nandmarkbad.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 nand-utils/nandmarkbad.c

Comments

David Oberhollenzer Sept. 13, 2017, 9:39 a.m. | #1
Hi,


sorry for the late reply. After requesting the initial changes, I caught up on
the IRC discussion and came under the impression that it was pretty clear to
everyone that there are a few major issues with this kind of tool.

First of all, marking a block bad can currently not be undone. The changes this
tool makes are thus both destructive and irreversible.

If the MTD contains UBI data, UBI has no way of moving the data somewhere else
if a block just randomly goes bad completely out of the blue.

Besides, if the MTD is attached to an UBI device, UBI already takes care of
detecting blocks that go bad *over time* and takes care of marking them as
bad after moving the data somewhere else.

It might be interesting to have such a tool if nand scrub functionality was
added to the kernel (such as in [1]), allowing the changes done by the tool
to be undone. But even then, what would be the practical use case for
such a tool?


Thanks,

David


[1] http://patchwork.ozlabs.org/patch/764978/
Uwe Kleine-König Sept. 13, 2017, 9:53 a.m. | #2
Hello David,

On Wed, Sep 13, 2017 at 11:39:31AM +0200, David Oberhollenzer wrote:
> 
> sorry for the late reply. After requesting the initial changes, I caught up on
> the IRC discussion and came under the impression that it was pretty clear to
> everyone that there are a few major issues with this kind of tool.
> 
> First of all, marking a block bad can currently not be undone. The changes this
> tool makes are thus both destructive and irreversible.
> 
> If the MTD contains UBI data, UBI has no way of moving the data somewhere else
> if a block just randomly goes bad completely out of the blue.
> 
> Besides, if the MTD is attached to an UBI device, UBI already takes care of
> detecting blocks that go bad *over time* and takes care of marking them as
> bad after moving the data somewhere else.
> 
> It might be interesting to have such a tool if nand scrub functionality was
> added to the kernel (such as in [1]), allowing the changes done by the tool
> to be undone. But even then, what would be the practical use case for
> such a tool?

I needed it as part of reproducing a problem I was told to fix. I only
used it on a nandsim device for testing. I could imagine though that the
tool could also come handy while developping a mtd driver. But yes, it
would be more useful in combination with a possibility to unmark a bad
block.

In sum: I don't care much if you take it or not. I just thought it might
come handy for someone else and so I shared my work.

Best regards
Uwe

Patch

diff --git a/.gitignore b/.gitignore
index 38bd04deb34e..754da97b43bf 100644
--- a/.gitignore
+++ b/.gitignore
@@ -36,6 +36,7 @@  mkfs.jffs2
 mtd_debug
 mtdpart
 nanddump
+nandmarkbad
 nandtest
 nandwrite
 nftl_format
diff --git a/nand-utils/Makemodule.am b/nand-utils/Makemodule.am
index d75b0cb3e36c..c31dcb01f06e 100644
--- a/nand-utils/Makemodule.am
+++ b/nand-utils/Makemodule.am
@@ -7,6 +7,9 @@  nandwrite_LDADD = libmtd.a
 nandtest_SOURCES = nand-utils/nandtest.c
 nandtest_LDADD = libmtd.a
 
+nandmarkbad_SOURCES = nand-utils/nandmarkbad.c
+nandmarkbad_LDADD = libmtd.a
+
 nftldump_SOURCES = nand-utils/nftldump.c
 nftldump_LDADD = libmtd.a
 
@@ -14,7 +17,7 @@  nftl_format_SOURCES = nand-utils/nftl_format.c
 nftl_format_LDADD = libmtd.a
 
 NAND_BINS = \
-	nanddump nandwrite nandtest nftldump nftl_format
+	nanddump nandwrite nandtest nandmarkbad nftldump nftl_format
 
 NAND_SH = \
 	nand-utils/load_nandsim.sh
diff --git a/nand-utils/nandmarkbad.c b/nand-utils/nandmarkbad.c
new file mode 100644
index 000000000000..cf05698c3609
--- /dev/null
+++ b/nand-utils/nandmarkbad.c
@@ -0,0 +1,119 @@ 
+#define PROGRAM_NAME "nandmarkbad"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <getopt.h>
+
+#include "common.h"
+#include <libmtd.h>
+
+static void usage(int status)
+{
+	fprintf(status ? stderr : stdout,
+		"usage: %s [OPTIONS] <device>\n\n"
+		"  -b, --markbad=blockno        Mark block bad\n"
+		"  -h, --help                   Display this help and exit\n"
+		"  -V, --version                Output version information and exit\n"
+		"  -y, --i-know-what-i-do       really do mark blocks as bad\n",
+		PROGRAM_NAME);
+	exit(status);
+}
+
+/*
+ * Main program
+ */
+int main(int argc, char **argv)
+{
+	loff_t mark_bad[32];
+	unsigned cnt_bad = 0;
+	struct mtd_dev_info mtd;
+	libmtd_t mtd_desc;
+	int fd;
+	int error = 0;
+	int ret;
+	unsigned int i;
+	int iknowwhatido = 0;
+
+	for (;;) {
+		static const char short_options[] = "b:hVy";
+		static const struct option long_options[] = {
+			{ "help", no_argument, 0, 'h' },
+			{ "markbad", required_argument, 0, 'b' },
+			{ "version", no_argument, 0, 'V'},
+			{ "i-know-what-i-do", no_argument, 0, 'y' },
+			{0, 0, 0, 0},
+		};
+		int option_index = 0;
+		int c = getopt_long(argc, argv, short_options, long_options,
+				    &option_index);
+		if (c == EOF)
+			break;
+
+		switch (c) {
+		case '?':
+			usage(EXIT_FAILURE);
+			break;
+
+		case 'b':
+			if (cnt_bad < ARRAY_SIZE(mark_bad)) {
+				mark_bad[cnt_bad] =
+					simple_strtoll(optarg, &error);
+				++cnt_bad;
+			} else {
+				errmsg_die("Can't handle so many bad blocks\n");
+			}
+
+			break;
+
+		case 'h':
+			usage(EXIT_SUCCESS);
+			break;
+
+		case 'V':
+			common_print_version();
+			return EXIT_SUCCESS;
+
+		case 'y':
+			iknowwhatido = 1;
+			break;
+		}
+	}
+
+	argc -= optind;
+	argv += optind;
+
+	if (error)
+		usage(EXIT_FAILURE);
+
+	if (argc != 1)
+		errmsg_die("You must specify a device to operate on\n");
+
+	if (!cnt_bad)
+		errmsg_die("You must specify at least one block to mark bad\n");
+
+	if (!iknowwhatido)
+		errmsg_die(PROGRAM_NAME " does things that are hard to undo.\n"
+			   "\tPlease convince yourself you understand the risks,\n"
+			   "\tthen add --i-know-what-i-do to the options.\n");
+
+	fd = open(argv[0], O_RDWR);
+	if (fd < 0)
+		sys_errmsg_die("Failed to open mtd device\n");
+
+	mtd_desc = libmtd_open();
+	if (!mtd_desc)
+		errmsg_die("Can't initialize libmtd");
+
+	if (mtd_get_dev_info(mtd_desc, argv[0], &mtd) < 0)
+		errmsg_die("mtd_get_dev_info failed");
+
+	for (i = 0; i < cnt_bad; ++i) {
+		ret = mtd_mark_bad(&mtd, fd, mark_bad[i]);
+		if (ret)
+			sys_errmsg_die("%s: MTD Mark bad block failure",
+				       argv[0]);
+	}
+
+	return EXIT_SUCCESS;
+}