[RFC PATCH 0/6] Convert mount_single-using filesystems to fs_context

[RFC PATCH 0/6] Convert mount_single-using filesystems to fs_context

[David Howells]

Hi Al,

Here's a set of patches that convert romfs, cramfs, jffs2 and squashfs to
use the new fs_context struct and replace mount_mtd().  They have
prerequisite commits (such as replacing mount_bdev()) that can be found in
the branch detailed below.

 (1) Provide a replacement for mount_mtd() that takes an fs_context to
     specify the parameters.

     I also put an mtd device pointer into the fs_context struct for use in
     the sget_fc() test and set functions plus a destructor because
     put_fs_context() can't reach into the mtd module.

 (2) Convert romfs.

 (3) Convert cramfs.

 (4) Convert jffs2.

 (5) Get rid of the now-obsolete mount_mtd() function.

 (6) Convert squashfs.

These can be found in the following branch:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=mount-api-viro

Thanks,
David
---
David Howells (6):
      mtd: Provide fs_context-aware mount_mtd() replacement
      romfs: Convert to fs_context
      cramfs: Convert to fs_context
      jffs2: Convert to fs_context
      mtd: Kill off mount_mtd()
      squashfs: Convert to fs_context


 drivers/mtd/mtdcore.h      |    1 
 drivers/mtd/mtdsuper.c     |  181 ++++++++++++++++++++++----------------------
 fs/cramfs/inode.c          |   69 +++++++++--------
 fs/fs_context.c            |    2 
 fs/jffs2/fs.c              |   21 +++--
 fs/jffs2/os-linux.h        |    4 -
 fs/jffs2/super.c           |  172 ++++++++++++++++++++----------------------
 fs/romfs/super.c           |   46 +++++++----
 fs/squashfs/super.c        |  100 +++++++++++++-----------
 include/linux/fs_context.h |    2 
 include/linux/mtd/super.h  |    6 +
 11 files changed, 316 insertions(+), 288 deletions(-)

[RFC PATCH 2/6] romfs: Convert to fs_context

[David Howells]

Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-mtd@lists.infradead.org
cc: linux-block@vger.kernel.org
---

 fs/romfs/super.c |   46 ++++++++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 18 deletions(-)

diff --git a/fs/romfs/super.c b/fs/romfs/super.c
index 6ccb51993a76..627c2b64ff03 100644
--- a/fs/romfs/super.c
+++ b/fs/romfs/super.c
@@ -65,7 +65,7 @@
 #include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/blkdev.h>
-#include <linux/parser.h>
+#include <linux/fs_context.h>
 #include <linux/mount.h>
 #include <linux/namei.h>
 #include <linux/statfs.h>
@@ -430,10 +430,10 @@ static int romfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 /*
  * remounting must involve read-only
  */
-static int romfs_remount(struct super_block *sb, int *flags, char *data)
+static int romfs_reconfigure(struct fs_context *fc)
 {
-	sync_filesystem(sb);
-	*flags |= SB_RDONLY;
+	sync_filesystem(fc->root->d_sb);
+	fc->sb_flags |= SB_RDONLY;
 	return 0;
 }
 
@@ -441,7 +441,6 @@ static const struct super_operations romfs_super_ops = {
 	.alloc_inode	= romfs_alloc_inode,
 	.destroy_inode	= romfs_destroy_inode,
 	.statfs		= romfs_statfs,
-	.remount_fs	= romfs_remount,
 };
 
 /*
@@ -464,7 +463,7 @@ static __u32 romfs_checksum(const void *data, int size)
 /*
  * fill in the superblock
  */
-static int romfs_fill_super(struct super_block *sb, void *data, int silent)
+static int romfs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct romfs_super_block *rsb;
 	struct inode *root;
