linux-embedded.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/4] cramfs refresh for embedded usage
@ 2017-10-12  6:16 Nicolas Pitre
  2017-10-12  6:16 ` [PATCH v6 1/4] cramfs: direct memory access support Nicolas Pitre
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Nicolas Pitre @ 2017-10-12  6:16 UTC (permalink / raw)
  To: Alexander Viro, Christoph Hellwig
  Cc: linux-fsdevel, linux-mm, linux-embedded, linux-kernel, Chris Brandt

This series brings a nice refresh to the cramfs filesystem, adding the
following capabilities:

- Direct memory access, bypassing the block and/or MTD layers entirely.

- Ability to store individual data blocks uncompressed.

- Ability to locate individual data blocks anywhere in the filesystem.

The end result is a very tight filesystem that can be accessed directly
from ROM without any other subsystem underneath. This also allows for
user space XIP which is a very important feature for tiny embedded
systems.

This series is also available based on v4.14-rc3 via git here:

  http://git.linaro.org/people/nicolas.pitre/linux xipcramfs

Why cramfs?

  Because cramfs is very simple and small. With CONFIG_CRAMFS_BLOCK=n and
  CONFIG_CRAMFS_PHYSMEM=y the cramfs driver may use as little as 3704 bytes
  of code. That's many times smaller than squashfs. And the runtime memory
  usage is also much less with cramfs than squashfs. It packs very tightly
  already compared to romfs which has no compression support. And the cramfs
  format was simple to extend, allowing for both compressed and uncompressed
  blocks within the same file.

Does it use MTD or not?

  The MTD layer is used to get at the actual physical/virtual address
  for the filesystem image. The underlying MTD device (or a partition
  based on it) specified as mount argument must use a driver that
  implements the mtd->_point method. Once mtd_point() is successful,
  all accesses are performed directly in memory and the MTD layer is
  no longer involved. Patches adding point support to a few more MTD
  drivers can be found here:

    http://git.linaro.org/people/nicolas.pitre/linux mtd_point

Why not using DAX?

  DAX stands for "Direct Access" and is a generic kernel layer helping
  with the necessary tasks involved with XIP. It is tailored for large
  writable filesystems and relies on the presence of an MMU. It also has
  the following shortcoming: "The DAX code does not work correctly on
  architectures which have virtually mapped caches such as ARM, MIPS and
  SPARC." That makes it unsuitable for a large portion of the intended
  targets for this series. And due to the read-only nature of cramfs, it is
  possible to achieve the intended result with a much simpler approach making
  DAX somewhat overkill in this context.

The maximum size of a cramfs image can't exceed 272MB. In practice it is
likely to be much much less. Given this series is concerned with small
memory systems, even in the MMU case there is always plenty of vmalloc
space left to map it all and even a 272MB memremap() wouldn't be a
problem. If it is then maybe your system is big enough with large
resources to manage already and you're pretty unlikely to be using cramfs
in the first place.

Of course, while this cramfs remains backward compatible with existing
filesystem images, a newer mkcramfs version is necessary to take advantage
of the extended data layout. I created a version of mkcramfs that
detects ELF files and marks text+rodata segments for XIP and compresses the
rest of those ELF files automatically.

So here it is. I'm also willing to step up as cramfs maintainer given
that no sign of any maintenance activities appeared for years.


Changes from v5:

- Switch to MTD for obtaining the cramfs image memory address rather than
  accepting a physical address directly as a mount argument.
- Drop the patch for making root mount possible as the MTD mount case is
  already supported and that covers our usage.
- There is no longer a separate filesystem type. It is "cramfs" for both
  blockdev based and MTD/memory based accesses.
- Fix NULL deref in cramfs_statfs() when using direct memory access.

Changes from v4:

- Remove fault handler with vma splitting code in favor of VM_MIXEDMAP for
  much simpler code. Thanks to Christoph Hellwig for review and suggestions.
- Additional code cleanups, mostly from Christoph's suggestions.

Changes from v3:

- Rebased on v4.13.
- Made direct access depend on cramfs not being modular due to unexported
  vma handling functions.
- Solicit comments from mm people explicitly.

Changes from v2:

- Plugged a few races in cramfs_vmasplit_fault(). Thanks to Al Viro for
  highlighting them.
- Fixed some checkpatch warnings

Changes from v1:

- Improved mmap() support by adding the ability to partially populate a
  mapping and lazily split the non directly mapable pages to a separate
  vma at fault time (thanks to Chris Brandt for testing).
- Clarified the documentation some more.


diffstat:

 Documentation/filesystems/cramfs.txt |  42 +++
 MAINTAINERS                          |   4 +-
 fs/cramfs/Kconfig                    |  39 ++-
 fs/cramfs/README                     |  31 +-
 fs/cramfs/inode.c                    | 514 +++++++++++++++++++++++++----
 include/uapi/linux/cramfs_fs.h       |  26 +-
 6 files changed, 587 insertions(+), 69 deletions(-)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v6 1/4] cramfs: direct memory access support
  2017-10-12  6:16 [PATCH v6 0/4] cramfs refresh for embedded usage Nicolas Pitre
@ 2017-10-12  6:16 ` Nicolas Pitre
  2017-10-12 17:03   ` Chris Brandt
                     ` (2 more replies)
  2017-10-12  6:16 ` [PATCH v6 2/4] cramfs: implement uncompressed and arbitrary data block positioning Nicolas Pitre
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 17+ messages in thread
From: Nicolas Pitre @ 2017-10-12  6:16 UTC (permalink / raw)
  To: Alexander Viro, Christoph Hellwig
  Cc: linux-fsdevel, linux-mm, linux-embedded, linux-kernel, Chris Brandt

Small embedded systems typically execute the kernel code in place (XIP)
directly from flash to save on precious RAM usage. This adds the ability
to consume filesystem data directly from flash to the cramfs filesystem
as well. Cramfs is particularly well suited to this feature as it is
very simple and its RAM usage is already very low, and with this feature
it is possible to use it with no block device support and even lower RAM
usage.

This patch was inspired by a similar patch from Shane Nay dated 17 years
ago that used to be very popular in embedded circles but never made it
into mainline. This is a cleaned-up implementation that uses far fewer
ifdef's and gets the actual memory location for the filesystem image
via MTD at run time. In the context of small IoT deployments, this
functionality has become relevant and useful again.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
 fs/cramfs/Kconfig |  30 +++++++-
 fs/cramfs/inode.c | 215 +++++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 202 insertions(+), 43 deletions(-)

diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
index 11b29d491b..ef86b06bc0 100644
--- a/fs/cramfs/Kconfig
+++ b/fs/cramfs/Kconfig
@@ -1,6 +1,5 @@
 config CRAMFS
 	tristate "Compressed ROM file system support (cramfs) (OBSOLETE)"
-	depends on BLOCK
 	select ZLIB_INFLATE
 	help
 	  Saying Y here includes support for CramFs (Compressed ROM File
@@ -20,3 +19,32 @@ config CRAMFS
 	  in terms of performance and features.
 
 	  If unsure, say N.
+
+config CRAMFS_BLOCKDEV
+	bool "Support CramFs image over a regular block device" if EXPERT
+	depends on CRAMFS && BLOCK
+	default y
+	help
+	  This option allows the CramFs driver to load data from a regular
+	  block device such a disk partition or a ramdisk.
+
+config CRAMFS_MTD
+	bool "Support CramFs image directly mapped in physical memory"
+	depends on CRAMFS && MTD
+	default y if !CRAMFS_BLOCKDEV
+	help
+	  This option allows the CramFs driver to load data directly from
+	  a linear adressed memory range (usually non volatile memory
+	  like flash) instead of going through the block device layer.
+	  This saves some memory since no intermediate buffering is
+	  necessary.
+
+	  The location of the CramFs image is determined by a
+	  MTD device capable of direct memory mapping e.g. from
+	  the 'physmap' map driver or a resulting MTD partition.
+	  For example, this would mount the cramfs image stored in
+	  the MTD partition named "xip_fs" on the /mnt mountpoint:
+
+	  mount -t cramfs mtd:xip_fs /mnt
+
+	  If unsure, say N.
diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
index 7919967488..321a1fe17e 100644
--- a/fs/cramfs/inode.c
+++ b/fs/cramfs/inode.c
@@ -19,6 +19,8 @@
 #include <linux/init.h>
 #include <linux/string.h>
 #include <linux/blkdev.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/super.h>
 #include <linux/slab.h>
 #include <linux/vfs.h>
 #include <linux/mutex.h>
@@ -36,6 +38,9 @@ struct cramfs_sb_info {
 	unsigned long blocks;
 	unsigned long files;
 	unsigned long flags;
+	void *linear_virt_addr;
+	resource_size_t linear_phys_addr;
+	size_t mtd_point_size;
 };
 
 static inline struct cramfs_sb_info *CRAMFS_SB(struct super_block *sb)
@@ -140,6 +145,9 @@ static struct inode *get_cramfs_inode(struct super_block *sb,
  * BLKS_PER_BUF*PAGE_SIZE, so that the caller doesn't need to
  * worry about end-of-buffer issues even when decompressing a full
  * page cache.
+ *
+ * Note: This is all optimized away at compile time when
+ *       CONFIG_CRAMFS_BLOCKDEV=n.
  */
 #define READ_BUFFERS (2)
 /* NEXT_BUFFER(): Loop over [0..(READ_BUFFERS-1)]. */
@@ -160,10 +168,10 @@ static struct super_block *buffer_dev[READ_BUFFERS];
 static int next_buffer;
 
 /*
- * Returns a pointer to a buffer containing at least LEN bytes of
- * filesystem starting at byte offset OFFSET into the filesystem.
+ * Populate our block cache and return a pointer from it.
  */
-static void *cramfs_read(struct super_block *sb, unsigned int offset, unsigned int len)
+static void *cramfs_blkdev_read(struct super_block *sb, unsigned int offset,
+				unsigned int len)
 {
 	struct address_space *mapping = sb->s_bdev->bd_inode->i_mapping;
 	struct page *pages[BLKS_PER_BUF];
@@ -239,11 +247,52 @@ static void *cramfs_read(struct super_block *sb, unsigned int offset, unsigned i
 	return read_buffers[buffer] + offset;
 }
 
+/*
+ * Return a pointer to the linearly addressed cramfs image in memory.
+ */
+static void *cramfs_direct_read(struct super_block *sb, unsigned int offset,
+				unsigned int len)
+{
+	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
+
+	if (!len)
+		return NULL;
+	if (len > sbi->size || offset > sbi->size - len)
+		return page_address(ZERO_PAGE(0));
+	return sbi->linear_virt_addr + offset;
+}
+
+/*
+ * Returns a pointer to a buffer containing at least LEN bytes of
+ * filesystem starting at byte offset OFFSET into the filesystem.
+ */
+static void *cramfs_read(struct super_block *sb, unsigned int offset,
+			 unsigned int len)
+{
+	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
+
+	if (IS_ENABLED(CONFIG_CRAMFS_MTD) && sbi->linear_virt_addr)
+		return cramfs_direct_read(sb, offset, len);
+	else if (IS_ENABLED(CONFIG_CRAMFS_BLOCKDEV))
+		return cramfs_blkdev_read(sb, offset, len);
+	else
+		return NULL;
+}
+
 static void cramfs_kill_sb(struct super_block *sb)
 {
 	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
 
-	kill_block_super(sb);
+	if (IS_ENABLED(CCONFIG_CRAMFS_MTD)) {
+		if (sbi->mtd_point_size)
+			mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
+		if (sb->s_mtd)
+			kill_mtd_super(sb);
+	}
+	if (IS_ENABLED(CONFIG_CRAMFS_BLOCKDEV)) {
+		if (sb->s_bdev)
+			kill_block_super(sb);
+	}
 	kfree(sbi);
 }
 
@@ -254,34 +303,24 @@ static int cramfs_remount(struct super_block *sb, int *flags, char *data)
 	return 0;
 }
 
-static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
+static int cramfs_read_super(struct super_block *sb,
+			     struct cramfs_super *super, int silent)
 {
-	int i;
-	struct cramfs_super super;
+	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
 	unsigned long root_offset;
-	struct cramfs_sb_info *sbi;
-	struct inode *root;
-
-	sb->s_flags |= MS_RDONLY;
-
-	sbi = kzalloc(sizeof(struct cramfs_sb_info), GFP_KERNEL);
-	if (!sbi)
-		return -ENOMEM;
-	sb->s_fs_info = sbi;
 
-	/* Invalidate the read buffers on mount: think disk change.. */
-	mutex_lock(&read_mutex);
-	for (i = 0; i < READ_BUFFERS; i++)
-		buffer_blocknr[i] = -1;
+	/* We don't know the real size yet */
+	sbi->size = PAGE_SIZE;
 
 	/* Read the first block and get the superblock from it */
-	memcpy(&super, cramfs_read(sb, 0, sizeof(super)), sizeof(super));
+	mutex_lock(&read_mutex);
+	memcpy(super, cramfs_read(sb, 0, sizeof(*super)), sizeof(*super));
 	mutex_unlock(&read_mutex);
 
 	/* Do sanity checks on the superblock */
-	if (super.magic != CRAMFS_MAGIC) {
+	if (super->magic != CRAMFS_MAGIC) {
 		/* check for wrong endianness */
-		if (super.magic == CRAMFS_MAGIC_WEND) {
+		if (super->magic == CRAMFS_MAGIC_WEND) {
 			if (!silent)
 				pr_err("wrong endianness\n");
 			return -EINVAL;
@@ -289,10 +328,12 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
 
 		/* check at 512 byte offset */
 		mutex_lock(&read_mutex);
-		memcpy(&super, cramfs_read(sb, 512, sizeof(super)), sizeof(super));
+		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)
+		if (super->magic != CRAMFS_MAGIC) {
+			if (super->magic == CRAMFS_MAGIC_WEND && !silent)
 				pr_err("wrong endianness\n");
 			else if (!silent)
 				pr_err("wrong magic\n");
@@ -301,34 +342,34 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
 	}
 
 	/* get feature flags first */
-	if (super.flags & ~CRAMFS_SUPPORTED_FLAGS) {
+	if (super->flags & ~CRAMFS_SUPPORTED_FLAGS) {
 		pr_err("unsupported filesystem features\n");
 		return -EINVAL;
 	}
 
 	/* Check that the root inode is in a sane state */
-	if (!S_ISDIR(super.root.mode)) {
+	if (!S_ISDIR(super->root.mode)) {
 		pr_err("root is not a directory\n");
 		return -EINVAL;
 	}
 	/* correct strange, hard-coded permissions of mkcramfs */
-	super.root.mode |= (S_IRUSR | S_IXUSR | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH);
+	super->root.mode |= 0555;
 
-	root_offset = super.root.offset << 2;
-	if (super.flags & CRAMFS_FLAG_FSID_VERSION_2) {
-		sbi->size = super.size;
-		sbi->blocks = super.fsid.blocks;
-		sbi->files = super.fsid.files;
+	root_offset = super->root.offset << 2;
+	if (super->flags & CRAMFS_FLAG_FSID_VERSION_2) {
+		sbi->size = super->size;
+		sbi->blocks = super->fsid.blocks;
+		sbi->files = super->fsid.files;
 	} else {
 		sbi->size = 1<<28;
 		sbi->blocks = 0;
 		sbi->files = 0;
 	}
-	sbi->magic = super.magic;
-	sbi->flags = super.flags;
+	sbi->magic = super->magic;
+	sbi->flags = super->flags;
 	if (root_offset == 0)
 		pr_info("empty filesystem");
-	else if (!(super.flags & CRAMFS_FLAG_SHIFTED_ROOT_OFFSET) &&
+	else if (!(super->flags & CRAMFS_FLAG_SHIFTED_ROOT_OFFSET) &&
 		 ((root_offset != sizeof(struct cramfs_super)) &&
 		  (root_offset != 512 + sizeof(struct cramfs_super))))
 	{
@@ -336,9 +377,18 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
 		return -EINVAL;
 	}
 
+	return 0;
+}
+
+static int cramfs_finalize_super(struct super_block *sb,
+				 struct cramfs_inode *cramfs_root)
+{
+	struct inode *root;
+
 	/* Set it all up.. */
+	sb->s_flags |= MS_RDONLY;
 	sb->s_op = &cramfs_ops;
-	root = get_cramfs_inode(sb, &super.root, 0);
+	root = get_cramfs_inode(sb, cramfs_root, 0);
 	if (IS_ERR(root))
 		return PTR_ERR(root);
 	sb->s_root = d_make_root(root);
@@ -347,10 +397,79 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
 	return 0;
 }
 
+static int cramfs_blkdev_fill_super(struct super_block *sb, void *data,
+				    int silent)
+{
+	struct cramfs_sb_info *sbi;
+	struct cramfs_super super;
+	int i, err;
+
+	sbi = kzalloc(sizeof(struct cramfs_sb_info), GFP_KERNEL);
+	if (!sbi)
+		return -ENOMEM;
+	sb->s_fs_info = sbi;
+
+	/* Invalidate the read buffers on mount: think disk change.. */
+	for (i = 0; i < READ_BUFFERS; i++)
+		buffer_blocknr[i] = -1;
+
+	err = cramfs_read_super(sb, &super, silent);
+	if (err)
+		return err;
+	return cramfs_finalize_super(sb, &super.root);
+}
+
+static int cramfs_mtd_fill_super(struct super_block *sb, void *data,
+				 int silent)
+{
+	struct cramfs_sb_info *sbi;
+	struct cramfs_super super;
+	int err;
+
+	sbi = kzalloc(sizeof(struct cramfs_sb_info), GFP_KERNEL);
+	if (!sbi)
+		return -ENOMEM;
+	sb->s_fs_info = sbi;
+
+	/* Map only one page for now.  Will remap it when fs size is known. */
+	err = mtd_point(sb->s_mtd, 0, PAGE_SIZE, &sbi->mtd_point_size,
+			&sbi->linear_virt_addr, &sbi->linear_phys_addr);
+	if (err || sbi->mtd_point_size != PAGE_SIZE) {
+		pr_err("unable to get direct memory access to mtd:%s\n",
+		       sb->s_mtd->name);
+		return err ? : -ENODATA;
+	}
+
+	pr_info("checking physical address %pap for linear cramfs image\n",
+		&sbi->linear_phys_addr);
+	err = cramfs_read_super(sb, &super, silent);
+	if (err)
+		return err;
+
+	/* Remap the whole filesystem now */
+	pr_info("linear cramfs image on mtd:%s appears to be %lu KB in size\n",
+		sb->s_mtd->name, sbi->size/1024);
+	mtd_unpoint(sb->s_mtd, 0, PAGE_SIZE);
+	err = mtd_point(sb->s_mtd, 0, sbi->size, &sbi->mtd_point_size,
+			&sbi->linear_virt_addr, &sbi->linear_phys_addr);
+	if (err || sbi->mtd_point_size != sbi->size) {
+		pr_err("unable to get direct memory access to mtd:%s\n",
+		       sb->s_mtd->name);
+		return err ? : -ENODATA;
+	}
+
+	return cramfs_finalize_super(sb, &super.root);
+}
+
 static int cramfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 {
 	struct super_block *sb = dentry->d_sb;
-	u64 id = huge_encode_dev(sb->s_bdev->bd_dev);
+	u64 id = 0;
+
+	if (sb->s_bdev)
+		id = huge_encode_dev(sb->s_bdev->bd_dev);
+	else if (sb->s_dev)
+		id = huge_encode_dev(sb->s_dev);
 
 	buf->f_type = CRAMFS_MAGIC;
 	buf->f_bsize = PAGE_SIZE;
@@ -573,10 +692,22 @@ static const struct super_operations cramfs_ops = {
 	.statfs		= cramfs_statfs,
 };
 
-static struct dentry *cramfs_mount(struct file_system_type *fs_type,
-	int flags, const char *dev_name, void *data)
+static struct dentry *cramfs_mount(struct file_system_type *fs_type, int flags,
+				   const char *dev_name, void *data)
 {
-	return mount_bdev(fs_type, flags, dev_name, data, cramfs_fill_super);
+	struct dentry *ret = ERR_PTR(-ENOPROTOOPT);
+
+	if (IS_ENABLED(CONFIG_CRAMFS_MTD)) {
+		ret = mount_mtd(fs_type, flags, dev_name, data,
+				cramfs_mtd_fill_super);
+		if (!IS_ERR(ret))
+			return ret;
+	}
+	if (IS_ENABLED(CONFIG_CRAMFS_BLOCKDEV)) {
+		ret = mount_bdev(fs_type, flags, dev_name, data,
+				 cramfs_blkdev_fill_super);
+	}
+	return ret;
 }
 
 static struct file_system_type cramfs_fs_type = {
-- 
2.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v6 2/4] cramfs: implement uncompressed and arbitrary data block positioning
  2017-10-12  6:16 [PATCH v6 0/4] cramfs refresh for embedded usage Nicolas Pitre
  2017-10-12  6:16 ` [PATCH v6 1/4] cramfs: direct memory access support Nicolas Pitre
