[PATCH][MTD-UTILS] Set mkfs.jffs2 page size runtime instead of fixed.

[PATCH][MTD-UTILS] Set mkfs.jffs2 page size runtime instead of fixed.

[Ricard Wanderlof]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1954 bytes --]


This patch reads the default PAGE_SIZE from sysconf(), i.e. the system
mkfs.jffs2 is running on, instead of just setting it to 4096 (which of 
course is valid for most systems but not all).

This is useful if mkfs.jffs2 is running on the target system, e.g. to 
create a backup image during firmware upgrade, so that the page size does 
not have to be set explicitly using a command line parameter.

The --pagesize option is supported just as before.

Patch is included both as an attachment for patching and inline for 
review. (Patch made against git head).

Signed-off-by Ricard Wanderlöf <ricardw@axis.com> .

Index: mkfs.jffs2.c
===================================================================
RCS file: /usr/local/cvs/linux/apps/utils/mtdutils/mkfs.jffs2.c,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -r1.2 -r1.3
--- mkfs.jffs2.c	17 Oct 2006 11:40:40 -0000	1.2
+++ mkfs.jffs2.c	6 Dec 2007 07:46:47 -0000	1.3
@@ -672,9 +672,9 @@
  	0xff, 0xff, 0xff, 0xff, 0xff
  };

-/* We default to 4096, per x86.  When building a fs for
- * 64-bit arches and whatnot, use the --pagesize=SIZE option */
-int page_size = 4096;
+/* We set this at start of main() using sysconf(), -1 means we don't know */
+/* When building an fs for non-native systems, use --pagesize=SIZE option */
+int page_size = -1;

  #include "compr.h"

@@ -787,6 +787,10 @@
  	struct stat *statbuf;
  	unsigned int totcomp = 0;

+	page_size = sysconf(_SC_PAGESIZE);
+	if (page_size < 0) /* System doesn't know so ... */
+		page_size = 4096; /* ... we make an educated guess */
+
  	statbuf = &(e->sb);
  	if (statbuf->st_size >= JFFS2_MAX_FILE_SIZE) {
  		error_msg("Skipping file \"%s\" too large.", e->path);

/Ricard
--
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

[-- Attachment #2: patch file --]
[-- Type: TEXT/x-diff, Size: 1106 bytes --]

Index: mkfs.jffs2.c
===================================================================
RCS file: /usr/local/cvs/linux/apps/utils/mtdutils/mkfs.jffs2.c,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -r1.2 -r1.3
--- mkfs.jffs2.c	17 Oct 2006 11:40:40 -0000	1.2
+++ mkfs.jffs2.c	6 Dec 2007 07:46:47 -0000	1.3
@@ -672,9 +672,9 @@
 	0xff, 0xff, 0xff, 0xff, 0xff
 };
 
-/* We default to 4096, per x86.  When building a fs for
- * 64-bit arches and whatnot, use the --pagesize=SIZE option */
-int page_size = 4096;
+/* We set this at start of main() using sysconf(), -1 means we don't know */
+/* When building an fs for non-native systems, use --pagesize=SIZE option */
+int page_size = -1;
 
 #include "compr.h"
 
@@ -787,6 +787,10 @@
 	struct stat *statbuf;
 	unsigned int totcomp = 0;
 
+	page_size = sysconf(_SC_PAGESIZE);
+	if (page_size < 0) /* System doesn't know so ... */
+		page_size = 4096; /* ... we make an educated guess */
+
 	statbuf = &(e->sb);
 	if (statbuf->st_size >= JFFS2_MAX_FILE_SIZE) {
 		error_msg("Skipping file \"%s\" too large.", e->path);

Re: [PATCH][MTD-UTILS] Set mkfs.jffs2 page size runtime instead of fixed.

[Josh Boyer]

On Thu, 6 Dec 2007 12:43:47 +0100 (CET)
Ricard Wanderlof <ricard.wanderlof@axis.com> wrote:

> 
> This patch reads the default PAGE_SIZE from sysconf(), i.e. the system
> mkfs.jffs2 is running on, instead of just setting it to 4096 (which of 
> course is valid for most systems but not all).

This makes sense overall.  I'd like to do it a slightly different way
though.

> This is useful if mkfs.jffs2 is running on the target system, e.g. to 
> create a backup image during firmware upgrade, so that the page size does 
> not have to be set explicitly using a command line parameter.
> 
> The --pagesize option is supported just as before.
> 
> Patch is included both as an attachment for patching and inline for 
> review. (Patch made against git head).
> 
> Signed-off-by Ricard Wanderlöf <ricardw@axis.com> .
> 
> Index: mkfs.jffs2.c
> ===================================================================
> RCS file: /usr/local/cvs/linux/apps/utils/mtdutils/mkfs.jffs2.c,v
> retrieving revision 1.2
> retrieving revision 1.3
> diff -u -r1.2 -r1.3
> --- mkfs.jffs2.c	17 Oct 2006 11:40:40 -0000	1.2
> +++ mkfs.jffs2.c	6 Dec 2007 07:46:47 -0000	1.3
> @@ -672,9 +672,9 @@
>   	0xff, 0xff, 0xff, 0xff, 0xff
>   };
> 
> -/* We default to 4096, per x86.  When building a fs for
> - * 64-bit arches and whatnot, use the --pagesize=SIZE option */
> -int page_size = 4096;
> +/* We set this at start of main() using sysconf(), -1 means we don't know */
> +/* When building an fs for non-native systems, use --pagesize=SIZE option */
> +int page_size = -1;
> 
>   #include "compr.h"
> 
> @@ -787,6 +787,10 @@
>   	struct stat *statbuf;
>   	unsigned int totcomp = 0;
> 
> +	page_size = sysconf(_SC_PAGESIZE);
> +	if (page_size < 0) /* System doesn't know so ... */
> +		page_size = 4096; /* ... we make an educated guess */
> +

Could we add an additional check here to see if page_size != 4096, and
print a loud warning with the page size that is actually being used and
suggest the --pagesize option if that isn't what is desired?

It's not that we have tons of systems using a page size different than
4KiB, but it's a change in behavior for those that do and I'd like to
at least warn users about it.  Otherwise the patch looks fine.

josh

Re: [PATCH][MTD-UTILS] Set mkfs.jffs2 page size runtime instead of fixed.

[Ricard Wanderlof]

On Fri, 7 Dec 2007, Josh Boyer wrote:

>> This patch reads the default PAGE_SIZE from sysconf(), i.e. the system
>> mkfs.jffs2 is running on, instead of just setting it to 4096 (which of
>> course is valid for most systems but not all).
>
> This makes sense overall.  I'd like to do it a slightly different way
> though.

I'm open to suggestions. Care to elaborate on that?

> Could we add an additional check here to see if page_size != 4096, and
> print a loud warning with the page size that is actually being used and
> suggest the --pagesize option if that isn't what is desired?
>
> It's not that we have tons of systems using a page size different than
> 4KiB, but it's a change in behavior for those that do and I'd like to
> at least warn users about it.  Otherwise the patch looks fine.

I guess you are considering the case of making a file system on a host 
with a page size of != 4096 for a target system with 4096 byte pages. Good 
point, I hadn't thought of that.

I was very much into the 'my host is x86 and my target is something else' 
train of though; in this case it wouldn't be much of a problem, as the 
user would be supplying a --pagesize anyway to get the correct page size 
for the target.

I'll get back with a new patch as soon as possible.

/Ricard
--
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

[PATCH][MTD-UTILS] Set mkfs.jffs2 page size runtime instead of fixed [respoin]

[Ricard Wanderlof]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3808 bytes --]