@@ -511,8 +510,8 @@ static int romfs_fill_super(struct super_block *sb, void *data, int silent)
 
 	if (rsb->word0 != ROMSB_WORD0 || rsb->word1 != ROMSB_WORD1 ||
 	    img_size < ROMFH_SIZE) {
-		if (!silent)
-			pr_warn("VFS: Can't find a romfs filesystem on dev %s.\n",
+		if (!(fc->sb_flags & SB_SILENT))
+			errorf(fc, "VFS: Can't find a romfs filesystem on dev %s.\n",
 			       sb->s_id);
 		goto error_rsb_inval;
 	}
@@ -525,7 +524,7 @@ static int romfs_fill_super(struct super_block *sb, void *data, int silent)
 	storage = sb->s_mtd ? "MTD" : "the block layer";
 
 	len = strnlen(rsb->name, ROMFS_MAXFN);
-	if (!silent)
+	if (!(fc->sb_flags & SB_SILENT))
 		pr_notice("Mounting image '%*.*s' through %s\n",
 			  (unsigned) len, (unsigned) len, rsb->name, storage);
 
@@ -555,23 +554,34 @@ static int romfs_fill_super(struct super_block *sb, void *data, int silent)
 /*
  * get a superblock for mounting
  */
-static struct dentry *romfs_mount(struct file_system_type *fs_type,
-			int flags, const char *dev_name,
-			void *data)
+static int romfs_get_tree(struct fs_context *fc)
 {
-	struct dentry *ret = ERR_PTR(-EINVAL);
+	int ret = -EINVAL;
 
 #ifdef CONFIG_ROMFS_ON_MTD
-	ret = mount_mtd(fs_type, flags, dev_name, data, romfs_fill_super);
+	ret = vfs_get_mtd_super(fc, romfs_fill_super);
 #endif
 #ifdef CONFIG_ROMFS_ON_BLOCK
-	if (ret == ERR_PTR(-EINVAL))
-		ret = mount_bdev(fs_type, flags, dev_name, data,
-				  romfs_fill_super);
+	if (ret == -EINVAL)
+		ret = vfs_get_block_super(fc, romfs_fill_super);
 #endif
 	return ret;
 }
 
+static const struct fs_context_operations romfs_context_ops = {
+	.get_tree	= romfs_get_tree,
+	.reconfigure	= romfs_reconfigure,
+};
+
+/*
+ * Set up the filesystem mount context.
+ */
+static int romfs_init_fs_context(struct fs_context *fc)
+{
+	fc->ops = &romfs_context_ops;
+	return 0;
+}
+
 /*
  * destroy a romfs superblock in the appropriate manner
  */
@@ -594,7 +604,7 @@ static void romfs_kill_sb(struct super_block *sb)
 static struct file_system_type romfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "romfs",
-	.mount		= romfs_mount,
+	.init_fs_context = romfs_init_fs_context,
 	.kill_sb	= romfs_kill_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };

[RFC PATCH 3/6] cramfs: Convert to fs_context

[David Howells]

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Nicolas Pitre <nico@linaro.org>
cc: linux-mtd@lists.infradead.org
cc: linux-block@vger.kernel.org
---

 fs/cramfs/inode.c |   69 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 39 insertions(+), 30 deletions(-)

diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
index 9352487bd0fc..2ee89a353d64 100644
--- a/fs/cramfs/inode.c
+++ b/fs/cramfs/inode.c
@@ -24,6 +24,7 @@
 #include <linux/blkdev.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/super.h>
+#include <linux/fs_context.h>
 #include <linux/slab.h>
 #include <linux/vfs.h>
 #include <linux/mutex.h>
@@ -506,18 +507,19 @@ static void cramfs_kill_sb(struct super_block *sb)
 	kfree(sbi);
 }
 
-static int cramfs_remount(struct super_block *sb, int *flags, char *data)
+static int cramfs_reconfigure(struct fs_context *fc)
 {
-	sync_filesystem(sb);
-	*flags |= SB_RDONLY;
+	sync_filesystem(fc->root->d_sb);
+	fc->sb_flags |= SB_RDONLY;
 	return 0;
 }
 