@ 2017-10-12  6:16 ` Nicolas Pitre
  2017-10-12  6:16 ` [PATCH v6 3/4] cramfs: add mmap support Nicolas Pitre
  2017-10-12  6:16 ` [PATCH v6 4/4] cramfs: rehabilitate it Nicolas Pitre
  3 siblings, 0 replies; 17+ messages in thread
From: Nicolas Pitre @ 2017-10-12  6:16 UTC (permalink / raw)
  To: Alexander Viro, Christoph Hellwig
  Cc: linux-fsdevel, linux-mm, linux-embedded, linux-kernel, Chris Brandt

Two new capabilities are introduced here:

- The ability to store some blocks uncompressed.

- The ability to locate blocks anywhere.

Those capabilities can be used independently, but the combination
opens the possibility for execute-in-place (XIP) of program text segments
that must remain uncompressed, and in the MMU case, must have a specific
alignment.  It is even possible to still have the writable data segments
from the same file compressed as they have to be copied into RAM anyway.

This is achieved by giving special meanings to some unused block pointer
bits while remaining compatible with legacy cramfs images.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
Tested-by: Chris Brandt <chris.brandt@renesas.com>
---
 fs/cramfs/README               | 31 ++++++++++++++-
 fs/cramfs/inode.c              | 90 +++++++++++++++++++++++++++++++++---------
 include/uapi/linux/cramfs_fs.h | 26 +++++++++++-
 3 files changed, 126 insertions(+), 21 deletions(-)

diff --git a/fs/cramfs/README b/fs/cramfs/README
index 9d4e7ea311..d71b27e0ff 100644
--- a/fs/cramfs/README
+++ b/fs/cramfs/README
@@ -49,17 +49,46 @@ same as the start of the (i+1)'th <block> if there is one).  The first
 <block> immediately follows the last <block_pointer> for the file.
 <block_pointer>s are each 32 bits long.
 
+When the CRAMFS_FLAG_EXT_BLOCK_POINTERS capability bit is set, each
+<block_pointer>'s top bits may contain special flags as follows:
+
+CRAMFS_BLK_FLAG_UNCOMPRESSED (bit 31):
+	The block data is not compressed and should be copied verbatim.
+
+CRAMFS_BLK_FLAG_DIRECT_PTR (bit 30):
+	The <block_pointer> stores the actual block start offset and not
+	its end, shifted right by 2 bits. The block must therefore be
+	aligned to a 4-byte boundary. The block size is either blksize
+	if CRAMFS_BLK_FLAG_UNCOMPRESSED is also specified, otherwise
+	the compressed data length is included in the first 2 bytes of
+	the block data. This is used to allow discontiguous data layout
+	and specific data block alignments e.g. for XIP applications.
+
+
 The order of <file_data>'s is a depth-first descent of the directory
 tree, i.e. the same order as `find -size +0 \( -type f -o -type l \)
 -print'.
 
 
 <block>: The i'th <block> is the output of zlib's compress function
