Make JFFS2 endianness configurable
diff mbox series

Message ID 20181101225603.17806-1-nkela@cisco.com
State New
Delegated to: David Woodhouse
Headers show
Series
  • Make JFFS2 endianness configurable
Related show

Commit Message

Nikunj Kela (nkela) Nov. 1, 2018, 10:56 p.m. UTC
This patch allows the endianness of the JFSS2 filesystem to be
specified by config options.

It defaults to native-endian (the previously hard-coded option).

Some architectures benefit from having a single known endianness
of JFFS2 filesystem (for data, not executables) independent of the
endianness of the processor (ARM processors can be switched to either
endianness at run-time).

This patch is taken from:
http://www.infradead.org/pipermail/linux-mtd/2006-January/014717.html

Cc: xe-linux-external@cisco.com
Signed-off-by: Rod Whitby <rod@whitby.id.au>
Signed-off-by: Nikunj Kela <nkela@cisco.com>
---
 fs/jffs2/Kconfig    | 25 +++++++++++++++++++++++++
 fs/jffs2/nodelist.h |  8 +++-----
 2 files changed, 28 insertions(+), 5 deletions(-)

Comments

Daniel Walker (danielwa) Nov. 2, 2018, 12:02 a.m. UTC | #1
On Thu, Nov 01, 2018 at 03:56:03PM -0700, Nikunj Kela wrote:
> This patch allows the endianness of the JFSS2 filesystem to be
> specified by config options.
> 
> It defaults to native-endian (the previously hard-coded option).
> 
> Some architectures benefit from having a single known endianness
> of JFFS2 filesystem (for data, not executables) independent of the
> endianness of the processor (ARM processors can be switched to either
> endianness at run-time).
> 


The description is pretty sad .. We have a product which we released that uses
JFFS2, and that product was release with a kernel in one endianness. Then later
on we decided to change the endianness and now we're stuck with a JFFS2
partition that has the wrong endiannes, in a released product. This patch allows
us to set the endianness to something different from the architecture setting.

So there a significant use case for the change, at least for Cisco.

Daniel
Al Viro Nov. 2, 2018, 12:34 a.m. UTC | #2
On Thu, Nov 01, 2018 at 05:02:36PM -0700, Daniel Walker wrote:
> 
> 
> 
> On Thu, Nov 01, 2018 at 03:56:03PM -0700, Nikunj Kela wrote:
> > This patch allows the endianness of the JFSS2 filesystem to be
> > specified by config options.
> > 
> > It defaults to native-endian (the previously hard-coded option).
> > 
> > Some architectures benefit from having a single known endianness
> > of JFFS2 filesystem (for data, not executables) independent of the
> > endianness of the processor (ARM processors can be switched to either
> > endianness at run-time).
> > 
> 
> 
> The description is pretty sad .. We have a product which we released that uses
> JFFS2, and that product was release with a kernel in one endianness. Then later
> on we decided to change the endianness and now we're stuck with a JFFS2
> partition that has the wrong endiannes, in a released product. This patch allows
> us to set the endianness to something different from the architecture setting.
> 
> So there a significant use case for the change, at least for Cisco.

FWIW, can't we detect it at mount time, as e.g. UFS does?
Joakim Tjernlund Nov. 2, 2018, 8:45 a.m. UTC | #3
On Thu, 2018-11-01 at 15:56 -0700, Nikunj Kela wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> This patch allows the endianness of the JFSS2 filesystem to be
> specified by config options.
> 
> It defaults to native-endian (the previously hard-coded option).
> 
> Some architectures benefit from having a single known endianness
> of JFFS2 filesystem (for data, not executables) independent of the
> endianness of the processor (ARM processors can be switched to either
> endianness at run-time).

I like this too. I have on several occasions needed to mount a JFFS2 BE on
my PC. This saves me from having to hack the src.

 Jocke


