[PATCH 0/2] cramfs: support for other endianness

[PATCH 0/2] cramfs: support for other endianness

[Andi Drebes]

The following patchset enables cramfs to mount images that were
created on a machine whose endianness differs from the mounting
machine's one.

Tested on an i386 box.

A discussion about this feature can be found here:

http://marc.info/?l=linux-fsdevel&m=119438573309330&w=2

	Andi

[PATCH 0/2] cramfs: Add mount option "swapendian"

[Andi Drebes]

This patch introduces the mount option "swapendian" for cramfs.
When this option is set, cramfs is able to mount an image that
was created on a machine whose endianness differs from the mounting
machine's one.
If somebody tries to mount an image with another endianness but
forgets to set this option, cramfs will give a hint for it.

Signed-off-by: Andi Drebes <andi@programmierforen.de>
---
 fs/cramfs/inode.c             |  112 +++++++++++++++++++++++++++++++----------
 include/linux/cramfs_endian.h |   58 +++++++++++++++++++++
 include/linux/cramfs_fs_sb.h  |    1 +
 3 files changed, 144 insertions(+), 27 deletions(-)

diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
index 350680f..8da03b0 100644
--- a/fs/cramfs/inode.c
+++ b/fs/cramfs/inode.c
@@ -4,6 +4,10 @@
  * Copyright (C) 1999 Linus Torvalds.
  *
  * This file is released under the GPL.