-applied to the i'th blksize-sized chunk of the input data.
+applied to the i'th blksize-sized chunk of the input data if the
+corresponding CRAMFS_BLK_FLAG_UNCOMPRESSED <block_ptr> bit is not set,
+otherwise it is the input data directly.
 (For the last <block> of the file, the input may of course be smaller.)
 Each <block> may be a different size.  (See <block_pointer> above.)
+
 <block>s are merely byte-aligned, not generally u32-aligned.
 
+When CRAMFS_BLK_FLAG_DIRECT_PTR is specified then the corresponding
+<block> may be located anywhere and not necessarily contiguous with
+the previous/next blocks. In that case it is minimally u32-aligned.
+If CRAMFS_BLK_FLAG_UNCOMPRESSED is also specified then the size is always
+blksize except for the last block which is limited by the file length.
+If CRAMFS_BLK_FLAG_DIRECT_PTR is set and CRAMFS_BLK_FLAG_UNCOMPRESSED
+is not set then the first 2 bytes of the block contains the size of the
+remaining block data as this cannot be determined from the placement of
+logically adjacent blocks.
+
 
 Holes
 -----
diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
index 321a1fe17e..d3066a8534 100644
--- a/fs/cramfs/inode.c
+++ b/fs/cramfs/inode.c
@@ -621,34 +621,86 @@ static int cramfs_readpage(struct file *file, struct page *page)
 
 	if (page->index < maxblock) {
 		struct super_block *sb = inode->i_sb;
-		u32 blkptr_offset = OFFSET(inode) + page->index*4;
-		u32 start_offset, compr_len;
+		u32 blkptr_offset = OFFSET(inode) + page->index * 4;
+		u32 block_ptr, block_start, block_len;
+		bool uncompressed, direct;
 
-		start_offset = OFFSET(inode) + maxblock*4;
 		mutex_lock(&read_mutex);
-		if (page->index)
-			start_offset = *(u32 *) cramfs_read(sb, blkptr_offset-4,
-				4);
-		compr_len = (*(u32 *) cramfs_read(sb, blkptr_offset, 4) -
-			start_offset);
-		mutex_unlock(&read_mutex);
+		block_ptr = *(u32 *) cramfs_read(sb, blkptr_offset, 4);
+		uncompressed = (block_ptr & CRAMFS_BLK_FLAG_UNCOMPRESSED);
+		direct = (block_ptr & CRAMFS_BLK_FLAG_DIRECT_PTR);
+		block_ptr &= ~CRAMFS_BLK_FLAGS;
+
+		if (direct) {
+			/*
+			 * The block pointer is an absolute start pointer,
+			 * shifted by 2 bits. The size is included in the
+			 * first 2 bytes of the data block when compressed,
+			 * or PAGE_SIZE otherwise.
+			 */
+			block_start = block_ptr << CRAMFS_BLK_DIRECT_PTR_SHIFT;
+			if (uncompressed) {
+				block_len = PAGE_SIZE;
+				/* if last block: cap to file length */
+				if (page->index == maxblock - 1)
+					block_len =
+						offset_in_page(inode->i_size);
+			} else {
+				block_len = *(u16 *)
+					cramfs_read(sb, block_start, 2);
+				block_start += 2;
+			}
+		} else {
+			/*
+			 * The block pointer indicates one past the end of
+			 * the current block (start of next block). If this
+			 * is the first block then it starts where the block
+			 * pointer table ends, otherwise its start comes
+			 * from the previous block's pointer.
+			 */
+			block_start = OFFSET(inode) + maxblock * 4;
+			if (page->index)
+				block_start = *(u32 *)
+					cramfs_read(sb, blkptr_offset - 4, 4);
+			/* Beware... previous ptr might be a direct ptr */
+			if (unlikely(block_start & CRAMFS_BLK_FLAG_DIRECT_PTR)) {
+				/* See comments on earlier code. */
+				u32 prev_start = block_start;
+			       block_start = prev_start & ~CRAMFS_BLK_FLAGS;
+			       block_start <<= CRAMFS_BLK_DIRECT_PTR_SHIFT;
+				if (prev_start & CRAMFS_BLK_FLAG_UNCOMPRESSED) {
+					block_start += PAGE_SIZE;
+				} else {
+					block_len = *(u16 *)
+						cramfs_read(sb, block_start, 2);
+					block_start += 2 + block_len;
+				}
+			}
+			block_start &= ~CRAMFS_BLK_FLAGS;
+			block_len = block_ptr - block_start;
+		}
 
-		if (compr_len == 0)
+		if (block_len == 0)
 			; /* hole */
-		else if (unlikely(compr_len > (PAGE_SIZE << 1))) {
-			pr_err("bad compressed blocksize %u\n",
-				compr_len);
+		else if (unlikely(block_len > 2*PAGE_SIZE ||
+				  (uncompressed && block_len > PAGE_SIZE))) {
+			mutex_unlock(&read_mutex);
+			pr_err("bad data blocksize %u\n", block_len);
 			goto err;
+		} else if (uncompressed) {
+			memcpy(pgdata,
+			       cramfs_read(sb, block_start, block_len),
+			       block_len);
+			bytes_filled = block_len;
 		} else {
-			mutex_lock(&read_mutex);
 			bytes_filled = cramfs_uncompress_block(pgdata,
 				 PAGE_SIZE,
-				 cramfs_read(sb, start_offset, compr_len),
-				 compr_len);
-			mutex_unlock(&read_mutex);
-			if (unlikely(bytes_filled < 0))
-				goto err;
+				 cramfs_read(sb, block_start, block_len),
+				 block_len);
 		}
+		mutex_unlock(&read_mutex);
+		if (unlikely(bytes_filled < 0))
+			goto err;
 	}
 
 	memset(pgdata + bytes_filled, 0, PAGE_SIZE - bytes_filled);
diff --git a/include/uapi/linux/cramfs_fs.h b/include/uapi/linux/cramfs_fs.h
index e4611a9b92..ce2c885133 100644
--- a/include/uapi/linux/cramfs_fs.h
+++ b/include/uapi/linux/cramfs_fs.h
@@ -73,6 +73,7 @@ struct cramfs_super {
 #define CRAMFS_FLAG_HOLES		0x00000100	/* support for holes */
 #define CRAMFS_FLAG_WRONG_SIGNATURE	0x00000200	/* reserved */
 #define CRAMFS_FLAG_SHIFTED_ROOT_OFFSET	0x00000400	/* shifted root fs */
+#define CRAMFS_FLAG_EXT_BLOCK_POINTERS	0x00000800	/* block pointer extensions */
 
 /*
  * Valid values in super.flags.  Currently we refuse to mount
@@ -82,7 +83,30 @@ struct cramfs_super {
 #define CRAMFS_SUPPORTED_FLAGS	( 0x000000ff \
 				| CRAMFS_FLAG_HOLES \
 				| CRAMFS_FLAG_WRONG_SIGNATURE \
-				| CRAMFS_FLAG_SHIFTED_ROOT_OFFSET )
+				| CRAMFS_FLAG_SHIFTED_ROOT_OFFSET \
+				| CRAMFS_FLAG_EXT_BLOCK_POINTERS )
 
+/*
+ * Block pointer flags
+ *
+ * The maximum block offset that needs to be represented is roughly:
+ *
+ *   (1 << CRAMFS_OFFSET_WIDTH) * 4 +
+ *   (1 << CRAMFS_SIZE_WIDTH) / PAGE_SIZE * (4 + PAGE_SIZE)
+ *   = 0x11004000
+ *
+ * That leaves room for 3 flag bits in the block pointer table.
+ */
+#define CRAMFS_BLK_FLAG_UNCOMPRESSED	(1 << 31)
+#define CRAMFS_BLK_FLAG_DIRECT_PTR	(1 << 30)
+
+#define CRAMFS_BLK_FLAGS	( CRAMFS_BLK_FLAG_UNCOMPRESSED \
+				| CRAMFS_BLK_FLAG_DIRECT_PTR )
+
+/*
+ * Direct blocks are at least 4-byte aligned.
+ * Pointers to direct blocks are shifted down by 2 bits.
+ */
+#define CRAMFS_BLK_DIRECT_PTR_SHIFT	2
 
 #endif /* _UAPI__CRAMFS_H */
-- 
2.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v6 3/4] cramfs: add mmap support
  2017-10-12  6:16 [PATCH v6 0/4] cramfs refresh for embedded usage Nicolas Pitre
  2017-10-12  6:16 ` [PATCH v6 1/4] cramfs: direct memory access support Nicolas Pitre
  2017-10-12  6:16 ` [PATCH v6 2/4] cramfs: implement uncompressed and arbitrary data block positioning Nicolas Pitre