-static int cramfs_read_super(struct super_block *sb,
-			     struct cramfs_super *super, int silent)
+static int cramfs_read_super(struct super_block *sb, struct fs_context *fc,
+			     struct cramfs_super *super)
 {
 	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
 	unsigned long root_offset;
+	bool silent = fc->sb_flags & SB_SILENT;
 
 	/* We don't know the real size yet */
 	sbi->size = PAGE_SIZE;
@@ -532,7 +534,7 @@ static int cramfs_read_super(struct super_block *sb,
 		/* check for wrong endianness */
 		if (super->magic == CRAMFS_MAGIC_WEND) {
 			if (!silent)
-				pr_err("wrong endianness\n");
+				errorf(fc, "cramfs: wrong endianness");
 			return -EINVAL;
 		}
 
@@ -544,22 +546,22 @@ static int cramfs_read_super(struct super_block *sb,
 		mutex_unlock(&read_mutex);
 		if (super->magic != CRAMFS_MAGIC) {
 			if (super->magic == CRAMFS_MAGIC_WEND && !silent)
-				pr_err("wrong endianness\n");
+				errorf(fc, "cramfs: wrong endianness");
 			else if (!silent)
-				pr_err("wrong magic\n");
+				errorf(fc, "cramfs: wrong magic");
 			return -EINVAL;
 		}
 	}
 
 	/* get feature flags first */
 	if (super->flags & ~CRAMFS_SUPPORTED_FLAGS) {
-		pr_err("unsupported filesystem features\n");
+		errorf(fc, "cramfs: unsupported filesystem features");
 		return -EINVAL;
 	}
 
 	/* Check that the root inode is in a sane state */
 	if (!S_ISDIR(super->root.mode)) {
-		pr_err("root is not a directory\n");
+		errorf(fc, "cramfs: root is not a directory");
 		return -EINVAL;
 	}
 	/* correct strange, hard-coded permissions of mkcramfs */
@@ -578,12 +580,12 @@ static int cramfs_read_super(struct super_block *sb,
 	sbi->magic = super->magic;
 	sbi->flags = super->flags;
 	if (root_offset == 0)
-		pr_info("empty filesystem");
+		infof(fc, "cramfs: empty filesystem");
 	else if (!(super->flags & CRAMFS_FLAG_SHIFTED_ROOT_OFFSET) &&
 		 ((root_offset != sizeof(struct cramfs_super)) &&
 		  (root_offset != 512 + sizeof(struct cramfs_super))))
 	{
-		pr_err("bad root offset %lu\n", root_offset);
+		errorf(fc, "cramfs: bad root offset %lu", root_offset);
 		return -EINVAL;
 	}
 
@@ -607,8 +609,7 @@ static int cramfs_finalize_super(struct super_block *sb,
 	return 0;
 }
 
-static int cramfs_blkdev_fill_super(struct super_block *sb, void *data,
-				    int silent)
+static int cramfs_blkdev_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct cramfs_sb_info *sbi;
 	struct cramfs_super super;
@@ -623,14 +624,13 @@ static int cramfs_blkdev_fill_super(struct super_block *sb, void *data,
 	for (i = 0; i < READ_BUFFERS; i++)
 		buffer_blocknr[i] = -1;
 
-	err = cramfs_read_super(sb, &super, silent);
+	err = cramfs_read_super(sb, fc, &super);
 	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)
+static int cramfs_mtd_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct cramfs_sb_info *sbi;
 	struct cramfs_super super;
@@ -652,7 +652,7 @@ static int cramfs_mtd_fill_super(struct super_block *sb, void *data,
 
 	pr_info("checking physical address %pap for linear cramfs image\n",
 		&sbi->linear_phys_addr);
-	err = cramfs_read_super(sb, &super, silent);
+	err = cramfs_read_super(sb, fc, &super);
 	if (err)
 		return err;
 
@@ -947,32 +947,41 @@ static const struct inode_operations cramfs_dir_inode_operations = {
 };
 
 static const struct super_operations cramfs_ops = {
-	.remount_fs	= cramfs_remount,
 	.statfs		= cramfs_statfs,
 };
 
-static struct dentry *cramfs_mount(struct file_system_type *fs_type, int flags,
-				   const char *dev_name, void *data)
+static int cramfs_get_tree(struct fs_context *fc)
 {
-	struct dentry *ret = ERR_PTR(-ENOPROTOOPT);
+	int ret = -ENOPROTOOPT;
 
 	if (IS_ENABLED(CONFIG_CRAMFS_MTD)) {
-		ret = mount_mtd(fs_type, flags, dev_name, data,
-				cramfs_mtd_fill_super);
-		if (!IS_ERR(ret))
+		ret = vfs_get_mtd_super(fc, cramfs_mtd_fill_super);
+		if (ret < 0)
 			return ret;
 	}
-	if (IS_ENABLED(CONFIG_CRAMFS_BLOCKDEV)) {
-		ret = mount_bdev(fs_type, flags, dev_name, data,
-				 cramfs_blkdev_fill_super);
-	}
+	if (IS_ENABLED(CONFIG_CRAMFS_BLOCKDEV))
+		ret = vfs_get_block_super(fc, cramfs_blkdev_fill_super);
 	return ret;
 }
 
+static const struct fs_context_operations cramfs_context_ops = {
+	.get_tree	= cramfs_get_tree,
+	.reconfigure	= cramfs_reconfigure,
+};
+
+/*
+ * Set up the filesystem mount context.
+ */
+static int cramfs_init_fs_context(struct fs_context *fc)
+{
+	fc->ops = &cramfs_context_ops;
+	return 0;
+}
+
 static struct file_system_type cramfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "cramfs",
-	.mount		= cramfs_mount,
+	.init_fs_context = cramfs_init_fs_context,
 	.kill_sb	= cramfs_kill_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };

Re: [RFC PATCH 0/6] Convert mount_single-using filesystems to fs_context

[David Howells]

Sorry, I included the wrong cover message with this.  Will repost.

David

Re: [RFC PATCH 0/6] Convert mount_single-using filesystems to fs_context

[Richard Weinberger]

Am Donnerstag, 21. März 2019, 12:50:24 CET schrieb David Howells:
> 
> Hi Al,
> 
> Here's a set of patches that convert romfs, cramfs, jffs2 and squashfs to
> use the new fs_context struct and replace mount_mtd().  They have
> prerequisite commits (such as replacing mount_bdev()) that can be found in
> the branch detailed below.
> 
>  (1) Provide a replacement for mount_mtd() that takes an fs_context to
>      specify the parameters.
> 
>      I also put an mtd device pointer into the fs_context struct for use in
>      the sget_fc() test and set functions plus a destructor because
>      put_fs_context() can't reach into the mtd module.
> 
>  (2) Convert romfs.
> 
>  (3) Convert cramfs.
> 
>  (4) Convert jffs2.
> 
>  (5) Get rid of the now-obsolete mount_mtd() function.
> 
>  (6) Convert squashfs.
> 
> These can be found in the following branch:
> 
> 	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=mount-api-viro

Hmm, I pulled this into 5.1-rc1 to test jffs2 but it oopes already while booting:

[    5.999847] BUG: unable to handle kernel NULL pointer dereference at 0000000000000068
[    6.002273] #PF error: [normal kernel read fault]
[    6.002912] PGD 0 P4D 0 
[    6.003241] Oops: 0000 [#1] SMP PTI
[    6.003690] CPU: 3 PID: 1221 Comm: mount Not tainted 5.1.0-rc1+ #372
[    6.004494] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.11.0-0-g63451fc-prebuilt.qemu-project.org 04/01/2014
[    6.004504] RIP: 0010:reconfigure_super+0x17/0x1e0
[    6.004505] Code: 00 01 75 83 48 89 df e8 c7 e5 ff ff e9 76 ff ff ff 66 90 41 56 41 55 41 54 55 53 f7 47 68 ae ff 7f fd 48 8b 47 28 44 8b 77 64 <48> 8b 68 68 0f 85 a2 01 00 00 8b 95 38 02 00 00 85 d2 0f 85 89 01
[    6.004506] RSP: 0018:ffffb43cc0dafdf0 EFLAGS: 00010246
[    6.004507] RAX: 0000000000000000 RBX: ffff9d21b925cb00 RCX: ffff9d21b973009c
[    6.004508] RDX: 0000000000000001 RSI: ffff9d21b925cb00 RDI: ffff9d21b925cb00
[    6.004508] RBP: ffff9d21b9730000 R08: 0000000000200000 R09: 0000000000000000
[    6.004509] R10: ffffb43cc0dafe78 R11: 61c8864680b583eb R12: ffff9d21ba0cd000
[    6.004509] R13: ffffffff8d95a460 R14: 0000000000000000 R15: 0000000000000020
[    6.004512] FS:  00007fad96cea480(0000) GS:ffff9d21bab80000(0000) knlGS:0000000000000000
[    6.004514] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    6.004515] CR2: 0000000000000068 CR3: 0000000178dec000 CR4: 00000000000006e0
[    6.004516] Call Trace:
[    6.004530]  ? debugfs_get_inode+0x50/0x50
[    6.022494]  vfs_get_super+0x9c/0xe0
[    6.022973]  vfs_get_tree+0x38/0xf0
[    6.023451]  do_mount+0x728/0xd20
[    6.023905]  ? _copy_from_user+0x37/0x60
[    6.024436]  ? memdup_user+0x39/0x60
[    6.024917]  ksys_mount+0x7b/0xd0
[    6.025368]  __x64_sys_mount+0x1c/0x20
[    6.025876]  do_syscall_64+0x43/0xf0
[    6.026375]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
 
Do I miss some other fix?

Thanks,
//richard

Re: [RFC PATCH 0/6] Convert mount_single-using filesystems to fs_context

[David Howells]

Richard Weinberger <richard@nod.at> wrote:

> [    6.004504] RIP: 0010:reconfigure_super+0x17/0x1e0

Can you load the kernel into gdb and do:

	i li *reconfigure_super+0x17

and disassemble the first 32 bytes of that function for me?

> Do I miss some other fix?

Don't think so - I hadn't noticed that it had oopsed since the crash was lost
in the log and it seemed to have booted otherwise normally.

Note that I don't know if a lot of the filesystems I've converted work because
I don't have a way to test all of them.


David

Re: [RFC PATCH 0/6] Convert mount_single-using filesystems to fs_context

[David Howells]

Bah.  The problem is that vfs_get_super() doesn't set fc->root before calling
reconfigure_super().

David

Re: [RFC PATCH 0/6] Convert mount_single-using filesystems to fs_context

[Richard Weinberger]

Am Donnerstag, 21. März 2019, 16:16:26 CET schrieb David Howells:
> Bah.  The problem is that vfs_get_super() doesn't set fc->root before calling
> reconfigure_super().

Do you still need the disassembly?

Thanks,
//richard

Re: [RFC PATCH 0/6] Convert mount_single-using filesystems to fs_context

[David Howells]

Richard Weinberger <richard@nod.at> wrote:

> > Bah.  The problem is that vfs_get_super() doesn't set fc->root before
> > calling reconfigure_super().
> 
> Do you still need the disassembly?

Thanks, but no.  I've pushed replacement patches to my git branch.

David

Re: [RFC PATCH 0/6] Convert mount_single-using filesystems to fs_context

[Richard Weinberger]

Am Donnerstag, 21. März 2019, 16:45:02 CET schrieb David Howells:
> Richard Weinberger <richard@nod.at> wrote:
> 
> > > Bah.  The problem is that vfs_get_super() doesn't set fc->root before
> > > calling reconfigure_super().
> > 
> > Do you still need the disassembly?
> 
> Thanks, but no.  I've pushed replacement patches to my git branch.

Just gave it a try, jffs2 does not mount anymore. The problem is that you define
an Opt_source in super.c, but never evaluate it -> jffs2_parse_param() returns
-EINVAL.

What I really don't get is the new logging stuff in include/linux/fs_context.h.
printk() is already an overloaded beat, why another layer?

The log output seems very odd to me:
[   13.117593] e MTD: Couldn't look up '/dev/mtd0': -15

Please notice the "e" before MTD, first I though this is a memory
corruption, but is your log level?!

Thanks,
//richard

Re: [RFC PATCH 0/6] Convert mount_single-using filesystems to fs_context

[David Howells]

Richard Weinberger <richard@nod.at> wrote:

> Just gave it a try, jffs2 does not mount anymore. The problem is that you
> define an Opt_source in super.c, but never evaluate it ->
> jffs2_parse_param() returns -EINVAL.

Will fix.

> What I really don't get is the new logging stuff in
> include/linux/fs_context.h.  printk() is already an overloaded beat, why
> another layer?

Because there are some patches that aren't upstream yet that allow you to read
these out through the file descriptor you control the mount process through
rather than them going into dmesg.

> The log output seems very odd to me:
> [   13.117593] e MTD: Couldn't look up '/dev/mtd0': -15
> 
> Please notice the "e" before MTD, first I though this is a memory
> corruption, but is your log level?!

Thanks - I'll fix that.

David