+ *
+ * Changelog:
+ *	11/07 - Andi Drebes <andi@programmierforen.de>
+ *	Added mount option "swapendian"
  */
 
 /*
@@ -18,6 +22,7 @@
 #include <linux/string.h>
 #include <linux/blkdev.h>
 #include <linux/cramfs_fs.h>
+#include <linux/cramfs_endian.h>
 #include <linux/slab.h>
 #include <linux/cramfs_fs_sb.h>
 #include <linux/buffer_head.h>
@@ -157,19 +162,24 @@ static void *cramfs_read(struct super_block *sb, unsigned int offset, unsigned i
 	blocknr = offset >> PAGE_CACHE_SHIFT;
 	offset &= PAGE_CACHE_SIZE - 1;
 
-	/* Check if an existing buffer already has the data.. */
-	for (i = 0; i < READ_BUFFERS; i++) {
-		unsigned int blk_offset;
-
-		if (buffer_dev[i] != sb)
-			continue;
-		if (blocknr < buffer_blocknr[i])
-			continue;
-		blk_offset = (blocknr - buffer_blocknr[i]) << PAGE_CACHE_SHIFT;
-		blk_offset += offset;
-		if (blk_offset + len > BUFFER_SIZE)
-			continue;
-		return read_buffers[i] + blk_offset;
+	/* Caching is disabled if the filesystem's
+	   and the machine's endianness differ. */
+	if(likely(CRAMFS_SB(sb)->endian))
+	{
+		/* Check if an existing buffer already has the data.. */
+		for (i = 0; i < READ_BUFFERS; i++) {
+			unsigned int blk_offset;
+
+			if (buffer_dev[i] != sb)
+				continue;
+			if (blocknr < buffer_blocknr[i])
+				continue;
+			blk_offset = (blocknr - buffer_blocknr[i]) << PAGE_CACHE_SHIFT;
+			blk_offset += offset;
+			if (blk_offset + len > BUFFER_SIZE)
+				continue;
+			return read_buffers[i] + blk_offset;
+		}
 	}
 
 	devsize = mapping->host->i_size >> PAGE_CACHE_SHIFT;
@@ -246,6 +256,14 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
 		return -ENOMEM;
 	sb->s_fs_info = sbi;
 
+	/* assume the right endianness */
+	sbi->endian = 1;
+
+	/* Check mount options:
+	   Does the user want to mount an image with a different endianness? */
+	if(strcmp("swapendian", data) == 0)
+		sbi->endian = 0;
+
 	/* Invalidate the read buffers on mount: think disk change.. */
 	mutex_lock(&read_mutex);
 	for (i = 0; i < READ_BUFFERS; i++)
@@ -256,26 +274,49 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
 	mutex_unlock(&read_mutex);
 
 	/* Do sanity checks on the superblock */
-	if (super.magic != CRAMFS_MAGIC) {
-		/* check for wrong endianess */
-		if (super.magic == CRAMFS_MAGIC_WEND) {
-			if (!silent)
-				printk(KERN_ERR "cramfs: wrong endianess\n");
-			goto out;
-		}
-
+	if (super.magic != CRAMFS_MAGIC && super.magic != CRAMFS_MAGIC_WEND) {
 		/* check at 512 byte offset */
 		mutex_lock(&read_mutex);
 		memcpy(&super, cramfs_read(sb, 512, sizeof(super)), sizeof(super));
 		mutex_unlock(&read_mutex);
-		if (super.magic != CRAMFS_MAGIC) {
-			if (super.magic == CRAMFS_MAGIC_WEND && !silent)
-				printk(KERN_ERR "cramfs: wrong endianess\n");
-			else if (!silent)
+
+		if (super.magic == CRAMFS_MAGIC_WEND) {
+			goto other_endian;
+		}
+		else if (super.magic != CRAMFS_MAGIC) {
+			if (!silent)
 				printk(KERN_ERR "cramfs: wrong magic\n");
+
 			goto out;
 		}
 	}
+	/* check for wrong endianess */
+	else if (super.magic == CRAMFS_MAGIC_WEND)
+	{
+other_endian:
+		if (sbi->endian) {
+			if (!silent) {
+				printk(KERN_ERR "cramfs: it seems as if you were trying to mount a filesystem "
+					"whose endianness does not match the host's one. You might want to try "
+					"the option \"swapendian\" when mounting the filesystem.\n");
+				printk(KERN_ERR "cramfs: the filesystem will not be mounted.\n");
+			}
+			goto out;
+		}
+		else {
+			if (!sbi->endian) {
+				if (!silent)
+					printk(KERN_INFO "cramfs: mounting cramfs with another endianness\n");
+				CRAMFS_CONVERT_SUPER(super);
+			}
+		}
+	}
+	else if (super.magic == CRAMFS_MAGIC && !sbi->endian)
+	{
+		printk(KERN_ERR "cramfs: you are trying to mount a filesystem whose endianness matches the "
+			"host's one. Do not use the option \"swapendian\".\n");
+		goto out;
+	}
 
 	/* get feature flags first */
 	if (super.flags & ~CRAMFS_SUPPORTED_FLAGS) {
@@ -352,6 +393,7 @@ static int cramfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
 	char *buf;
 	unsigned int offset;
 	int copied;
+	unsigned long endian = CRAMFS_SB(filp->f_path.dentry->d_inode->i_sb)->endian;
 
 	/* Offset within the thing. */
 	offset = filp->f_pos;
@@ -377,6 +419,8 @@ static int cramfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
 		mutex_lock(&read_mutex);
 		de = cramfs_read(sb, OFFSET(inode) + offset, sizeof(*de)+CRAMFS_MAXPATHLEN);
 		name = (char *)(de+1);
+		if(unlikely(!endian))
+			CRAMFS_CONVERT_INODE(de);
 
 		/*
 		 * Namelengths on disk are shifted by two
@@ -417,8 +461,10 @@ static struct dentry * cramfs_lookup(struct inode *dir, struct dentry *dentry, s
 {
 	unsigned int offset = 0;
 	int sorted;
+	unsigned long endian;
 
 	mutex_lock(&read_mutex);
+	endian = CRAMFS_SB(dir->i_sb)->endian;
 	sorted = CRAMFS_SB(dir->i_sb)->flags & CRAMFS_FLAG_SORTED_DIRS;
 	while (offset < dir->i_size) {
 		struct cramfs_inode *de;
@@ -427,6 +473,8 @@ static struct dentry * cramfs_lookup(struct inode *dir, struct dentry *dentry, s
 
 		de = cramfs_read(dir->i_sb, OFFSET(dir) + offset, sizeof(*de)+CRAMFS_MAXPATHLEN);
 		name = (char *)(de+1);
+		if(unlikely(!endian))
+			CRAMFS_CONVERT_INODE(de);
 
 		/* Try to take advantage of sorted directories */
 		if (sorted && (dentry->d_name.name[0] < name[0]))
@@ -473,6 +521,7 @@ static int cramfs_readpage(struct file *file, struct page * page)
 	struct inode *inode = page->mapping->host;
 	u32 maxblock, bytes_filled;
 	void *pgdata;
+	unsigned long endian = CRAMFS_SB(inode->i_sb)->endian;
 
 	maxblock = (inode->i_size + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
 	bytes_filled = 0;
@@ -483,9 +532,18 @@ static int cramfs_readpage(struct file *file, struct page * page)
 
 		start_offset = OFFSET(inode) + maxblock*4;
 		mutex_lock(&read_mutex);
-		if (page->index)
+		if (page->index) {
 			start_offset = *(u32 *) cramfs_read(sb, blkptr_offset-4, 4);
-		compr_len = (*(u32 *) cramfs_read(sb, blkptr_offset, 4) - start_offset);
+
+			if(unlikely(!endian))
+				start_offset = CPU_ENDIAN_32(start_offset);
+		}
+
+		if(unlikely(!endian))
+			compr_len = CPU_ENDIAN_32(*(u32 *) cramfs_read(sb, blkptr_offset, 4)) - start_offset;
+		else
+			compr_len = (*(u32 *) cramfs_read(sb, blkptr_offset, 4) - start_offset);
+
 		mutex_unlock(&read_mutex);
 		pgdata = kmap(page);
 		if (compr_len == 0)
diff --git a/include/linux/cramfs_endian.h b/include/linux/cramfs_endian.h
new file mode 100644
index 0000000..9b90b26
--- /dev/null
+++ b/include/linux/cramfs_endian.h
@@ -0,0 +1,58 @@
+/*
+ * cramfs_endian.h provides definitions used when mounting
+ * a cram filesystem whose endianness doesn't match the host's
+ * one.
+ *
+ * Copyright 2007 (C) Andi Drebes <andi@programmierforen.de>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#ifndef __CRAMFS_ENDIAN_H
+#define __CRAMFS_ENDIAN_H
+
+#ifdef __LITTLE_ENDIAN
+	#define CPU_ENDIAN_32(x) (be32_to_cpu(x))
+	#define CPU_ENDIAN_16(x) (be16_to_cpu(x))
+#elif defined __BIG_ENDIAN
+	#define CPU_ENDIAN_32(x) (le32_to_cpu(x))
+	#define CPU_ENDIAN_16(x) (le16_to_cpu(x))
+#else
+	#error "Neither __LITTLE_ENDIAN nor __BIG_ENDIAN is defined!"
+#endif
+
+/* Converts a cramfs_info from the wrong endianess
+   to host endianess. */
+#define CRAMFS_CONVERT_INFO(info) \
+	do { \
+		(info).crc = CPU_ENDIAN_32((info).crc); \
+		(info).edition = CPU_ENDIAN_32((info).edition); \
+		(info).blocks = CPU_ENDIAN_32((info).blocks); \
+		(info).files = CPU_ENDIAN_32((info).files); \
+	} while(0)
+
+/* Converts a cramfs_info from the wrong endianess
+   to host endianess. */
+#define CRAMFS_CONVERT_INODE(inode) \
+	do { \
+		__u8* ptr = (__u8*)(inode);\
+		(inode)->mode = CPU_ENDIAN_16((inode)->mode); \
+		(inode)->uid = CPU_ENDIAN_16((inode)->uid); \
+		(inode)->size = (ptr[4] << 16) | (ptr[5] << 8) | (ptr[6]) ; \
+		(inode)->offset = ((ptr[8] & 0x03) << 24) | (ptr[9] << 16) | (ptr[10] << 8) | ptr[11]; \
+		(inode)->namelen = (ptr[8] & 0x3f) >> 2; \
+	} while(0)
+
+/* Converts a cramfs superblock from the wrong endianess
+   to host endianess. */
+#define CRAMFS_CONVERT_SUPER(super) \
+	do { \
+		(super).magic = CPU_ENDIAN_32((super).magic); \
+		(super).size = CPU_ENDIAN_32((super).size); \
+		(super).flags = CPU_ENDIAN_32((super).flags); \
+		(super).future = CPU_ENDIAN_32((super).future); \
+		CRAMFS_CONVERT_INFO((super).fsid); \
+		CRAMFS_CONVERT_INODE(&(super).root); \
+	} while(0)
+
+#endif //__CRAMFS_ENDIAN_H
diff --git a/include/linux/cramfs_fs_sb.h b/include/linux/cramfs_fs_sb.h
index 8390693..dda8e09 100644
--- a/include/linux/cramfs_fs_sb.h
+++ b/include/linux/cramfs_fs_sb.h
@@ -10,6 +10,7 @@ struct cramfs_sb_info {
 			unsigned long blocks;
 			unsigned long files;
 			unsigned long flags;
+			unsigned long endian; /* 1: host endian */
 };
 
 static inline struct cramfs_sb_info *CRAMFS_SB(struct super_block *sb)

[PATCH 0/2] cramfs: update README file

[Andi Drebes]

This patch updates the readme file according to the
new mount option "swapendian".

Signed-off-by: Andi Drebes <andi@programmierforen.de>
---
 fs/cramfs/README |   21 +++++++++------------
 1 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/fs/cramfs/README b/fs/cramfs/README
index 445d1c2..acbbdc4 100644
--- a/fs/cramfs/README
+++ b/fs/cramfs/README
@@ -1,3 +1,12 @@
+Mount options
+-------------
+
+Currently, there is only one mount option available for cramfs:
+
+swapendian:
+	causes the filesystem's metadata to be converted from
+	non-host-endianness to host-endianness.
+
 Notes on Filesystem Layout
 --------------------------
 
@@ -108,18 +117,6 @@ kernels, not even necessarily kernels of the same architecture if
 PAGE_CACHE_SIZE is subject to change between kernel versions
 (currently possible with arm and ia64).
 
-The remaining options try to make cramfs more sharable.
-
-One part of that is addressing endianness.  The two options here are
-`always use little-endian' (like ext2fs) or `writer chooses
-endianness; kernel adapts at runtime'.  Little-endian wins because of
-code simplicity and little CPU overhead even on big-endian machines.
-
-The cost of swabbing is changing the code to use the le32_to_cpu
-etc. macros as used by ext2fs.  We don't need to swab the compressed
-data, only the superblock, inodes and block pointers.
-
-
 The other part of making cramfs more sharable is choosing a block
 size.  The options are:
 

Re: [PATCH 0/2] cramfs: support for other endianness

[Andi Drebes]

I checked everything twice, except the patch numbers.
Sorry about that.

The right version of the patchset should look like this:

[PATCH 1/2] cramfs: Add mount option "swapendian"
[PATCH 2/2] cramfs: update README file

	Andi

Re: [PATCH 0/2] cramfs: Add mount option "swapendian"

[Linus Torvalds]

On Thu, 15 Nov 2007, Andi Drebes wrote:
>
> This patch introduces the mount option "swapendian" for cramfs.
> When this option is set, cramfs is able to mount an image that
> was created on a machine whose endianness differs from the mounting
> machine's one.

Please don't do it this way.

It would be *much* better to just standardize on one endianness, and be 
done with it. That way there are no config options, no confusion, and the 
code is smaller, simpler, and faster. Because nn unconditional byte swap 
is generally faster than a conditional non-byte-swap!

So can you please just make it little-endian? 

There can't be that many big-endian machines that really care about old 
cramfs images..

		Linus

Re: [PATCH 0/2] cramfs: Add mount option "swapendian"

[Linus Torvalds]

On Thu, 15 Nov 2007, Linus Torvalds wrote:
> 
> It would be *much* better to just standardize on one endianness, and be 
> done with it. That way there are no config options, no confusion, and the 
> code is smaller, simpler, and faster.

.. it's also statically checkable with tools like "sparse", so it avoids 
bugs not only by being simpler, but by simply being fundamentally more 
robust to start with.

		Linus

Re: [PATCH 0/2] cramfs: Add mount option "swapendian"

[Christoph Hellwig]

On Thu, Nov 15, 2007 at 12:45:20PM -0800, Linus Torvalds wrote:
> Please don't do it this way.
> 
> It would be *much* better to just standardize on one endianness, and be 
> done with it. That way there are no config options, no confusion, and the 
> code is smaller, simpler, and faster. Because nn unconditional byte swap 
> is generally faster than a conditional non-byte-swap!
> 
> So can you please just make it little-endian? 
> 
> There can't be that many big-endian machines that really care about old 
> cramfs images..

Actually there are as lots of initrd are cramfs.  This means you'd need
to update mkcramfs all big endian machines out there.

Re: [PATCH 0/2] cramfs: Add mount option "swapendian"

[Christoph Hellwig]

On Thu, Nov 15, 2007 at 12:46:11PM -0800, Linus Torvalds wrote:
> 
> 
> On Thu, 15 Nov 2007, Linus Torvalds wrote:
> > 
> > It would be *much* better to just standardize on one endianness, and be 
> > done with it. That way there are no config options, no confusion, and the 
> > code is smaller, simpler, and faster.
> 
> .. it's also statically checkable with tools like "sparse", so it avoids 
> bugs not only by being simpler, but by simply being fundamentally more 
> robust to start with.

We can do proper typechecking with sparse even for dual-endian filesystems,
see ufs and sysvfs for example.  And yes, for a new filesystems I'd always
chose one endianes and stick to it, but becase I certain someone forgot
that when creating cramfs we're stuck now :)

Re: [PATCH 0/2] cramfs: Add mount option "swapendian"

[Linus Torvalds]

On Thu, 15 Nov 2007, Christoph Hellwig wrote:
> 
> Actually there are as lots of initrd are cramfs.  This means you'd need
> to update mkcramfs all big endian machines out there.

So? Normally the initrd goes with the kernel anyway.

		Linus

Re: [PATCH 0/2] cramfs: Add mount option "swapendian"

[Jörn Engel]

On Thu, 15 November 2007 21:35:16 +0100, Andi Drebes wrote:
> +	/* Caching is disabled if the filesystem's
> +	   and the machine's endianness differ. */
> +	if(likely(CRAMFS_SB(sb)->endian))
> +	{

Codingstyle: extra space after "if", keep brace on the same line.
Same goes for most of this patch.

Unlikely not likely to be a good idea.  It clutters up the code for a
minimal gain on host-endian filesystems (and I doubt you can measure
that even in micro-benchmarks) and forcibly mispredicts every branch for
cross-endian filesystems.

The name "endian" could be replaces with something more descriptive.
"host_endian" or "swap_endian" with reversed logic or something.

> +	/* check for wrong endianess */
> +	else if (super.magic == CRAMFS_MAGIC_WEND)
> +	{
> +other_endian:
> +		if (sbi->endian) {
> +			if (!silent) {
> +				printk(KERN_ERR "cramfs: it seems as if you were trying to mount a filesystem "
> +					"whose endianness does not match the host's one. You might want to try "
> +					"the option \"swapendian\" when mounting the filesystem.\n");
> +				printk(KERN_ERR "cramfs: the filesystem will not be mounted.\n");
> +			}
> +			goto out;
> +		}
> +		else {
> +			if (!sbi->endian) {
> +				if (!silent)
> +					printk(KERN_INFO "cramfs: mounting cramfs with another endianness\n");
> +				CRAMFS_CONVERT_SUPER(super);
> +			}
> +		}
> +	}
> +	else if (super.magic == CRAMFS_MAGIC && !sbi->endian)
> +	{
> +		printk(KERN_ERR "cramfs: you are trying to mount a filesystem whose endianness matches the "
> +			"host's one. Do not use the option \"swapendian\".\n");
> +		goto out;
> +	}

You could remove most of this by removing the mount option and simply
deciding endianness based on super.magic.  So why the extra work?

> @@ -483,9 +532,18 @@ static int cramfs_readpage(struct file *file, struct page * page)
>  
>  		start_offset = OFFSET(inode) + maxblock*4;
>  		mutex_lock(&read_mutex);
> -		if (page->index)
> +		if (page->index) {
>  			start_offset = *(u32 *) cramfs_read(sb, blkptr_offset-4, 4);
> -		compr_len = (*(u32 *) cramfs_read(sb, blkptr_offset, 4) - start_offset);
> +
> +			if(unlikely(!endian))
> +				start_offset = CPU_ENDIAN_32(start_offset);
> +		}
> +
> +		if(unlikely(!endian))
> +			compr_len = CPU_ENDIAN_32(*(u32 *) cramfs_read(sb, blkptr_offset, 4)) - start_offset;
> +		else
> +			compr_len = (*(u32 *) cramfs_read(sb, blkptr_offset, 4) - start_offset);

The two conditional can be combined into one.

> diff --git a/include/linux/cramfs_endian.h b/include/linux/cramfs_endian.h
> new file mode 100644
> index 0000000..9b90b26
> --- /dev/null
> +++ b/include/linux/cramfs_endian.h
> @@ -0,0 +1,58 @@
> +/*
> + * cramfs_endian.h provides definitions used when mounting
> + * a cram filesystem whose endianness doesn't match the host's
> + * one.
> + *
> + * Copyright 2007 (C) Andi Drebes <andi@programmierforen.de>
> + *
> + * This file is released under the GPLv2.
> + */
> +
> +#ifndef __CRAMFS_ENDIAN_H
> +#define __CRAMFS_ENDIAN_H
> +
> +#ifdef __LITTLE_ENDIAN
> +	#define CPU_ENDIAN_32(x) (be32_to_cpu(x))
> +	#define CPU_ENDIAN_16(x) (be16_to_cpu(x))
> +#elif defined __BIG_ENDIAN
> +	#define CPU_ENDIAN_32(x) (le32_to_cpu(x))
> +	#define CPU_ENDIAN_16(x) (le16_to_cpu(x))
> +#else
> +	#error "Neither __LITTLE_ENDIAN nor __BIG_ENDIAN is defined!"
> +#endif

You're using Xe32_to_cpu on host-endian numbers?  Sparse won't be happy.
Better just use swab32 directly.

> +
> +/* Converts a cramfs_info from the wrong endianess
> +   to host endianess. */
> +#define CRAMFS_CONVERT_INFO(info) \
> +	do { \
> +		(info).crc = CPU_ENDIAN_32((info).crc); \
> +		(info).edition = CPU_ENDIAN_32((info).edition); \
> +		(info).blocks = CPU_ENDIAN_32((info).blocks); \
> +		(info).files = CPU_ENDIAN_32((info).files); \
> +	} while(0)

Better make it a static inline function for type safety.  Possibly even
an out-of-line function.

> +/* Converts a cramfs_info from the wrong endianess
> +   to host endianess. */
> +#define CRAMFS_CONVERT_INODE(inode) \
> +	do { \
> +		__u8* ptr = (__u8*)(inode);\
> +		(inode)->mode = CPU_ENDIAN_16((inode)->mode); \
> +		(inode)->uid = CPU_ENDIAN_16((inode)->uid); \
> +		(inode)->size = (ptr[4] << 16) | (ptr[5] << 8) | (ptr[6]) ; \
> +		(inode)->offset = ((ptr[8] & 0x03) << 24) | (ptr[9] << 16) | (ptr[10] << 8) | ptr[11]; \
> +		(inode)->namelen = (ptr[8] & 0x3f) >> 2; \
> +	} while(0)
> +
> +/* Converts a cramfs superblock from the wrong endianess
> +   to host endianess. */
> +#define CRAMFS_CONVERT_SUPER(super) \
> +	do { \
> +		(super).magic = CPU_ENDIAN_32((super).magic); \
> +		(super).size = CPU_ENDIAN_32((super).size); \
> +		(super).flags = CPU_ENDIAN_32((super).flags); \
> +		(super).future = CPU_ENDIAN_32((super).future); \
> +		CRAMFS_CONVERT_INFO((super).fsid); \
> +		CRAMFS_CONVERT_INODE(&(super).root); \
> +	} while(0)

dito.

Jörn

-- 
The rabbit runs faster than the fox, because the rabbit is rinning for
his life while the fox is only running for his dinner.
-- Aesop
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] cramfs: Add mount option "swapendian"

[Andi Drebes]

> > This patch introduces the mount option "swapendian" for cramfs.
> > When this option is set, cramfs is able to mount an image that
> > was created on a machine whose endianness differs from the mounting
> > machine's one.
> 
> Please don't do it this way.
> 
> It would be *much* better to just standardize on one endianness, and be 
> done with it. That way there are no config options, no confusion, and the 
> code is smaller, simpler, and faster. Because nn unconditional byte swap 
> is generally faster than a conditional non-byte-swap!
This is a valid objection. The problem is that the endianness for cramfs
has never been standardized. Now there are filesystem images in both little
and big endian out there. I encountered this problem first when I tried to
mount my router's initrd. Yes, I know, I could have converted the image into
little endian. I just find that it is more consistent to support both kinds
of endianness.

> So can you please just make it little-endian? 
Actually, in my eyes, it would be better to create a new version of cramfs
that standardizes the endianness and the block size. But that doesn't solve
the problems one might have with old images.

> There can't be that many big-endian machines that really care about old 
> cramfs images..
s.a. (There's at least one...)

	Andi

Re: [PATCH 0/2] cramfs: Add mount option "swapendian"

[Linus Torvalds]

On Thu, 15 Nov 2007, Andi Drebes wrote:
>
> Actually, in my eyes, it would be better to create a new version of cramfs
> that standardizes the endianness and the block size.

Perhaps even more importantly, you can do a much better job.

The thing is, when I did cramfs originally, I had a total "senior moment", 
and the reason I didn't compress the metadata was that it appeared hard to 
do and fill it in later (ie compressing the metadata would mean that the 
location of the data changes - and since the metadata obviously contains 
pointers to the data, there's a chicken-and-egg problem there).

But it should be *trivial* to compress the metadata too if the code just 
were to put the metadata at the beginning of the image, and the real data 
at the end, and then you can build up the image from both ends and you 
*can* have a fixed starting point for the data (up at the top of the 
image) even though you are changing the size of the metadata by 
compression.

But I literally designed and wrote the thing in a couple of days, and I 
really didn't think it through right. As a result, the metadata may be 
dense, but it's totally uncompressed. It would have been better to allow a 
less dense basic format (allowing bigger uid/gid values, and offsets and 
file sizes), but compress it.

So a "v2" cramfs would be a great idea. Not just for fixing endianness 
etc.

		Linus

Re: [PATCH 0/2] cramfs: Add mount option "swapendian"

[Phillip Lougher]

Linus Torvalds wrote:
> 
> 
> But it should be *trivial* to compress the metadata too if the code just 
> were to put the metadata at the beginning of the image, and the real data 
> at the end, and then you can build up the image from both ends and you 
> *can* have a fixed starting point for the data (up at the top of the 
> image) even though you are changing the size of the metadata by 
> compression.
> 

I decided to compress the metadata when I designed Squashfs, a read-only 
filesystem which was inspired by Cramfs. Squashfs stores the data at the 
front of the filesystem and puts the metadata at the end, so the data is 
always at a fixed point.  Doing that and a couple of other things allows 
the metadata to be built up and compressed in one-pass while the 
filesystem is being created.  The metadata is split into an inode table 
and a directory table and compressed separately because it compresses 
better than way.

> But I literally designed and wrote the thing in a couple of days, and I 
> really didn't think it through right. As a result, the metadata may be 
> dense, but it's totally uncompressed. It would have been better to allow a 
> less dense basic format (allowing bigger uid/gid values, and offsets and 
> file sizes), but compress it.
> 

Squashfs stores much more metadata information, but as it is compressed 
it is much smaller than Cramfs.  Typically the inode table compresses
to less than 40% and the directory table to less than 50%.


> So a "v2" cramfs would be a great idea.

That is what I always considered Squashfs to be.  But I also made the 
mistake of making Squashfs both little and big endian.  That's going to
be fixed and then I'll make a second attempt at submitting it for
inclusion in the mainline kernel.

Phillip

Re: [PATCH 0/2] cramfs: Add mount option "swapendian"

[Andi Drebes]

Am Donnerstag, 15. November 2007 23:48 schrieb Phillip Lougher:
> > So a "v2" cramfs would be a great idea.
> 
> That is what I always considered Squashfs to be.  But I also made the 
> mistake of making Squashfs both little and big endian.  That's going to
> be fixed and then I'll make a second attempt at submitting it for
> inclusion in the mainline kernel.
I didn't have a closer look on squashfs, but I am going to have it this weekend.
So far, it seems as if no "cramfs v2" is needed. My proposal is the following:

I'll write a new patch for inclusion in the mainline kernel that makes cramfs
"little endian only". For people who really want to be able to mount filesystems
with both kinds of endianness there will be a seperate patch (not intended to be
merged into mainline) available somewhere on the net (my website or so). It will
definitely be marked as non-official in order to prevent people from creating
images in big endian. Officially, cramfs should only support little endian images.
If squashfs will be merged, cramfs should be marked as obsolete.

	Andi

Re: [PATCH 0/2] cramfs: Add mount option "swapendian"

[Linus Torvalds]

On Fri, 16 Nov 2007, Andi Drebes wrote:
> 
> I'll write a new patch for inclusion in the mainline kernel that makes 
> cramfs "little endian only". For people who really want to be able to 
> mount filesystems with both kinds of endianness there will be a seperate 
> patch (not intended to be merged into mainline) available somewhere on 
> the net (my website or so). It will definitely be marked as non-official 
> in order to prevent people from creating images in big endian. 
> Officially, cramfs should only support little endian images.

Good. We do actually have precedence for exactly this kind of situation 
before: it's what happened to ext2 as well (people were trying to push a 
switch-endian version, and I said no) and the m68k people had their own 
patches for a while but we're *so* much better off from being fixed-endian 
that it's not even funny.

> If squashfs will be merged, cramfs should be marked as obsolete.

Sounds like that, yes.

		Linus