@ 2017-10-12  6:16 ` Nicolas Pitre
  2017-10-13  7:31   ` Christoph Hellwig
  2017-10-12  6:16 ` [PATCH v6 4/4] cramfs: rehabilitate it Nicolas Pitre
  3 siblings, 1 reply; 17+ messages in thread
From: Nicolas Pitre @ 2017-10-12  6:16 UTC (permalink / raw)
  To: Alexander Viro, Christoph Hellwig
  Cc: linux-fsdevel, linux-mm, linux-embedded, linux-kernel, Chris Brandt

When cramfs in physical memory is used then we have the opportunity
to map files directly from ROM, directly into user space, saving on
RAM usage. This gives us Execute-In-Place (XIP) support.

For a file to be mmap()-able, the map area has to correspond to a range
of uncompressed and contiguous blocks, and in the MMU case it also has
to be page aligned. A version of mkcramfs with appropriate support is
necessary to create such a filesystem image.

In the MMU case it may happen for a vma structure to extend beyond the
actual file size. This is notably the case in binfmt_elf.c:elf_map().
Or the file's last block is shared with other files and cannot be mapped
as is. Rather than refusing to mmap it, we do a "mixed" map and let the
regular fault handler populate the unmapped area with RAM-backed pages.
In practice the unmapped area is seldom accessed so page faults might
never occur before this area is discarded.

In the non-MMU case it is the get_unmapped_area method that is responsible
for providing the address where the actual data can be found. No mapping
is necessary of course.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
Tested-by: Chris Brandt <chris.brandt@renesas.com>
---
 fs/cramfs/inode.c | 209 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 209 insertions(+)

diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
index d3066a8534..d967904c53 100644
--- a/fs/cramfs/inode.c
+++ b/fs/cramfs/inode.c
@@ -15,7 +15,10 @@
 
 #include <linux/module.h>
 #include <linux/fs.h>
+#include <linux/file.h>
 #include <linux/pagemap.h>
+#include <linux/pfn_t.h>
+#include <linux/ramfs.h>
 #include <linux/init.h>
 #include <linux/string.h>
 #include <linux/blkdev.h>
@@ -51,6 +54,7 @@ static inline struct cramfs_sb_info *CRAMFS_SB(struct super_block *sb)
 static const struct super_operations cramfs_ops;
 static const struct inode_operations cramfs_dir_inode_operations;
 static const struct file_operations cramfs_directory_operations;
+static const struct file_operations cramfs_physmem_fops;
 static const struct address_space_operations cramfs_aops;
 
 static DEFINE_MUTEX(read_mutex);
@@ -98,6 +102,10 @@ static struct inode *get_cramfs_inode(struct super_block *sb,
 	case S_IFREG:
 		inode->i_fop = &generic_ro_fops;
 		inode->i_data.a_ops = &cramfs_aops;
+		if (IS_ENABLED(CONFIG_CRAMFS_MTD) &&
+		    CRAMFS_SB(sb)->flags & CRAMFS_FLAG_EXT_BLOCK_POINTERS &&
+		    CRAMFS_SB(sb)->linear_phys_addr)
+			inode->i_fop = &cramfs_physmem_fops;
 		break;
 	case S_IFDIR:
 		inode->i_op = &cramfs_dir_inode_operations;
@@ -279,6 +287,207 @@ static void *cramfs_read(struct super_block *sb, unsigned int offset,
 		return NULL;
 }
 
+/*
+ * For a mapping to be possible, we need a range of uncompressed and
+ * contiguous blocks. Return the offset for the first block and number of
+ * valid blocks for which that is true, or zero otherwise.
+ */
+static u32 cramfs_get_block_range(struct inode *inode, u32 pgoff, u32 *pages)
+{
+	struct cramfs_sb_info *sbi = CRAMFS_SB(inode->i_sb);
+	int i;
+	u32 *blockptrs, first_block_addr;
+
+	/*
+	 * We can dereference memory directly here as this code may be
+	 * reached only when there is a direct filesystem image mapping
+	 * available in memory.
+	 */
+	blockptrs = (u32 *)(sbi->linear_virt_addr + OFFSET(inode) + pgoff * 4);
+	first_block_addr = blockptrs[0] & ~CRAMFS_BLK_FLAGS;
+	i = 0;
+	do {
+		u32 block_off = i * (PAGE_SIZE >> CRAMFS_BLK_DIRECT_PTR_SHIFT);
+		u32 expect = (first_block_addr + block_off) |
+			     CRAMFS_BLK_FLAG_DIRECT_PTR |
+			     CRAMFS_BLK_FLAG_UNCOMPRESSED;
+		if (blockptrs[i] != expect) {
+			pr_debug("range: block %d/%d got %#x expects %#x\n",
+				 pgoff+i, pgoff + *pages - 1,
+				 blockptrs[i], expect);
+			if (i == 0)
+				return 0;
+			break;
+		}
+	} while (++i < *pages);
+
+	*pages = i;
+	return first_block_addr << CRAMFS_BLK_DIRECT_PTR_SHIFT;
+}
+
+#ifdef CONFIG_MMU
+
+/*
+ * Return true if the last page of a file in the filesystem image contains
+ * some other data that doesn't belong to that file. It is assumed that the
+ * last block is CRAMFS_BLK_FLAG_DIRECT_PTR | CRAMFS_BLK_FLAG_UNCOMPRESSED
+ * (verified by cramfs_get_block_range() and directly accessible in memory.
+ */
+static bool cramfs_last_page_is_shared(struct inode *inode)
+{
+	struct cramfs_sb_info *sbi = CRAMFS_SB(inode->i_sb);
+	u32 partial, last_page, blockaddr, *blockptrs;
+	char *tail_data;
+
+	partial = offset_in_page(inode->i_size);
+	if (!partial)
+		return false;
+	last_page = inode->i_size >> PAGE_SHIFT;
+	blockptrs = (u32 *)(sbi->linear_virt_addr + OFFSET(inode));
+	blockaddr = blockptrs[last_page] & ~CRAMFS_BLK_FLAGS;
+	blockaddr <<= CRAMFS_BLK_DIRECT_PTR_SHIFT;
+	tail_data = sbi->linear_virt_addr + blockaddr + partial;
+	return memchr_inv(tail_data, 0, PAGE_SIZE - partial) ? true : false;
+}
+
+static int cramfs_physmem_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	struct inode *inode = file_inode(file);
+	struct cramfs_sb_info *sbi = CRAMFS_SB(inode->i_sb);
+	unsigned int pages, max_pages, offset;
+	unsigned long address, pgoff = vma->vm_pgoff;
+	char *bailout_reason;
+	int ret;
+
+	ret = generic_file_readonly_mmap(file, vma);
+	if (ret)
+		return ret;
+
+	/*
+	 * Now try to pre-populate ptes for this vma with a direct
+	 * mapping avoiding memory allocation when possible.
+	 */
+
+	/* Could COW work here? */
+	bailout_reason = "vma is writable";
+	if (vma->vm_flags & VM_WRITE)
+		goto bailout;
+
+	max_pages = (inode->i_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
+	bailout_reason = "beyond file limit";
+	if (pgoff >= max_pages)
+		goto bailout;
+	pages = min(vma_pages(vma), max_pages - pgoff);
+
+	offset = cramfs_get_block_range(inode, pgoff, &pages);
+	bailout_reason = "unsuitable block layout";
+	if (!offset)
+		goto bailout;
+	address = sbi->linear_phys_addr + offset;
+	bailout_reason = "data is not page aligned";
+	if (!PAGE_ALIGNED(address))
+		goto bailout;
+
+	/* Don't map the last page if it contains some other data */
+	if (pgoff + pages == max_pages && cramfs_last_page_is_shared(inode)) {
+		pr_debug("mmap: %s: last page is shared\n",
+			 file_dentry(file)->d_name.name);
+		pages--;
+	}
+
+	if (!pages) {
+		bailout_reason = "no suitable block remaining";
+		goto bailout;
+	}
+
+	if (pages == vma_pages(vma)) {
+		/*
+		 * The entire vma is mappable. remap_pfn_range() will
+		 * make it distinguishable from a non-direct mapping
+		 * in /proc/<pid>/maps by substituting the file offset
+		 * with the actual physical address.
+		 */
+		ret = remap_pfn_range(vma, vma->vm_start, address >> PAGE_SHIFT,
+				      pages * PAGE_SIZE, vma->vm_page_prot);
+	} else {
+		/*
+		 * Let's create a mixed map if we can't map it all.
+		 * The normal paging machinery will take care of the
+		 * unpopulated ptes via cramfs_readpage().
+		 */
+		int i;
+		vma->vm_flags |= VM_MIXEDMAP;
+		for (i = 0; i < pages && !ret; i++) {
+			unsigned long off = i * PAGE_SIZE;
+			pfn_t pfn = phys_to_pfn_t(address + off, PFN_DEV);
+			ret = vm_insert_mixed(vma, vma->vm_start + off, pfn);
+		}
+	}
+
+	if (!ret)
+		pr_debug("mapped %s[%lu] at 0x%08lx (%u/%lu pages) "
+			 "to vma 0x%08lx, page_prot 0x%llx\n",
+			 file_dentry(file)->d_name.name, pgoff,
+			 address, pages, vma_pages(vma), vma->vm_start,
+			 (unsigned long long)pgprot_val(vma->vm_page_prot));
+	return ret;
+
+bailout:
+	pr_debug("%s[%lu]: direct mmap impossible: %s\n",
+		 file_dentry(file)->d_name.name, pgoff, bailout_reason);
+	/* Didn't manage any direct map, but normal paging is still possible */
+	return 0;
+}
+
+#else /* CONFIG_MMU */
+
+static int cramfs_physmem_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	return vma->vm_flags & (VM_SHARED | VM_MAYSHARE) ? 0 : -ENOSYS;
+}
+
+static unsigned long cramfs_physmem_get_unmapped_area(struct file *file,
+			unsigned long addr, unsigned long len,
+			unsigned long pgoff, unsigned long flags)
+{
+	struct inode *inode = file_inode(file);
+	struct super_block *sb = inode->i_sb;
+	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
+	unsigned int pages, block_pages, max_pages, offset;
+
+	pages = (len + PAGE_SIZE - 1) >> PAGE_SHIFT;
+	max_pages = (inode->i_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
+	if (pgoff >= max_pages || pages > max_pages - pgoff)
+		return -EINVAL;
+	block_pages = pages;
+	offset = cramfs_get_block_range(inode, pgoff, &block_pages);
+	if (!offset || block_pages != pages)
+		return -ENOSYS;
+	addr = sbi->linear_phys_addr + offset;
+	pr_debug("get_unmapped for %s ofs %#lx siz %lu at 0x%08lx\n",
+		 file_dentry(file)->d_name.name, pgoff*PAGE_SIZE, len, addr);
+	return addr;
+}
+
+static unsigned int cramfs_physmem_mmap_capabilities(struct file *file)
+{
+	return NOMMU_MAP_COPY | NOMMU_MAP_DIRECT |
+	       NOMMU_MAP_READ | NOMMU_MAP_EXEC;
+}
+
+#endif /* CONFIG_MMU */
+
+static const struct file_operations cramfs_physmem_fops = {
+	.llseek			= generic_file_llseek,
+	.read_iter		= generic_file_read_iter,
+	.splice_read		= generic_file_splice_read,
+	.mmap			= cramfs_physmem_mmap,
+#ifndef CONFIG_MMU
+	.get_unmapped_area	= cramfs_physmem_get_unmapped_area,
+	.mmap_capabilities	= cramfs_physmem_mmap_capabilities,
+#endif
+};
+
 static void cramfs_kill_sb(struct super_block *sb)
 {
 	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
-- 
2.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v6 4/4] cramfs: rehabilitate it
  2017-10-12  6:16 [PATCH v6 0/4] cramfs refresh for embedded usage Nicolas Pitre
                   ` (2 preceding siblings ...)
  2017-10-12  6:16 ` [PATCH v6 3/4] cramfs: add mmap support Nicolas Pitre
@ 2017-10-12  6:16 ` Nicolas Pitre
  3 siblings, 0 replies; 17+ messages in thread
From: Nicolas Pitre @ 2017-10-12  6:16 UTC (permalink / raw)
  To: Alexander Viro, Christoph Hellwig
  Cc: linux-fsdevel, linux-mm, linux-embedded, linux-kernel, Chris Brandt

Update documentation, pointer to latest tools, appoint myself as
maintainer. Given it's been unloved for so long, I don't expect anyone
will protest.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
Tested-by: Chris Brandt <chris.brandt@renesas.com>
---
 Documentation/filesystems/cramfs.txt | 42 ++++++++++++++++++++++++++++++++++++
 MAINTAINERS                          |  4 ++--
 fs/cramfs/Kconfig                    |  9 +++++---
 3 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/Documentation/filesystems/cramfs.txt b/Documentation/filesystems/cramfs.txt
index 4006298f67..8e19a53d64 100644
--- a/Documentation/filesystems/cramfs.txt
+++ b/Documentation/filesystems/cramfs.txt
@@ -45,6 +45,48 @@ you can just change the #define in mkcramfs.c, so long as you don't
 mind the filesystem becoming unreadable to future kernels.
 
 
+Memory Mapped cramfs image
+--------------------------
+
+The CRAMFS_MTD Kconfig option adds support for loading data directly from
+a physical linear memory range (usually non volatile memory like Flash)
+instead of going through the block device layer. This saves some memory
+since no intermediate buffering is necessary to hold the data before
+decompressing.
+
+And when data blocks are kept uncompressed and properly aligned, they will
+automatically be mapped directly into user space whenever possible providing
+eXecute-In-Place (XIP) from ROM of read-only segments. Data segments mapped
+read-write (hence they have to be copied to RAM) may still be compressed in
+the cramfs image in the same file along with non compressed read-only
+segments. Both MMU and no-MMU systems are supported. This is particularly
+handy for tiny embedded systems with very tight memory constraints.
+
+The location of the cramfs image in memory is system dependent. You must
+know the proper physical address where the cramfs image is located and
+configure an MTD device for it. Also, that MTD device must be supported
+by a map driver that implements the "point" method. Examples of such
+MTD drivers are cfi_cmdset_0001 (Intel/Sharp CFI flash) or physmap
+(Flash device in physical memory map). MTD partitions based on such devices
+are fine too. Then that device should be specified with the "mtd:" prefix
+as the mount device argument. For example, to mount the MTD device named
+"fs_partition" on the /mnt directory:
+
+$ mount -t cramfs mtd:fs_partition /mnt
+
+To boot a kernel with this as root filesystem, suffice to specify
+something like "root=mtd:fs_partition" on the kernel command line.
+
+
+Tools
+-----
+
+A version of mkcramfs that can take advantage of the latest capabilities
+described above can be found here:
+
+https://github.com/npitre/cramfs-tools
+
+
 For /usr/share/magic
 --------------------
 