> 
> This patch is taken from:
> http://www.infradead.org/pipermail/linux-mtd/2006-January/014717.html
> 
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Rod Whitby <rod@whitby.id.au>
> Signed-off-by: Nikunj Kela <nkela@cisco.com>
> ---
>  fs/jffs2/Kconfig    | 25 +++++++++++++++++++++++++
>  fs/jffs2/nodelist.h |  8 +++-----
>  2 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/jffs2/Kconfig b/fs/jffs2/Kconfig
> index ad850c5bf2ca..86e93fbc9d74 100644
> --- a/fs/jffs2/Kconfig
> +++ b/fs/jffs2/Kconfig
> @@ -182,3 +182,28 @@ config JFFS2_CMODE_FAVOURLZO
>           decompression) at the expense of size.
> 
>  endchoice
> +
> +choice
> +       prompt "JFFS2 endianness"
> +       default JFFS2_NATIVE_ENDIAN
> +       depends on JFFS2_FS
> +       help
> +         You can set here the default endianness of JFFS2 from
> +         the available options. Do not touch if unsure.
> +
> +config JFFS2_NATIVE_ENDIAN
> +       bool "native endian"
> +       help
> +         Uses a native endian bytestream.
> +
> +config JFFS2_BIG_ENDIAN
> +       bool "big endian"
> +       help
> +         Uses a big endian bytestream.
> +
> +config JFFS2_LITTLE_ENDIAN
> +       bool "little endian"
> +       help
> +         Uses a little endian bytestream.
> +
> +endchoice
> diff --git a/fs/jffs2/nodelist.h b/fs/jffs2/nodelist.h
> index 0637271f3770..a1ebf04f217c 100644
> --- a/fs/jffs2/nodelist.h
> +++ b/fs/jffs2/nodelist.h
> @@ -27,12 +27,10 @@
>  #include "os-linux.h"
>  #endif
> 
> -#define JFFS2_NATIVE_ENDIAN
> -
>  /* Note we handle mode bits conversion from JFFS2 (i.e. Linux) to/from
>     whatever OS we're actually running on here too. */
> 
> -#if defined(JFFS2_NATIVE_ENDIAN)
> +#if defined(CONFIG_JFFS2_NATIVE_ENDIAN)
>  #define cpu_to_je16(x) ((jint16_t){x})
>  #define cpu_to_je32(x) ((jint32_t){x})
>  #define cpu_to_jemode(x) ((jmode_t){os_to_jffs2_mode(x)})
> @@ -43,7 +41,7 @@
>  #define je16_to_cpu(x) ((x).v16)
>  #define je32_to_cpu(x) ((x).v32)
>  #define jemode_to_cpu(x) (jffs2_to_os_mode((x).m))
> -#elif defined(JFFS2_BIG_ENDIAN)
> +#elif defined(CONFIG_JFFS2_BIG_ENDIAN)
>  #define cpu_to_je16(x) ((jint16_t){cpu_to_be16(x)})
>  #define cpu_to_je32(x) ((jint32_t){cpu_to_be32(x)})
>  #define cpu_to_jemode(x) ((jmode_t){cpu_to_be32(os_to_jffs2_mode(x))})
> @@ -54,7 +52,7 @@
>  #define je16_to_cpu(x) (be16_to_cpu(x.v16))
>  #define je32_to_cpu(x) (be32_to_cpu(x.v32))
>  #define jemode_to_cpu(x) (be32_to_cpu(jffs2_to_os_mode((x).m)))
> -#elif defined(JFFS2_LITTLE_ENDIAN)
> +#elif defined(CONFIG_JFFS2_LITTLE_ENDIAN)
>  #define cpu_to_je16(x) ((jint16_t){cpu_to_le16(x)})
>  #define cpu_to_je32(x) ((jint32_t){cpu_to_le32(x)})
>  #define cpu_to_jemode(x) ((jmode_t){cpu_to_le32(os_to_jffs2_mode(x))})
> --
> 2.19.1
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Richard Weinberger Nov. 2, 2018, 9:28 a.m. UTC | #4
On Fri, Nov 2, 2018 at 9:46 AM Joakim Tjernlund
<Joakim.Tjernlund@infinera.com> wrote:
>
> On Thu, 2018-11-01 at 15:56 -0700, Nikunj Kela wrote:
> > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> >
> >
> > This patch allows the endianness of the JFSS2 filesystem to be
> > specified by config options.
> >
> > It defaults to native-endian (the previously hard-coded option).
> >
> > Some architectures benefit from having a single known endianness
> > of JFFS2 filesystem (for data, not executables) independent of the
> > endianness of the processor (ARM processors can be switched to either
> > endianness at run-time).
>
> I like this too. I have on several occasions needed to mount a JFFS2 BE on
> my PC. This saves me from having to hack the src.

The feature makes sense, but I don't like it being a Kconfig option.
As Al pointed out, it should be done like UFS does.