On Fri, 7 Dec 2007, Josh Boyer wrote:

> On Thu, 6 Dec 2007 12:43:47 +0100 (CET)
> Ricard Wanderlof <ricard.wanderlof@axis.com> wrote:
>
>>
>> This patch reads the default PAGE_SIZE from sysconf(), i.e. the system
>> mkfs.jffs2 is running on, instead of just setting it to 4096 (which of
>> course is valid for most systems but not all).
>
> This makes sense overall.  I'd like to do it a slightly different way 
> though.

I think I see what you mean. I had inadvertently put the setting 
of the page_size variable at the start of write_regular_file() instead of 
at the start of main() where I wanted it. Fixed.

> Could we add an additional check here to see if page_size != 4096, and
> print a loud warning with the page size that is actually being used and
> suggest the --pagesize option if that isn't what is desired?
>
> It's not that we have tons of systems using a page size different than
> 4KiB, but it's a change in behavior for those that do and I'd like to
> at least warn users about it.  Otherwise the patch looks fine.

Ok, good point, something like this then:

--------------------------

This patch reads the default PAGE_SIZE from sysconf(), i.e. the system
mkfs.jffs2 is running on, instead of just setting it to 4096 (which of
course is valid for most systems but not all).

This is useful if mkfs.jffs2 is running on the target system, e.g. to
create a backup image during firmware upgrade, so that the page size does
not have to be set explicitly using a command line parameter.

The --pagesize option is supported just as before.

If the user has not set the page size explicitly with --pagesize, and
the system page size is anything other than 4096, warn the user that
an unusual page size is being used, since this behavior is different
from before.

Patch is included both as an attachment for patching and inline for
review. (Patch made against git head).

Signed-off-by Ricard Wanderlöf <ricardw@axis.com> .

diff -u org/mkfs.jffs2.c work/mkfs.jffs2.c
--- org/mkfs.jffs2.c	2007-12-06 11:49:39.545489000 +0100
+++ work/mkfs.jffs2.c	2007-12-20 12:39:33.842344000 +0100
@@ -672,9 +672,9 @@
  	0xff, 0xff, 0xff, 0xff, 0xff
  };

-/* We default to 4096, per x86.  When building a fs for
- * 64-bit arches and whatnot, use the --pagesize=SIZE option */
-int page_size = 4096;
+/* We set this at start of main() using sysconf(), -1 means we don't know */
+/* When building an fs for non-native systems, use --pagesize=SIZE option */
+int page_size = -1;

  #include "compr.h"