diff --git a/MAINTAINERS b/MAINTAINERS
index 65b0c88d5e..cd621c5f52 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3676,8 +3676,8 @@ F:	drivers/cpuidle/*
 F:	include/linux/cpuidle.h
 
 CRAMFS FILESYSTEM
-W:	http://sourceforge.net/projects/cramfs/
-S:	Orphan / Obsolete
+M:	Nicolas Pitre <nico@linaro.org>
+S:	Maintained
 F:	Documentation/filesystems/cramfs.txt
 F:	fs/cramfs/
 
diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
index ef86b06bc0..f937082f32 100644
--- a/fs/cramfs/Kconfig
+++ b/fs/cramfs/Kconfig
@@ -1,5 +1,5 @@
 config CRAMFS
-	tristate "Compressed ROM file system support (cramfs) (OBSOLETE)"
+	tristate "Compressed ROM file system support (cramfs)"
 	select ZLIB_INFLATE
 	help
 	  Saying Y here includes support for CramFs (Compressed ROM File
@@ -15,8 +15,11 @@ config CRAMFS
 	  cramfs.  Note that the root file system (the one containing the
 	  directory /) cannot be compiled as a module.
 
-	  This filesystem is obsoleted by SquashFS, which is much better
-	  in terms of performance and features.
+	  This filesystem is limited in capabilities and performance on
+	  purpose to remain small and low on RAM usage. It is most suitable
+	  for small embedded systems. If you have ample RAM to spare, you may
+	  consider a more capable compressed filesystem such as SquashFS
+	  which is much better in terms of performance and features.
 
 	  If unsure, say N.
 
-- 
2.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* RE: [PATCH v6 1/4] cramfs: direct memory access support
  2017-10-12  6:16 ` [PATCH v6 1/4] cramfs: direct memory access support Nicolas Pitre
@ 2017-10-12 17:03   ` Chris Brandt
  2017-10-13  1:20     ` Nicolas Pitre
  2017-10-13  7:30   ` Christoph Hellwig
  2017-10-13 17:29   ` Al Viro
  2 siblings, 1 reply; 17+ messages in thread
From: Chris Brandt @ 2017-10-12 17:03 UTC (permalink / raw)
  To: Nicolas Pitre, Alexander Viro, Christoph Hellwig
  Cc: linux-fsdevel, linux-mm, linux-embedded, linux-kernel

On Thursday, October 12, 2017, Nicolas Pitre wrote:
> Small embedded systems typically execute the kernel code in place (XIP)
> directly from flash to save on precious RAM usage. This adds the ability
> to consume filesystem data directly from flash to the cramfs filesystem
> as well. Cramfs is particularly well suited to this feature as it is
> very simple and its RAM usage is already very low, and with this feature
> it is possible to use it with no block device support and even lower RAM
> usage.
> 
> This patch was inspired by a similar patch from Shane Nay dated 17 years
> ago that used to be very popular in embedded circles but never made it
> into mainline. This is a cleaned-up implementation that uses far fewer
> ifdef's and gets the actual memory location for the filesystem image
> via MTD at run time. In the context of small IoT deployments, this
> functionality has become relevant and useful again.
> 
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
>  fs/cramfs/Kconfig |  30 +++++++-
>  fs/cramfs/inode.c | 215 +++++++++++++++++++++++++++++++++++++++++++------
> -----

Works!

I first applied the MTD patch series from here:

http://patchwork.ozlabs.org/project/linux-mtd/list/?series=7504

Then this v6 patch series on top of it.

I created a mtd-rom/direct-mapped partition and was able to both mount after boot, and also boot as the rootfs.

Log from booting as rootfs:

[    1.586625] cramfs: checking physical address 0x1b000000 for linear cramfs image
[    1.594512] cramfs: linear cramfs image on mtd:rootfs_xipcramfs appears to be 15744 KB in size
[    1.603619] VFS: Mounted root (cramfs filesystem) readonly on device 31:1.


$ cat /proc/self/maps
00008000-000a1000 r-xp 1b005000 1f:01 18192      /bin/busybox
000a9000-000aa000 rw-p 00099000 1f:01 18192      /bin/busybox
000aa000-000ac000 rw-p 00000000 00:00 0          [heap]
b6e07000-b6ee0000 r-xp 00000000 1f:01 766540     /lib/libc-2.18-2013.10.so
b6ee0000-b6ee8000 ---p 000d9000 1f:01 766540     /lib/libc-2.18-2013.10.so
b6ee8000-b6eea000 r--p 000d9000 1f:01 766540     /lib/libc-2.18-2013.10.so
b6eea000-b6eeb000 rw-p 000db000 1f:01 766540     /lib/libc-2.18-2013.10.so
b6eeb000-b6eee000 rw-p 00000000 00:00 0
b6eee000-b6f05000 r-xp 00000000 1f:01 670372     /lib/ld-2.18-2013.10.so
b6f08000-b6f09000 rw-p 00000000 00:00 0
b6f0a000-b6f0c000 rw-p 00000000 00:00 0
b6f0c000-b6f0d000 r--p 00016000 1f:01 670372     /lib/ld-2.18-2013.10.so
b6f0d000-b6f0e000 rw-p 00017000 1f:01 670372     /lib/ld-2.18-2013.10.so
bedb0000-bedd1000 rw-p 00000000 00:00 0          [stack]
bedf4000-bedf5000 r-xp 00000000 00:00 0          [sigpage]
ffff0000-ffff1000 r-xp 00000000 00:00 0          [vectors]

So far, so good.

Thank you!


Tested-by: Chris Brandt <chris.brandt@renesas.com>


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v6 1/4] cramfs: direct memory access support
  2017-10-12 17:03   ` Chris Brandt
@ 2017-10-13  1:20     ` Nicolas Pitre
  0 siblings, 0 replies; 17+ messages in thread
From: Nicolas Pitre @ 2017-10-13  1:20 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Alexander Viro, Christoph Hellwig, linux-fsdevel, linux-mm,
	linux-embedded, linux-kernel

On Thu, 12 Oct 2017, Chris Brandt wrote:

> On Thursday, October 12, 2017, Nicolas Pitre wrote:
> > Small embedded systems typically execute the kernel code in place (XIP)
> > directly from flash to save on precious RAM usage. This adds the ability
> > to consume filesystem data directly from flash to the cramfs filesystem
> > as well. Cramfs is particularly well suited to this feature as it is
> > very simple and its RAM usage is already very low, and with this feature
> > it is possible to use it with no block device support and even lower RAM
> > usage.
> > 
> 
> Works!
> 
> I first applied the MTD patch series from here:
> 
> http://patchwork.ozlabs.org/project/linux-mtd/list/?series=7504
> 
> Then this v6 patch series on top of it.
> 
> I created a mtd-rom/direct-mapped partition and was able to both mount after boot, and also boot as the rootfs.
> 
> So far, so good.
> 
> Thank you!
> 
> Tested-by: Chris Brandt <chris.brandt@renesas.com>

Great! Thanks for testing.

Hopefully this series has finally addressed all objections that were 
raised.


Nicolas

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/4] cramfs: direct memory access support
  2017-10-12  6:16 ` [PATCH v6 1/4] cramfs: direct memory access support Nicolas Pitre
  2017-10-12 17:03   ` Chris Brandt
@ 2017-10-13  7:30   ` Christoph Hellwig
  2017-10-13 17:29   ` Al Viro
  2 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2017-10-13  7:30 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Alexander Viro, Christoph Hellwig, linux-fsdevel, linux-mm,
	linux-embedded, linux-kernel, Chris Brandt

This looks much better, thanks.  I'm not a big fan of the games with
IS_ENABLED and letting the compiler optimize code away, but you're
the maintainer..

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 3/4] cramfs: add mmap support
  2017-10-12  6:16 ` [PATCH v6 3/4] cramfs: add mmap support Nicolas Pitre
@ 2017-10-13  7:31   ` Christoph Hellwig
  0 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2017-10-13  7:31 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Alexander Viro, Christoph Hellwig, linux-fsdevel, linux-mm,
	linux-embedded, linux-kernel, Chris Brandt

As said before I'm no big fan of all the debug chatter, but the rest
looks fine, so:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/4] cramfs: direct memory access support
  2017-10-12  6:16 ` [PATCH v6 1/4] cramfs: direct memory access support Nicolas Pitre
  2017-10-12 17:03   ` Chris Brandt
  2017-10-13  7:30   ` Christoph Hellwig
@ 2017-10-13 17:29   ` Al Viro
  2017-10-13 17:39     ` Nicolas Pitre
  2 siblings, 1 reply; 17+ messages in thread
From: Al Viro @ 2017-10-13 17:29 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Christoph Hellwig, linux-fsdevel, linux-mm, linux-embedded,
	linux-kernel, Chris Brandt

On Thu, Oct 12, 2017 at 02:16:10AM -0400, Nicolas Pitre wrote:

>  static void cramfs_kill_sb(struct super_block *sb)
>  {
>  	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
>  
> -	kill_block_super(sb);
> +	if (IS_ENABLED(CCONFIG_CRAMFS_MTD)) {
> +		if (sbi->mtd_point_size)
> +			mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
> +		if (sb->s_mtd)
> +			kill_mtd_super(sb);

...

> +	mtd_unpoint(sb->s_mtd, 0, PAGE_SIZE);
> +	err = mtd_point(sb->s_mtd, 0, sbi->size, &sbi->mtd_point_size,
> +			&sbi->linear_virt_addr, &sbi->linear_phys_addr);
> +	if (err || sbi->mtd_point_size != sbi->size) {

What happens if that mtd_point() fails?  Note that ->kill_sb() will be
called anyway and ->mtd_point_size is going to be non-zero here...  Do
we get the second mtd_unpoint(), or am I misreading that code?

This logics does look fishy, but I'm not familiar enough with mtd guts
to tell if that's OK...

Rules regarding ->kill_sb(): any struct super_block instance that
got out of sget() and its ilk will have ->kill_sb() called.  In case of
mtd, it's simply "if that thing got past setting ->s_mtd, it will be
passed to ->kill_sb()".

Note, BTW, that you *must* have generic_shutdown_super() called once on
every reachable path in ->kill_sb().  AFAICS your patch is correct in
that area (all instances with that ->s_type are created either in
mount_bdev() or in mount_mtd(); the former will have non-NULL ->s_bdev,
the latter - non-NULL ->s_mtd), but that's one thing to watch out when
doing any modifications.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/4] cramfs: direct memory access support
  2017-10-13 17:29   ` Al Viro
@ 2017-10-13 17:39     ` Nicolas Pitre
  2017-10-13 17:52       ` Al Viro
  0 siblings, 1 reply; 17+ messages in thread
From: Nicolas Pitre @ 2017-10-13 17:39 UTC (permalink / raw)
  To: Al Viro
  Cc: Christoph Hellwig, linux-fsdevel, linux-mm, linux-embedded,
	linux-kernel, Chris Brandt

On Fri, 13 Oct 2017, Al Viro wrote:

> On Thu, Oct 12, 2017 at 02:16:10AM -0400, Nicolas Pitre wrote:
> 
> >  static void cramfs_kill_sb(struct super_block *sb)
> >  {
> >  	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
> >  
> > -	kill_block_super(sb);
> > +	if (IS_ENABLED(CCONFIG_CRAMFS_MTD)) {
> > +		if (sbi->mtd_point_size)
> > +			mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
> > +		if (sb->s_mtd)
> > +			kill_mtd_super(sb);
> 
> ...
> 
> > +	mtd_unpoint(sb->s_mtd, 0, PAGE_SIZE);
> > +	err = mtd_point(sb->s_mtd, 0, sbi->size, &sbi->mtd_point_size,
> > +			&sbi->linear_virt_addr, &sbi->linear_phys_addr);
> > +	if (err || sbi->mtd_point_size != sbi->size) {
> 
> What happens if that mtd_point() fails?  Note that ->kill_sb() will be
> called anyway and ->mtd_point_size is going to be non-zero here...

mtd_point() always clears sbi->mtd_point_size first thing upon entry 
even before it has a chance to fail. So it it fails then 
sbi->mtd_point_size will be zero and ->kill_sb() will skip the unpoint 
call.


Nicolas

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/4] cramfs: direct memory access support
  2017-10-13 17:39     ` Nicolas Pitre
@ 2017-10-13 17:52       ` Al Viro
  2017-10-13 18:18         ` Nicolas Pitre
  2017-10-13 20:09         ` Nicolas Pitre
  0 siblings, 2 replies; 17+ messages in thread
From: Al Viro @ 2017-10-13 17:52 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Christoph Hellwig, linux-fsdevel, linux-mm, linux-embedded,
	linux-kernel, Chris Brandt

On Fri, Oct 13, 2017 at 01:39:13PM -0400, Nicolas Pitre wrote:
> On Fri, 13 Oct 2017, Al Viro wrote:
> 
> > On Thu, Oct 12, 2017 at 02:16:10AM -0400, Nicolas Pitre wrote:
> > 
> > >  static void cramfs_kill_sb(struct super_block *sb)
> > >  {
> > >  	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
> > >  
> > > -	kill_block_super(sb);
> > > +	if (IS_ENABLED(CCONFIG_CRAMFS_MTD)) {
> > > +		if (sbi->mtd_point_size)
> > > +			mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
> > > +		if (sb->s_mtd)
> > > +			kill_mtd_super(sb);
> > 
> > ...
> > 
> > > +	mtd_unpoint(sb->s_mtd, 0, PAGE_SIZE);
> > > +	err = mtd_point(sb->s_mtd, 0, sbi->size, &sbi->mtd_point_size,
> > > +			&sbi->linear_virt_addr, &sbi->linear_phys_addr);
> > > +	if (err || sbi->mtd_point_size != sbi->size) {
> > 
> > What happens if that mtd_point() fails?  Note that ->kill_sb() will be
> > called anyway and ->mtd_point_size is going to be non-zero here...
> 
> mtd_point() always clears sbi->mtd_point_size first thing upon entry 
> even before it has a chance to fail. So it it fails then 
> sbi->mtd_point_size will be zero and ->kill_sb() will skip the unpoint 
> call.

OK...  I wonder if it should simply define stubs for kill_mtd_super(),
mtd_unpoint() and kill_block_super() in !CONFIG_MTD and !CONFIG_BLOCK
cases.  mount_mtd() and mount_bdev() as well - e.g.  mount_bdev()
returning ERR_PTR(-ENODEV) and kill_block_super() being simply BUG()
in !CONFIG_BLOCK case.  Then cramfs_kill_sb() would be
	if (sb->s_mtd) {
		if (sbi->mtd_point_size)
			mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
		kill_mtd_super(sb);
	} else {
		kill_block_super(sb);
	}
	kfree(sbi);

Wait.  Looking at that code... what happens if you hit this failure
exit:
        sbi = kzalloc(sizeof(struct cramfs_sb_info), GFP_KERNEL);
        if (!sbi)
                return -ENOMEM;

Current cramfs_kill_sb() will do kill_block_super() and kfree(NULL), which
works nicely, but you are dereferencing that sucker, not just passing it
to kfree().  IOW, that if (sbi->....) ought to be if (sbi && sbi->...)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/4] cramfs: direct memory access support
  2017-10-13 17:52       ` Al Viro
@ 2017-10-13 18:18         ` Nicolas Pitre
  2017-10-13 20:09         ` Nicolas Pitre
  1 sibling, 0 replies; 17+ messages in thread
From: Nicolas Pitre @ 2017-10-13 18:18 UTC (permalink / raw)
  To: Al Viro
  Cc: Christoph Hellwig, linux-fsdevel, linux-mm, linux-embedded,
	linux-kernel, Chris Brandt

On Fri, 13 Oct 2017, Al Viro wrote:

> On Fri, Oct 13, 2017 at 01:39:13PM -0400, Nicolas Pitre wrote:
> > On Fri, 13 Oct 2017, Al Viro wrote:
> > 
> > > On Thu, Oct 12, 2017 at 02:16:10AM -0400, Nicolas Pitre wrote:
> > > 
> > > >  static void cramfs_kill_sb(struct super_block *sb)
> > > >  {
> > > >  	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
> > > >  
> > > > -	kill_block_super(sb);
> > > > +	if (IS_ENABLED(CCONFIG_CRAMFS_MTD)) {
> > > > +		if (sbi->mtd_point_size)
> > > > +			mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
> > > > +		if (sb->s_mtd)
> > > > +			kill_mtd_super(sb);
> > > 
> > > ...
> > > 
> > > > +	mtd_unpoint(sb->s_mtd, 0, PAGE_SIZE);
> > > > +	err = mtd_point(sb->s_mtd, 0, sbi->size, &sbi->mtd_point_size,
> > > > +			&sbi->linear_virt_addr, &sbi->linear_phys_addr);
> > > > +	if (err || sbi->mtd_point_size != sbi->size) {
> > > 
> > > What happens if that mtd_point() fails?  Note that ->kill_sb() will be
> > > called anyway and ->mtd_point_size is going to be non-zero here...
> > 
> > mtd_point() always clears sbi->mtd_point_size first thing upon entry 
> > even before it has a chance to fail. So it it fails then 
> > sbi->mtd_point_size will be zero and ->kill_sb() will skip the unpoint 
> > call.
> 
> OK...  I wonder if it should simply define stubs for kill_mtd_super(),
> mtd_unpoint() and kill_block_super() in !CONFIG_MTD and !CONFIG_BLOCK
> cases.  mount_mtd() and mount_bdev() as well - e.g.  mount_bdev()
> returning ERR_PTR(-ENODEV) and kill_block_super() being simply BUG()
> in !CONFIG_BLOCK case.  Then cramfs_kill_sb() would be
> 	if (sb->s_mtd) {
> 		if (sbi->mtd_point_size)
> 			mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
> 		kill_mtd_super(sb);
> 	} else {
> 		kill_block_super(sb);
> 	}
> 	kfree(sbi);

What I really like about IS_ENABLED() usage is the immediate build 
coverage without having to run all config combinations. The compiler 
will discard unneeded code and avoid pesky unused variable warnings that 
require ugly #ifdefs otherwise.

> Wait.  Looking at that code... what happens if you hit this failure
> exit:
>         sbi = kzalloc(sizeof(struct cramfs_sb_info), GFP_KERNEL);
>         if (!sbi)
>                 return -ENOMEM;
> 
> Current cramfs_kill_sb() will do kill_block_super() and kfree(NULL), which
> works nicely, but you are dereferencing that sucker, not just passing it
> to kfree().  IOW, that if (sbi->....) ought to be if (sbi && sbi->...)

Right, good catch.
Fixed in my tree now.


Nicolas

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/4] cramfs: direct memory access support
  2017-10-13 17:52       ` Al Viro
  2017-10-13 18:18         ` Nicolas Pitre
@ 2017-10-13 20:09         ` Nicolas Pitre
  2017-10-14  0:31           ` Al Viro
  1 sibling, 1 reply; 17+ messages in thread
From: Nicolas Pitre @ 2017-10-13 20:09 UTC (permalink / raw)
  To: Al Viro
  Cc: Christoph Hellwig, linux-fsdevel, linux-mm, linux-embedded,
	linux-kernel, Chris Brandt

On Fri, 13 Oct 2017, Al Viro wrote:

> OK...  I wonder if it should simply define stubs for kill_mtd_super(),
> mtd_unpoint() and kill_block_super() in !CONFIG_MTD and !CONFIG_BLOCK
> cases.  mount_mtd() and mount_bdev() as well - e.g.  mount_bdev()
> returning ERR_PTR(-ENODEV) and kill_block_super() being simply BUG()
> in !CONFIG_BLOCK case.  Then cramfs_kill_sb() would be
> 	if (sb->s_mtd) {
> 		if (sbi->mtd_point_size)
> 			mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
> 		kill_mtd_super(sb);
> 	} else {
> 		kill_block_super(sb);
> 	}
> 	kfree(sbi);

Well... Stubs have to be named differently or they conflict with 
existing declarations. At that point that makes for more lines of code 
compared to the current patch and the naming indirection makes it less 
obvious when reading the code. Alternatively I could add those stubs in 
the corresponding header files and #ifdef the existing declarations 
away. That might look somewhat less cluttered in the main code but it 
also hides what is actually going on and left me unconvinced. And I'm 
not sure this is worth it in the end given this is not a common 
occurrence in the kernel either.

Still, I've rearanged it slightly and fixed the null deref you spotted 
earlier. Latest patch below:

----- >8
Subject: [PATCH] cramfs: direct memory access support

Small embedded systems typically execute the kernel code in place (XIP)
directly from flash to save on precious RAM usage. This patch adds to
cramfs the ability to consume filesystem data directly from flash as
well. Cramfs is particularly well suited to this feature as it is very
simple with low RAM usage, and with this feature it is possible to use
it with no block device support and consequently even lower RAM usage.

This patch was inspired by a similar patch from Shane Nay dated 17 years
ago that used to be very popular in embedded circles but never made it
into mainline. This is a cleaned-up implementation that uses far fewer
ifdef's and gets the actual memory location for the filesystem image
via MTD at run time. In the context of small IoT deployments, this
functionality has become relevant and useful again.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
Tested-by: Chris Brandt <chris.brandt@renesas.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>

diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
index 11b29d491b..ef86b06bc0 100644
--- a/fs/cramfs/Kconfig
+++ b/fs/cramfs/Kconfig
@@ -1,6 +1,5 @@
 config CRAMFS
 	tristate "Compressed ROM file system support (cramfs) (OBSOLETE)"
-	depends on BLOCK
 	select ZLIB_INFLATE
 	help
 	  Saying Y here includes support for CramFs (Compressed ROM File
@@ -20,3 +19,32 @@ config CRAMFS
 	  in terms of performance and features.
 
 	  If unsure, say N.
+
+config CRAMFS_BLOCKDEV
+	bool "Support CramFs image over a regular block device" if EXPERT
+	depends on CRAMFS && BLOCK
+	default y
+	help
+	  This option allows the CramFs driver to load data from a regular
+	  block device such a disk partition or a ramdisk.
+
+config CRAMFS_MTD
+	bool "Support CramFs image directly mapped in physical memory"
+	depends on CRAMFS && MTD
+	default y if !CRAMFS_BLOCKDEV
+	help
+	  This option allows the CramFs driver to load data directly from
+	  a linear adressed memory range (usually non volatile memory
+	  like flash) instead of going through the block device layer.
+	  This saves some memory since no intermediate buffering is
+	  necessary.
+
+	  The location of the CramFs image is determined by a
+	  MTD device capable of direct memory mapping e.g. from
+	  the 'physmap' map driver or a resulting MTD partition.
+	  For example, this would mount the cramfs image stored in
+	  the MTD partition named "xip_fs" on the /mnt mountpoint:
+
+	  mount -t cramfs mtd:xip_fs /mnt
+
+	  If unsure, say N.
diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
index 7919967488..bcdccb7a82 100644
--- a/fs/cramfs/inode.c
+++ b/fs/cramfs/inode.c
@@ -19,6 +19,8 @@
 #include <linux/init.h>
 #include <linux/string.h>
 #include <linux/blkdev.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/super.h>
 #include <linux/slab.h>
 #include <linux/vfs.h>
 #include <linux/mutex.h>
@@ -36,6 +38,9 @@ struct cramfs_sb_info {
 	unsigned long blocks;
 	unsigned long files;
 	unsigned long flags;
+	void *linear_virt_addr;
+	resource_size_t linear_phys_addr;
+	size_t mtd_point_size;
 };
 
 static inline struct cramfs_sb_info *CRAMFS_SB(struct super_block *sb)
@@ -140,6 +145,9 @@ static struct inode *get_cramfs_inode(struct super_block *sb,
  * BLKS_PER_BUF*PAGE_SIZE, so that the caller doesn't need to
  * worry about end-of-buffer issues even when decompressing a full
  * page cache.
+ *
+ * Note: This is all optimized away at compile time when
+ *       CONFIG_CRAMFS_BLOCKDEV=n.
  */
 #define READ_BUFFERS (2)
 /* NEXT_BUFFER(): Loop over [0..(READ_BUFFERS-1)]. */