IOW, implement a wrapper function which selects the endianness automatically.
That way you can force it as mount option or detect during mount what
kind of JFFS2
you have.
Daniel Walker (danielwa) Nov. 2, 2018, 2:04 p.m. UTC | #5
On Fri, Nov 02, 2018 at 12:34:34AM +0000, Al Viro wrote:
> On Thu, Nov 01, 2018 at 05:02:36PM -0700, Daniel Walker wrote:
> > 
> > 
> > 
> > On Thu, Nov 01, 2018 at 03:56:03PM -0700, Nikunj Kela wrote:
> > > This patch allows the endianness of the JFSS2 filesystem to be
> > > specified by config options.
> > > 
> > > It defaults to native-endian (the previously hard-coded option).
> > > 
> > > Some architectures benefit from having a single known endianness
> > > of JFFS2 filesystem (for data, not executables) independent of the
> > > endianness of the processor (ARM processors can be switched to either
> > > endianness at run-time).
> > > 
> > 
> > 
> > The description is pretty sad .. We have a product which we released that uses
> > JFFS2, and that product was release with a kernel in one endianness. Then later
> > on we decided to change the endianness and now we're stuck with a JFFS2
> > partition that has the wrong endiannes, in a released product. This patch allows
> > us to set the endianness to something different from the architecture setting.
> > 
> > So there a significant use case for the change, at least for Cisco.
> 
> FWIW, can't we detect it at mount time, as e.g. UFS does?

We're not JFFS2 (or even filesystems) experts, but we could make an attempt at
it.

Do you have any thought on how to implement it ? I was think abstracting the
endian functions in the nodelist.h

static inline jint16_t cpu_to_je16(jint16_t x) {
	if (check_big_endian()) {
		return cpu_to_be16(x);
	} else if (check_little_endian()) {
		return cpu_to_le16(x);
	}
}

but how would you setup the checking ?

This would also work well for when you have a filesystem image on say an
x86 machine. I found some sites that recommend using jffs2dump to convert the
endianness before you can mount it. You most likely wouldn't change the kernel
in that case, or wouldn't want to.

Daniel
Richard Weinberger Nov. 2, 2018, 8:15 p.m. UTC | #6
On Fri, Nov 2, 2018 at 3:04 PM Daniel Walker <danielwa@cisco.com> wrote:
> Do you have any thought on how to implement it ? I was think abstracting the
> endian functions in the nodelist.h

Yes, see what UFS does.

> static inline jint16_t cpu_to_je16(jint16_t x) {
>         if (check_big_endian()) {
>                 return cpu_to_be16(x);
>         } else if (check_little_endian()) {
>                 return cpu_to_le16(x);
>         }
> }
>
> but how would you setup the checking ?

Make it a mount option and store the endianness mode in the super block.
Daniel Walker (danielwa) Nov. 2, 2018, 9:14 p.m. UTC | #7
On Fri, Nov 02, 2018 at 09:15:06PM +0100, Richard Weinberger wrote:
> On Fri, Nov 2, 2018 at 3:04 PM Daniel Walker <danielwa@cisco.com> wrote:
> > Do you have any thought on how to implement it ? I was think abstracting the
> > endian functions in the nodelist.h
> 
> Yes, see what UFS does.
> 
> > static inline jint16_t cpu_to_je16(jint16_t x) {
> >         if (check_big_endian()) {
> >                 return cpu_to_be16(x);
> >         } else if (check_little_endian()) {
> >                 return cpu_to_le16(x);
> >         }
> > }
> >
> > but how would you setup the checking ?
> 
> Make it a mount option and store the endianness mode in the super block.

It's actually a mkfs option currently. I'm not sure how that factors in,

from the mkfs.jffs2 man page,

       -l, --little-endian
              Create a little-endian JFFS2 image.  Default is to make an image with the same endianness as the host.

       -b, --big-endian
              Create a big-endian JFFS2 image.  Default is to make an image with the same endianness as the host.


Daniel
Richard Weinberger Nov. 2, 2018, 9:22 p.m. UTC | #8
Am Freitag, 2. November 2018, 22:14:44 CET schrieb Daniel Walker:
> > Make it a mount option and store the endianness mode in the super block.
> 
> It's actually a mkfs option currently. I'm not sure how that factors in,
> 
> from the mkfs.jffs2 man page,
> 
>        -l, --little-endian
>               Create a little-endian JFFS2 image.  Default is to make an image with the same endianness as the host.
> 
>        -b, --big-endian
>               Create a big-endian JFFS2 image.  Default is to make an image with the same endianness as the host.

As long this setting is not stored in the filesystem itself, it is useless.
IIRC it just controls the endianness setting for mkfs.jffs2's
t32() and t16() macros.

That's why I think of a mount option like "force_endian=".