@@ -1617,9 +1617,16 @@
  	struct filesystem_entry *root;
  	char *compr_name = NULL;
  	int compr_prior  = -1;
+	int warn_page_size = 0;

  	jffs2_compressors_init();

+	page_size = sysconf(_SC_PAGESIZE);
+	if (page_size < 0) /* System doesn't know so ... */
+		page_size = 4096; /* ... we make an educated guess */
+	if (page_size != 4096)
+		warn_page_size = 1; /* warn user if page size not 4096 */
+
  	while ((opt = getopt_long(argc, argv,
  					"D:d:r:s:o:qUPfh?vVe:lbp::nc:m:x:X:Lty:i:", long_options, &c)) >= 0)
  	{
@@ -1642,6 +1649,7 @@

  			case 's':
  				page_size = strtol(optarg, NULL, 0);
+				warn_page_size = 0; /* set by user, so don't need to warn */
  				break;

  			case 'o':
@@ -1800,6 +1808,10 @@
  #endif
  		}
  	}
+	if (warn_page_size) {
+		error_msg("Page size for this system is by default %d", page_size);
+		error_msg("Use the --pagesize=SIZE option if this is not what you want");
+	}
  	if (out_fd == -1) {
  		if (isatty(1)) {
  			error_msg_and_die(helptext);

--
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

[-- Attachment #2: Patch file --]
[-- Type: TEXT/PLAIN, Size: 1599 bytes --]

diff -u org/mkfs.jffs2.c work/mkfs.jffs2.c
--- org/mkfs.jffs2.c	2007-12-06 11:49:39.545489000 +0100
+++ work/mkfs.jffs2.c	2007-12-20 12:39:33.842344000 +0100
@@ -672,9 +672,9 @@
 	0xff, 0xff, 0xff, 0xff, 0xff
 };
 
-/* We default to 4096, per x86.  When building a fs for
- * 64-bit arches and whatnot, use the --pagesize=SIZE option */
-int page_size = 4096;
+/* We set this at start of main() using sysconf(), -1 means we don't know */
+/* When building an fs for non-native systems, use --pagesize=SIZE option */
+int page_size = -1;
 
 #include "compr.h"
 
@@ -1617,9 +1617,16 @@
 	struct filesystem_entry *root;
 	char *compr_name = NULL;
 	int compr_prior  = -1;
+	int warn_page_size = 0;
 
 	jffs2_compressors_init();
 
+	page_size = sysconf(_SC_PAGESIZE);
+	if (page_size < 0) /* System doesn't know so ... */
+		page_size = 4096; /* ... we make an educated guess */
+	if (page_size != 4096)
+		warn_page_size = 1; /* warn user if page size not 4096 */
+
 	while ((opt = getopt_long(argc, argv,
 					"D:d:r:s:o:qUPfh?vVe:lbp::nc:m:x:X:Lty:i:", long_options, &c)) >= 0)
 	{
@@ -1642,6 +1649,7 @@
 
 			case 's':
 				page_size = strtol(optarg, NULL, 0);
+				warn_page_size = 0; /* set by user, so don't need to warn */
 				break;
 
 			case 'o':
@@ -1800,6 +1808,10 @@
 #endif
 		}
 	}
+	if (warn_page_size) {
+		error_msg("Page size for this system is by default %d", page_size);
+		error_msg("Use the --pagesize=SIZE option if this is not what you want");
+	}
 	if (out_fd == -1) {
 		if (isatty(1)) {
 			error_msg_and_die(helptext);

Re: [PATCH][MTD-UTILS] Set mkfs.jffs2 page size runtime instead of fixed [respoin]

[Josh Boyer]

On Thu, 20 Dec 2007 12:46:38 +0100 (CET)
Ricard Wanderlof <ricard.wanderlof@axis.com> wrote:

> 
> On Fri, 7 Dec 2007, Josh Boyer wrote:
> 
> > On Thu, 6 Dec 2007 12:43:47 +0100 (CET)
> > Ricard Wanderlof <ricard.wanderlof@axis.com> wrote:
> >
> >>
> >> This patch reads the default PAGE_SIZE from sysconf(), i.e. the system
> >> mkfs.jffs2 is running on, instead of just setting it to 4096 (which of
> >> course is valid for most systems but not all).
> >
> > This makes sense overall.  I'd like to do it a slightly different way 
> > though.
> 
> I think I see what you mean. I had inadvertently put the setting 
> of the page_size variable at the start of write_regular_file() instead of 
> at the start of main() where I wanted it. Fixed.
> 
> > Could we add an additional check here to see if page_size != 4096, and
> > print a loud warning with the page size that is actually being used and
> > suggest the --pagesize option if that isn't what is desired?
> >
> > It's not that we have tons of systems using a page size different than
> > 4KiB, but it's a change in behavior for those that do and I'd like to
> > at least warn users about it.  Otherwise the patch looks fine.
> 
> Ok, good point, something like this then:

Looks good.  Applied.

josh