@@ -160,10 +168,10 @@ static struct super_block *buffer_dev[READ_BUFFERS];
 static int next_buffer;
 
 /*
- * Returns a pointer to a buffer containing at least LEN bytes of
- * filesystem starting at byte offset OFFSET into the filesystem.
+ * Populate our block cache and return a pointer to it.
  */
-static void *cramfs_read(struct super_block *sb, unsigned int offset, unsigned int len)
+static void *cramfs_blkdev_read(struct super_block *sb, unsigned int offset,
+				unsigned int len)
 {
 	struct address_space *mapping = sb->s_bdev->bd_inode->i_mapping;
 	struct page *pages[BLKS_PER_BUF];
@@ -239,11 +247,49 @@ static void *cramfs_read(struct super_block *sb, unsigned int offset, unsigned i
 	return read_buffers[buffer] + offset;
 }
 
+/*
+ * Return a pointer to the linearly addressed cramfs image in memory.
+ */
+static void *cramfs_direct_read(struct super_block *sb, unsigned int offset,
+				unsigned int len)
+{
+	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
+
+	if (!len)
+		return NULL;
+	if (len > sbi->size || offset > sbi->size - len)
+		return page_address(ZERO_PAGE(0));
+	return sbi->linear_virt_addr + offset;
+}
+
+/*
+ * Returns a pointer to a buffer containing at least LEN bytes of
+ * filesystem starting at byte offset OFFSET into the filesystem.
+ */
+static void *cramfs_read(struct super_block *sb, unsigned int offset,
+			 unsigned int len)
+{
+	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
+
+	if (IS_ENABLED(CONFIG_CRAMFS_MTD) && sbi->linear_virt_addr)
+		return cramfs_direct_read(sb, offset, len);
+	else if (IS_ENABLED(CONFIG_CRAMFS_BLOCKDEV))
+		return cramfs_blkdev_read(sb, offset, len);
+	else
+		return NULL;
+}
+
 static void cramfs_kill_sb(struct super_block *sb)
 {
 	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
 
-	kill_block_super(sb);
+	if (IS_ENABLED(CCONFIG_CRAMFS_MTD) && sb->s_mtd) {
+		if (sbi && sbi->mtd_point_size)
+			mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
+		kill_mtd_super(sb);
+	} else if (IS_ENABLED(CONFIG_CRAMFS_BLOCKDEV) && sb->s_bdev) {
+		kill_block_super(sb);
+	}
 	kfree(sbi);
 }
 
@@ -254,34 +300,24 @@ static int cramfs_remount(struct super_block *sb, int *flags, char *data)
 	return 0;
 }
 