Thanks,
//richard
Nikunj Kela (nkela) Nov. 7, 2018, 1:09 a.m. UTC | #9
Thanks for input. I have posted another patch with mount option:

http://lists.infradead.org/pipermail/linux-mtd/2018-November/085252.html


On 11/2/18, 2:23 PM, "Richard Weinberger" <richard@nod.at> wrote:

    Am Freitag, 2. November 2018, 22:14:44 CET schrieb Daniel Walker:
    > > Make it a mount option and store the endianness mode in the super block.
    > 
    > It's actually a mkfs option currently. I'm not sure how that factors in,
    > 
    > from the mkfs.jffs2 man page,
    > 
    >        -l, --little-endian
    >               Create a little-endian JFFS2 image.  Default is to make an image with the same endianness as the host.
    > 
    >        -b, --big-endian
    >               Create a big-endian JFFS2 image.  Default is to make an image with the same endianness as the host.
    
    As long this setting is not stored in the filesystem itself, it is useless.
    IIRC it just controls the endianness setting for mkfs.jffs2's
    t32() and t16() macros.
    
    That's why I think of a mount option like "force_endian=".
    
    Thanks,
    //richard

Patch
diff mbox series

diff --git a/fs/jffs2/Kconfig b/fs/jffs2/Kconfig
index ad850c5bf2ca..86e93fbc9d74 100644
--- a/fs/jffs2/Kconfig
+++ b/fs/jffs2/Kconfig
@@ -182,3 +182,28 @@  config JFFS2_CMODE_FAVOURLZO
 	  decompression) at the expense of size.
 
 endchoice
+
+choice
+	prompt "JFFS2 endianness"
+	default JFFS2_NATIVE_ENDIAN
+	depends on JFFS2_FS
+	help
+	  You can set here the default endianness of JFFS2 from
+	  the available options. Do not touch if unsure.
+
+config JFFS2_NATIVE_ENDIAN
+	bool "native endian"
+	help
+	  Uses a native endian bytestream.
+
+config JFFS2_BIG_ENDIAN
+	bool "big endian"
+	help
+	  Uses a big endian bytestream.
+
+config JFFS2_LITTLE_ENDIAN
+	bool "little endian"
+	help
+	  Uses a little endian bytestream.
+
+endchoice
diff --git a/fs/jffs2/nodelist.h b/fs/jffs2/nodelist.h
index 0637271f3770..a1ebf04f217c 100644
--- a/fs/jffs2/nodelist.h
+++ b/fs/jffs2/nodelist.h
@@ -27,12 +27,10 @@ 
 #include "os-linux.h"
 #endif
 
-#define JFFS2_NATIVE_ENDIAN
-
 /* Note we handle mode bits conversion from JFFS2 (i.e. Linux) to/from
    whatever OS we're actually running on here too. */
 
-#if defined(JFFS2_NATIVE_ENDIAN)
+#if defined(CONFIG_JFFS2_NATIVE_ENDIAN)
 #define cpu_to_je16(x) ((jint16_t){x})
 #define cpu_to_je32(x) ((jint32_t){x})
 #define cpu_to_jemode(x) ((jmode_t){os_to_jffs2_mode(x)})
@@ -43,7 +41,7 @@ 
 #define je16_to_cpu(x) ((x).v16)
 #define je32_to_cpu(x) ((x).v32)
 #define jemode_to_cpu(x) (jffs2_to_os_mode((x).m))
-#elif defined(JFFS2_BIG_ENDIAN)
+#elif defined(CONFIG_JFFS2_BIG_ENDIAN)
 #define cpu_to_je16(x) ((jint16_t){cpu_to_be16(x)})
 #define cpu_to_je32(x) ((jint32_t){cpu_to_be32(x)})
 #define cpu_to_jemode(x) ((jmode_t){cpu_to_be32(os_to_jffs2_mode(x))})
@@ -54,7 +52,7 @@ 
 #define je16_to_cpu(x) (be16_to_cpu(x.v16))
 #define je32_to_cpu(x) (be32_to_cpu(x.v32))
 #define jemode_to_cpu(x) (be32_to_cpu(jffs2_to_os_mode((x).m)))
-#elif defined(JFFS2_LITTLE_ENDIAN)
+#elif defined(CONFIG_JFFS2_LITTLE_ENDIAN)
 #define cpu_to_je16(x) ((jint16_t){cpu_to_le16(x)})
 #define cpu_to_je32(x) ((jint32_t){cpu_to_le32(x)})
 #define cpu_to_jemode(x) ((jmode_t){cpu_to_le32(os_to_jffs2_mode(x))})