[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] --cleanmarkers option in mkfs.jffs2



On Wednesday 11 September 2002 00.04, David Woodhouse wrote:
> johan.adolfsson@xxxxxxx.com said:
> > OK, make sense - I just didn't want to change the default behavior.
>
> I do. I've been intending to do it for ages but haven't got round to it.
>
> Although perhaps I ought to wait till the morning and ponder the
> implications for a wrong-block-size fs.

Is there really a problem, more than that we waste space in the middle 
of a block, or that some blocks misses erasemarkers?
Does the fs code relies on that the block really is empty if there is a 
cleanmarker followed by a few 0xFF's?
in that case, the worst thing that could happen is that the write failes 
and that we have to erase the block again?

> > Why? Because we want to take them into account when garbage
> > collecting? We don't really need to put markers on the blocks we erase
> > if we know that we will write to them immediately - do we?
>
> Yeah, if we don't put them there in the first place then the fs will take
> more space after GC than it did to start with. Perhaps 12 bytes per block
> is less than our margin of error in other cases, but I'm hoping we can
> reduce the error to zero and start curring down the amount of free space
> required for correct operation, so I'd rather not let it slide.

I have attached a new patch, I added a missing pad_if_less_than() in 
output_reg() as well.
It's on by default, and can be turned of with -n -no-cleanmarkers.
Not very tested, but I think the image looks ok.

/Johan
diff -u -p -r1.27 mkfs.jffs2.c
--- mkfs.jffs2.c	5 Sep 2002 15:21:03 -0000	1.27
+++ mkfs.jffs2.c	11 Sep 2002 09:31:57 -0000
@@ -4,6 +4,7 @@
  * Copyright 2001, 2002 Red Hat, Inc.
  *           2001 David A. Schleef <ds@xxxxxxx.com>
  *           2001 Erik Andersen <andersen@xxxxxxx.org>
+ *           2002 Axis Communications AB
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -30,6 +31,8 @@
  *
  * I also added a sample device table file.  See device_table.txt
  *  -Erik, Septermber 2001
+ *
+ * cleanmarkers added by Axis Communications AB
  */
 
 /* $Id: mkfs.jffs2.c,v 1.27 2002/09/05 15:21:03 dwmw2 Exp $ */
@@ -106,6 +109,7 @@ extern unsigned char jffs2_compress(unsi
 
 static int erase_block_size = 65536;
 static int pad_fs_size = 0;
+static int add_cleanmarkers = 1; /* turn of with --no-cleanmarkers */
 static int page_size = 4096;
 static int out_fd = 1;
 static int out_ofs = 0;
@@ -147,6 +151,7 @@ static int target_endian = __BYTE_ORDER;
 	((jint32_t){(host_endian==target_endian)?(x):(swab32(x))})
 
 
+static struct jffs2_unknown_node cleanmarker ; 
 
 /* These are all stolen from busybox's libbb to make
  * error handling simpler (and since I maintain busybox, 
@@ -507,9 +512,19 @@ static void padblock(void)
 
 static inline void pad_block_if_less_than(int req)
 {
+	if (add_cleanmarkers) {
+		if ((out_ofs % erase_block_size) == 0) {
+			full_write(out_fd, &cleanmarker, sizeof(cleanmarker));
+		}
+	}
 	if ((out_ofs % erase_block_size) + req > erase_block_size) {
 		padblock();
 	}
+	if (add_cleanmarkers) {
+		if ((out_ofs % erase_block_size) == 0) {
+			full_write(out_fd, &cleanmarker, sizeof(cleanmarker));
+		}
+	}
 }
 
 static inline void padword(void)
@@ -632,6 +647,8 @@ static void output_reg(int fd, uint32_t 
 		ri.dsize = cpu_to_target32(0);
 		ri.node_crc = cpu_to_target32(crc32(0, &ri, sizeof(ri) - 8));
 
+		pad_block_if_less_than(sizeof(ri));
+
 		full_write(out_fd, &ri, sizeof(ri));
 		padword();
 	}
@@ -1105,6 +1122,7 @@ static struct option long_options[] = {
 	{"output", 1, NULL, 'o'},
 	{"help", 0, NULL, 'h'},
 	{"version", 0, NULL, 'v'},
+	{"no-cleanmarkers", 0, NULL, 'n'},
 	{"big-endian", 0, NULL, 'b'},
 	{"little-endian", 0, NULL, 'l'},
 	{"squash", 0, NULL, 'q'},
@@ -1122,6 +1140,7 @@ static char *helptext =
 	"  -p, --pad[=SIZE]       Pad output to SIZE bytes with 0xFF. If SIZE is\n"
 	"                         not specified, the output is padded to the end of\n"
 	"                         the final erase block\n"
+	"  -n, --no-cleanmarkers  Don't add a cleanmarker to every eraseblock when padding\n"
 	"  -r, -d, --root=DIR     Build filesystem from directory DIR (default: cwd)\n"
 	"  -s, --pagesize=SIZE    Use page size (max data node size) SIZE (default: 4KiB)\n"
 	"  -e, --eraseblock=SIZE  Use erase block size SIZE (default: 64KiB)\n"
@@ -1146,7 +1165,7 @@ int main(int argc, char **argv)
 	struct stat statbuf;
 	FILE *devtable = NULL;
 
-	while ((opt = getopt_long(argc, argv, "D:p::d:r:s:e:o:blqfh?v", 
+	while ((opt = getopt_long(argc, argv, "D:p::d:r:s:e:o:nblqfh?v", 
 			long_options, &c)) >= 0) {
 		switch (opt) {
 		case 'D':
@@ -1162,7 +1181,9 @@ int main(int argc, char **argv)
 			else
 				pad_fs_size = -1;
 			break;
-
+		case 'n':
+			add_cleanmarkers = 0;
+			break;
 		case 'r':
 		case 'd':				/* for compatibility with mkfs.jffs, genext2fs, etc... */
 			if (rootdir != default_rootdir) {
@@ -1234,12 +1255,27 @@ int main(int argc, char **argv)
 		}
 	}
 
+	cleanmarker.magic = cpu_to_target16(JFFS2_MAGIC_BITMASK);
+	cleanmarker.nodetype = cpu_to_target16(JFFS2_NODETYPE_CLEANMARKER);
+	cleanmarker.totlen =   cpu_to_target32(sizeof(cleanmarker));
+
 	go(rootdir, devtable);
 	if (pad_fs_size == -1) {
 		padblock();
 	} else {
-		while (out_ofs < pad_fs_size) {
-			full_write(out_fd, ffbuf, min(16, pad_fs_size - out_ofs));
+
+
+		if (pad_fs_size && add_cleanmarkers){
+			padblock();
+			while (out_ofs < pad_fs_size) {
+				full_write(out_fd, &cleanmarker, sizeof(cleanmarker));
+				padblock();
+			}
+		} else {
+			while (out_ofs < pad_fs_size) {
+				full_write(out_fd, ffbuf, min(16, pad_fs_size - out_ofs));
+			}
+
 		}
 	}