-static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
+static int cramfs_read_super(struct super_block *sb,
+			     struct cramfs_super *super, int silent)
 {
-	int i;
-	struct cramfs_super super;
+	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
 	unsigned long root_offset;
-	struct cramfs_sb_info *sbi;
-	struct inode *root;
-
-	sb->s_flags |= MS_RDONLY;
-
-	sbi = kzalloc(sizeof(struct cramfs_sb_info), GFP_KERNEL);
-	if (!sbi)
-		return -ENOMEM;
-	sb->s_fs_info = sbi;
 
-	/* Invalidate the read buffers on mount: think disk change.. */
-	mutex_lock(&read_mutex);
-	for (i = 0; i < READ_BUFFERS; i++)
-		buffer_blocknr[i] = -1;
+	/* We don't know the real size yet */
+	sbi->size = PAGE_SIZE;
 
 	/* Read the first block and get the superblock from it */
-	memcpy(&super, cramfs_read(sb, 0, sizeof(super)), sizeof(super));
+	mutex_lock(&read_mutex);
+	memcpy(super, cramfs_read(sb, 0, sizeof(*super)), sizeof(*super));
 	mutex_unlock(&read_mutex);
 
 	/* Do sanity checks on the superblock */
-	if (super.magic != CRAMFS_MAGIC) {
+	if (super->magic != CRAMFS_MAGIC) {
 		/* check for wrong endianness */
-		if (super.magic == CRAMFS_MAGIC_WEND) {
+		if (super->magic == CRAMFS_MAGIC_WEND) {
 			if (!silent)
 				pr_err("wrong endianness\n");
 			return -EINVAL;
@@ -289,10 +325,12 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
 
 		/* check at 512 byte offset */
 		mutex_lock(&read_mutex);
-		memcpy(&super, cramfs_read(sb, 512, sizeof(super)), sizeof(super));
+		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)
+		if (super->magic != CRAMFS_MAGIC) {
+			if (super->magic == CRAMFS_MAGIC_WEND && !silent)
 				pr_err("wrong endianness\n");
 			else if (!silent)
 				pr_err("wrong magic\n");
@@ -301,34 +339,34 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
 	}
 
 	/* get feature flags first */
-	if (super.flags & ~CRAMFS_SUPPORTED_FLAGS) {
+	if (super->flags & ~CRAMFS_SUPPORTED_FLAGS) {
 		pr_err("unsupported filesystem features\n");
 		return -EINVAL;
 	}
 
 	/* Check that the root inode is in a sane state */
-	if (!S_ISDIR(super.root.mode)) {
+	if (!S_ISDIR(super->root.mode)) {
 		pr_err("root is not a directory\n");
 		return -EINVAL;
 	}
 	/* correct strange, hard-coded permissions of mkcramfs */
-	super.root.mode |= (S_IRUSR | S_IXUSR | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH);
+	super->root.mode |= 0555;
 
-	root_offset = super.root.offset << 2;
-	if (super.flags & CRAMFS_FLAG_FSID_VERSION_2) {
-		sbi->size = super.size;
-		sbi->blocks = super.fsid.blocks;
-		sbi->files = super.fsid.files;
+	root_offset = super->root.offset << 2;
+	if (super->flags & CRAMFS_FLAG_FSID_VERSION_2) {
+		sbi->size = super->size;
+		sbi->blocks = super->fsid.blocks;
+		sbi->files = super->fsid.files;
 	} else {
 		sbi->size = 1<<28;
 		sbi->blocks = 0;
 		sbi->files = 0;
 	}
-	sbi->magic = super.magic;
-	sbi->flags = super.flags;
+	sbi->magic = super->magic;
+	sbi->flags = super->flags;
 	if (root_offset == 0)
 		pr_info("empty filesystem");
-	else if (!(super.flags & CRAMFS_FLAG_SHIFTED_ROOT_OFFSET) &&
+	else if (!(super->flags & CRAMFS_FLAG_SHIFTED_ROOT_OFFSET) &&
 		 ((root_offset != sizeof(struct cramfs_super)) &&
 		  (root_offset != 512 + sizeof(struct cramfs_super))))
 	{
@@ -336,9 +374,18 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
 		return -EINVAL;
 	}
 
+	return 0;
+}
+
+static int cramfs_finalize_super(struct super_block *sb,
+				 struct cramfs_inode *cramfs_root)
+{
+	struct inode *root;
+
 	/* Set it all up.. */
+	sb->s_flags |= MS_RDONLY;
 	sb->s_op = &cramfs_ops;
-	root = get_cramfs_inode(sb, &super.root, 0);
+	root = get_cramfs_inode(sb, cramfs_root, 0);
 	if (IS_ERR(root))
 		return PTR_ERR(root);
 	sb->s_root = d_make_root(root);
@@ -347,10 +394,79 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
 	return 0;
 }
 
+static int cramfs_blkdev_fill_super(struct super_block *sb, void *data,
+				    int silent)
+{
+	struct cramfs_sb_info *sbi;
+	struct cramfs_super super;
+	int i, err;
+
+	sbi = kzalloc(sizeof(struct cramfs_sb_info), GFP_KERNEL);
+	if (!sbi)
+		return -ENOMEM;
+	sb->s_fs_info = sbi;
+
+	/* Invalidate the read buffers on mount: think disk change.. */
+	for (i = 0; i < READ_BUFFERS; i++)
+		buffer_blocknr[i] = -1;
+
+	err = cramfs_read_super(sb, &super, silent);
+	if (err)
+		return err;
+	return cramfs_finalize_super(sb, &super.root);
+}
+
+static int cramfs_mtd_fill_super(struct super_block *sb, void *data,
+				 int silent)
+{
+	struct cramfs_sb_info *sbi;
+	struct cramfs_super super;
+	int err;
+
+	sbi = kzalloc(sizeof(struct cramfs_sb_info), GFP_KERNEL);
+	if (!sbi)
+		return -ENOMEM;
+	sb->s_fs_info = sbi;
+
+	/* Map only one page for now.  Will remap it when fs size is known. */
+	err = mtd_point(sb->s_mtd, 0, PAGE_SIZE, &sbi->mtd_point_size,
+			&sbi->linear_virt_addr, &sbi->linear_phys_addr);
+	if (err || sbi->mtd_point_size != PAGE_SIZE) {
+		pr_err("unable to get direct memory access to mtd:%s\n",
+		       sb->s_mtd->name);
+		return err ? : -ENODATA;
+	}
+
+	pr_info("checking physical address %pap for linear cramfs image\n",
+		&sbi->linear_phys_addr);
+	err = cramfs_read_super(sb, &super, silent);
+	if (err)
+		return err;
+
+	/* Remap the whole filesystem now */
+	pr_info("linear cramfs image on mtd:%s appears to be %lu KB in size\n",
+		sb->s_mtd->name, sbi->size/1024);
+	mtd_unpoint(sb->s_mtd, 0, PAGE_SIZE);
+	err = mtd_point(sb->s_mtd, 0, sbi->size, &sbi->mtd_point_size,
+			&sbi->linear_virt_addr, &sbi->linear_phys_addr);
+	if (err || sbi->mtd_point_size != sbi->size) {
+		pr_err("unable to get direct memory access to mtd:%s\n",
+		       sb->s_mtd->name);
+		return err ? : -ENODATA;
+	}
+
+	return cramfs_finalize_super(sb, &super.root);
+}
+
 static int cramfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 {
 	struct super_block *sb = dentry->d_sb;
-	u64 id = huge_encode_dev(sb->s_bdev->bd_dev);
+	u64 id = 0;
+
+	if (sb->s_bdev)
+		id = huge_encode_dev(sb->s_bdev->bd_dev);
+	else if (sb->s_dev)
+		id = huge_encode_dev(sb->s_dev);
 
 	buf->f_type = CRAMFS_MAGIC;
 	buf->f_bsize = PAGE_SIZE;
@@ -573,10 +689,22 @@ static const struct super_operations cramfs_ops = {
 	.statfs		= cramfs_statfs,
 };
 
-static struct dentry *cramfs_mount(struct file_system_type *fs_type,
-	int flags, const char *dev_name, void *data)
+static struct dentry *cramfs_mount(struct file_system_type *fs_type, int flags,
+				   const char *dev_name, void *data)
 {
-	return mount_bdev(fs_type, flags, dev_name, data, cramfs_fill_super);
+	struct dentry *ret = ERR_PTR(-ENOPROTOOPT);
+
+	if (IS_ENABLED(CONFIG_CRAMFS_MTD)) {
+		ret = mount_mtd(fs_type, flags, dev_name, data,
+				cramfs_mtd_fill_super);
+		if (!IS_ERR(ret))
+			return ret;
+	}
+	if (IS_ENABLED(CONFIG_CRAMFS_BLOCKDEV)) {
+		ret = mount_bdev(fs_type, flags, dev_name, data,
+				 cramfs_blkdev_fill_super);
+	}
+	return ret;
 }
 
 static struct file_system_type cramfs_fs_type = {

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/4] cramfs: direct memory access support
  2017-10-13 20:09         ` Nicolas Pitre
@ 2017-10-14  0:31           ` Al Viro
  2017-10-14  2:25             ` Nicolas Pitre
  0 siblings, 1 reply; 17+ messages in thread
From: Al Viro @ 2017-10-14  0:31 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Christoph Hellwig, linux-fsdevel, linux-mm, linux-embedded,
	linux-kernel, Chris Brandt

On Fri, Oct 13, 2017 at 04:09:23PM -0400, Nicolas Pitre wrote:
> On Fri, 13 Oct 2017, Al Viro wrote:
> 
> > OK...  I wonder if it should simply define stubs for kill_mtd_super(),
> > mtd_unpoint() and kill_block_super() in !CONFIG_MTD and !CONFIG_BLOCK
> > cases.  mount_mtd() and mount_bdev() as well - e.g.  mount_bdev()
> > returning ERR_PTR(-ENODEV) and kill_block_super() being simply BUG()
> > in !CONFIG_BLOCK case.  Then cramfs_kill_sb() would be
> > 	if (sb->s_mtd) {
> > 		if (sbi->mtd_point_size)
> > 			mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
> > 		kill_mtd_super(sb);
> > 	} else {
> > 		kill_block_super(sb);
> > 	}
> > 	kfree(sbi);
> 
> Well... Stubs have to be named differently or they conflict with 
> existing declarations. At that point that makes for more lines of code 
> compared to the current patch and the naming indirection makes it less 
> obvious when reading the code. Alternatively I could add those stubs in 
> the corresponding header files and #ifdef the existing declarations 
> away. That might look somewhat less cluttered in the main code but it 
> also hides what is actually going on and left me unconvinced. And I'm 
> not sure this is worth it in the end given this is not a common 
> occurrence in the kernel either.

What I mean is this (completely untested) for CONFIG_BLOCK side of things,
with something similar for CONFIG_MTD one:

Provide definitions of mount_bdev/kill_block_super() in case !CONFIG_BLOCK

mount_bdev() and kill_block_super() are defined only when CONFIG_BLOCK is
defined; however, their declarations in fs.h are unconditional.  We could
make these conditional upon CONFIG_BLOCK as well, but it's easy to provide
inline definitions for !CONFIG_BLOCK case - mount_bdev() should fail with
ENODEV, while kill_block_super() can be simply BUG(); there should be no
superblock instances with non-NULL ->s_bdev on such configs.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 339e73742e73..e773c1c51aad 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2094,9 +2094,18 @@ struct file_system_type {
 extern struct dentry *mount_ns(struct file_system_type *fs_type,
 	int flags, void *data, void *ns, struct user_namespace *user_ns,
 	int (*fill_super)(struct super_block *, void *, int));
+#ifdef CONFIG_BLOCK
 extern struct dentry *mount_bdev(struct file_system_type *fs_type,
 	int flags, const char *dev_name, void *data,
 	int (*fill_super)(struct super_block *, void *, int));
+#else
+static inline struct dentry *mount_bdev(struct file_system_type *fs_type,
+	int flags, const char *dev_name, void *data,
+	int (*fill_super)(struct super_block *, void *, int))
+{
+	return ERR_PTR(-ENODEV);
+}
+#endif
 extern struct dentry *mount_single(struct file_system_type *fs_type,
 	int flags, void *data,
 	int (*fill_super)(struct super_block *, void *, int));
@@ -2105,7 +2114,14 @@ extern struct dentry *mount_nodev(struct file_system_type *fs_type,
 	int (*fill_super)(struct super_block *, void *, int));
 extern struct dentry *mount_subtree(struct vfsmount *mnt, const char *path);
 void generic_shutdown_super(struct super_block *sb);
+#ifdef CONFIG_BLOCK
 void kill_block_super(struct super_block *sb);
+#else
+static inline void kill_block_super(struct super_block *sb)
+{
+	BUG();
+}
+#endif
 void kill_anon_super(struct super_block *sb);
 void kill_litter_super(struct super_block *sb);
 void deactivate_super(struct super_block *sb);

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/4] cramfs: direct memory access support
  2017-10-14  0:31           ` Al Viro
@ 2017-10-14  2:25             ` Nicolas Pitre
  2017-10-14  2:37               ` Nicolas Pitre
  0 siblings, 1 reply; 17+ messages in thread
From: Nicolas Pitre @ 2017-10-14  2:25 UTC (permalink / raw)
  To: Al Viro
  Cc: Christoph Hellwig, linux-fsdevel, linux-mm, linux-embedded,
	linux-kernel, Chris Brandt

On Sat, 14 Oct 2017, Al Viro wrote:

> On Fri, Oct 13, 2017 at 04:09:23PM -0400, Nicolas Pitre wrote:
> > On Fri, 13 Oct 2017, Al Viro wrote:
> > 
> > > OK...  I wonder if it should simply define stubs for kill_mtd_super(),
> > > mtd_unpoint() and kill_block_super() in !CONFIG_MTD and !CONFIG_BLOCK
> > > cases.  mount_mtd() and mount_bdev() as well - e.g.  mount_bdev()
> > > returning ERR_PTR(-ENODEV) and kill_block_super() being simply BUG()
> > > in !CONFIG_BLOCK case.  Then cramfs_kill_sb() would be
> > > 	if (sb->s_mtd) {
> > > 		if (sbi->mtd_point_size)
> > > 			mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
> > > 		kill_mtd_super(sb);
> > > 	} else {
> > > 		kill_block_super(sb);
> > > 	}
> > > 	kfree(sbi);
> > 
> > Well... Stubs have to be named differently or they conflict with 
> > existing declarations. At that point that makes for more lines of code 
> > compared to the current patch and the naming indirection makes it less 
> > obvious when reading the code. Alternatively I could add those stubs in 
> > the corresponding header files and #ifdef the existing declarations 
> > away. That might look somewhat less cluttered in the main code but it 
> > also hides what is actually going on and left me unconvinced. And I'm 
> > not sure this is worth it in the end given this is not a common 
> > occurrence in the kernel either.
> 
> What I mean is this (completely untested) for CONFIG_BLOCK side of things,
> with something similar for CONFIG_MTD one:
> 
> Provide definitions of mount_bdev/kill_block_super() in case !CONFIG_BLOCK

Yes, that's what I thought you meant, which corresponds to the second 
part of my comment above. And as I said I'm not convinced this hiding of 
kernel config effects is better for understanding what is actually going 
on locally, and my own preference is how things are right now.

But if you confirm you really want things that other way then I'll 
oblige and repost.


Nicolas

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/4] cramfs: direct memory access support
  2017-10-14  2:25             ` Nicolas Pitre
@ 2017-10-14  2:37               ` Nicolas Pitre
  0 siblings, 0 replies; 17+ messages in thread
From: Nicolas Pitre @ 2017-10-14  2:37 UTC (permalink / raw)
  To: Al Viro
  Cc: Christoph Hellwig, linux-fsdevel, linux-mm, linux-embedded,
	linux-kernel, Chris Brandt

On Fri, 13 Oct 2017, Nicolas Pitre wrote:

> On Sat, 14 Oct 2017, Al Viro wrote:
> 
> > On Fri, Oct 13, 2017 at 04:09:23PM -0400, Nicolas Pitre wrote:
> > > On Fri, 13 Oct 2017, Al Viro wrote:
> > > 
> > > > OK...  I wonder if it should simply define stubs for kill_mtd_super(),
> > > > mtd_unpoint() and kill_block_super() in !CONFIG_MTD and !CONFIG_BLOCK
> > > > cases.  mount_mtd() and mount_bdev() as well - e.g.  mount_bdev()
> > > > returning ERR_PTR(-ENODEV) and kill_block_super() being simply BUG()
> > > > in !CONFIG_BLOCK case.  Then cramfs_kill_sb() would be
> > > > 	if (sb->s_mtd) {
> > > > 		if (sbi->mtd_point_size)
> > > > 			mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
> > > > 		kill_mtd_super(sb);
> > > > 	} else {
> > > > 		kill_block_super(sb);
> > > > 	}
> > > > 	kfree(sbi);
> > > 
> > > Well... Stubs have to be named differently or they conflict with 
> > > existing declarations. At that point that makes for more lines of code 
> > > compared to the current patch and the naming indirection makes it less 
> > > obvious when reading the code. Alternatively I could add those stubs in 
> > > the corresponding header files and #ifdef the existing declarations 
> > > away. That might look somewhat less cluttered in the main code but it 
> > > also hides what is actually going on and left me unconvinced. And I'm 
> > > not sure this is worth it in the end given this is not a common 
> > > occurrence in the kernel either.
> > 
> > What I mean is this (completely untested) for CONFIG_BLOCK side of things,
> > with something similar for CONFIG_MTD one:
> > 
> > Provide definitions of mount_bdev/kill_block_super() in case !CONFIG_BLOCK
> 
> Yes, that's what I thought you meant, which corresponds to the second 
> part of my comment above. And as I said I'm not convinced this hiding of 
> kernel config effects is better for understanding what is actually going 
> on locally, and my own preference is how things are right now.

Another case that your suggestion doesn't cover well is the ability to 
still have block device support in the kernel for other filesystems but 
_without_ block device support in the cramfs case. In other words, 
having CONFIG_BLOCK=y and CONFIG_CRAMFS_BLOCKDEV=n. This is a common 
case to have embedded devices with the root filesystem in flash while 
still needing access to a FAT filesystem on SD cards. Your stubs are 
conditional on CONFIG_BLOCK but that is not sufficient in this example.


Nicolas

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2017-10-14  2:37 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-12  6:16 [PATCH v6 0/4] cramfs refresh for embedded usage Nicolas Pitre
2017-10-12  6:16 ` [PATCH v6 1/4] cramfs: direct memory access support Nicolas Pitre
2017-10-12 17:03   ` Chris Brandt
2017-10-13  1:20     ` Nicolas Pitre
2017-10-13  7:30   ` Christoph Hellwig
2017-10-13 17:29   ` Al Viro
2017-10-13 17:39     ` Nicolas Pitre
2017-10-13 17:52       ` Al Viro
2017-10-13 18:18         ` Nicolas Pitre
2017-10-13 20:09         ` Nicolas Pitre
2017-10-14  0:31           ` Al Viro
2017-10-14  2:25             ` Nicolas Pitre
2017-10-14  2:37               ` Nicolas Pitre
2017-10-12  6:16 ` [PATCH v6 2/4] cramfs: implement uncompressed and arbitrary data block positioning Nicolas Pitre
2017-10-12  6:16 ` [PATCH v6 3/4] cramfs: add mmap support Nicolas Pitre
2017-10-13  7:31   ` Christoph Hellwig
2017-10-12  6:16 ` [PATCH v6 4/4] cramfs: rehabilitate it Nicolas Pitre

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).