Patchwork [RFC] do_mounts: Allow mtd names for non-flash block filesystems

login
register
mail settings
Submitter karl beldan
Date Oct. 6, 2010, 9:02 p.m.
Message ID <1286398974-12114-1-git-send-email-karl.beldan@gmail.com>
Download mbox | patch
Permalink /patch/66987/
State New
Headers show

Comments

karl beldan - Oct. 6, 2010, 9:02 p.m.
Hi,

I have been using this tweak for some time now, and I am getting tired of 
having to resort to it so often.
It allows to pass such cmdline root as: 
 "root=mtdb:ubivolx rootfstype=squashfs"
I am pretty sure many people use squashfs/cramfs filesystems on top of nand, 
for example, and I thought that this might initiate discussion.
karl beldan - Oct. 6, 2010, 9:22 p.m.
Edit for my off-by-one typo:

On Wed, Oct 6, 2010 at 11:02 PM, Karl Beldan <karl.beldan@gmail.com> wrote:
> Hi,
>
> I have been using this tweak for some time now, and I am getting tired of
> having to resort to it so often.
> It allows to pass such cmdline root as:
>  "root=mtdb:ubivolx rootfstype=squashfs"
> I am pretty sure many people use squashfs/cramfs filesystems on top of nand,
> for example, and I thought that this might initiate discussion.
>
> --
> Karl
>
> Signed-off-by: Karl Beldan <karl.beldan@gmail.com>
> ---
>  init/do_mounts.c |   15 +++++++++++++--
>  1 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/init/do_mounts.c b/init/do_mounts.c
> index 02e3ca4..20d9380 100644
> --- a/init/do_mounts.c
> +++ b/init/do_mounts.c
> @@ -20,6 +20,7 @@
>  #include <linux/nfs_fs.h>
>  #include <linux/nfs_fs_sb.h>
>  #include <linux/nfs_mount.h>
> +#include <linux/mtd/mtd.h>
>
>  #include "do_mounts.h"
>
> @@ -386,8 +387,18 @@ void __init prepare_namespace(void)
>
>        if (saved_root_name[0]) {
>                root_device_name = saved_root_name;
> -               if (!strncmp(root_device_name, "mtd", 3) ||
> -                   !strncmp(root_device_name, "ubi", 3)) {
> +               if (!strncmp(root_device_name, "mtdb:", 4)) {
Here, it is 5 not 4

> +                       struct mtd_info *mtd = get_mtd_device_nm(root_device_name + 4);
same here

> +                       if (!IS_ERR(mtd))
> +                               sprintf(root_device_name, "/dev/mtdblock%d", mtd->index);
> +                       else {
> +                               printk(KERN_ERR "VFS: Unable to find mtd \"%s\"\n", root_device_name + 4);
and here

> +                               printk("List of all partitions:\n");
> +                               printk_all_partitions();
> +                               panic("VFS: Unable to mount root fs");
> +                       }
> +               } else if (!strncmp(root_device_name, "mtd", 3) ||
> +                          !strncmp(root_device_name, "ubi", 3)) {
>                        mount_block_root(root_device_name, root_mountflags);
>                        goto out;
>                }
> --
> 1.7.1.422.g049e9
>
Artem Bityutskiy - Oct. 11, 2010, 7:24 p.m.
On Wed, 2010-10-06 at 23:02 +0200, Karl Beldan wrote:
> Hi,
> 
> I have been using this tweak for some time now, and I am getting tired of 
> having to resort to it so often.
> It allows to pass such cmdline root as: 
>  "root=mtdb:ubivolx rootfstype=squashfs"
> I am pretty sure many people use squashfs/cramfs filesystems on top of nand, 
> for example, and I thought that this might initiate discussion.
> 
> -- 
> Karl
> 
> Signed-off-by: Karl Beldan <karl.beldan@gmail.com>
> ---
>  init/do_mounts.c |   15 +++++++++++++--
>  1 files changed, 13 insertions(+), 2 deletions(-)

Dunno... 

First of all, if you have to do this, let it be mtdblock, not mtdb - no
need to breed aliases.

But how about teaching squashfs to understand mtdX and mtd:name syntax
instead?
karl beldan - Oct. 12, 2010, 1:16 p.m.
On Mon, Oct 11, 2010 at 9:24 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Wed, 2010-10-06 at 23:02 +0200, Karl Beldan wrote:
>> Hi,
>>
>> I have been using this tweak for some time now, and I am getting tired of
>> having to resort to it so often.
>> It allows to pass such cmdline root as:
>>  "root=mtdb:ubivolx rootfstype=squashfs"
>> I am pretty sure many people use squashfs/cramfs filesystems on top of nand,
>> for example, and I thought that this might initiate discussion.
>>
>> --
>> Karl
>>
>> Signed-off-by: Karl Beldan <karl.beldan@gmail.com>
>> ---
>>  init/do_mounts.c |   15 +++++++++++++--
>>  1 files changed, 13 insertions(+), 2 deletions(-)
>
> Dunno...
>
> First of all, if you have to do this, let it be mtdblock, not mtdb - no
> need to breed aliases.
>
> But how about teaching squashfs to understand mtdX and mtd:name syntax
> instead?
>

Hi Artem,

I would not do that, squashfs and co just ask for a block device, putting mtdx
syntax awareness in each of them just does not seem a good fit to me.

BTW 'for-2.6.37' got https://patchwork.kernel.org/patch/145681/.



Regards,
Artem Bityutskiy - Oct. 12, 2010, 2:27 p.m.
On Tue, 2010-10-12 at 15:16 +0200, Karl Beldan wrote:
> On Mon, Oct 11, 2010 at 9:24 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > On Wed, 2010-10-06 at 23:02 +0200, Karl Beldan wrote:
> >> Hi,
> >>
> >> I have been using this tweak for some time now, and I am getting tired of
> >> having to resort to it so often.
> >> It allows to pass such cmdline root as:
> >>  "root=mtdb:ubivolx rootfstype=squashfs"
> >> I am pretty sure many people use squashfs/cramfs filesystems on top of nand,
> >> for example, and I thought that this might initiate discussion.
> >>
> >> --
> >> Karl
> >>
> >> Signed-off-by: Karl Beldan <karl.beldan@gmail.com>
> >> ---
> >>  init/do_mounts.c |   15 +++++++++++++--
> >>  1 files changed, 13 insertions(+), 2 deletions(-)
> >
> > Dunno...
> >
> > First of all, if you have to do this, let it be mtdblock, not mtdb - no
> > need to breed aliases.
> >
> > But how about teaching squashfs to understand mtdX and mtd:name syntax
> > instead?
> >
> 
> Hi Artem,
> 
> I would not do that, squashfs and co just ask for a block device, putting mtdx
> syntax awareness in each of them just does not seem a good fit to me.

Right. But the same arguments applies to do_mounts.c which you hack:
mtdblock is just a block device, putting awareness of specific types of
block devices is not a good thing to do.

May be this is why I'm not too happy about your patch?

> BTW 'for-2.6.37' got https://patchwork.kernel.org/patch/145681/.

Yeah, it is better to have a generic approach which can be applied to
all block devices. Make UUID of your mtdblock to be equivalent to the
name of the underlying mtd device and you are done: use
PARTUUID=<mtd_device_name>. Right?
karl beldan - Oct. 12, 2010, 3:11 p.m.
On Tue, Oct 12, 2010 at 4:27 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Tue, 2010-10-12 at 15:16 +0200, Karl Beldan wrote:
>> On Mon, Oct 11, 2010 at 9:24 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>> > On Wed, 2010-10-06 at 23:02 +0200, Karl Beldan wrote:
>> >> Hi,
>> >>
>> >> I have been using this tweak for some time now, and I am getting tired of
>> >> having to resort to it so often.
>> >> It allows to pass such cmdline root as:
>> >>  "root=mtdb:ubivolx rootfstype=squashfs"
>> >> I am pretty sure many people use squashfs/cramfs filesystems on top of nand,
>> >> for example, and I thought that this might initiate discussion.
>> >>
>> >> --
>> >> Karl
>> >>
>> >> Signed-off-by: Karl Beldan <karl.beldan@gmail.com>
>> >> ---
>> >>  init/do_mounts.c |   15 +++++++++++++--
>> >>  1 files changed, 13 insertions(+), 2 deletions(-)
>> >
>> > Dunno...
>> >
>> > First of all, if you have to do this, let it be mtdblock, not mtdb - no
>> > need to breed aliases.
>> >
>> > But how about teaching squashfs to understand mtdX and mtd:name syntax
>> > instead?
>> >
>>
>> Hi Artem,
>>
>> I would not do that, squashfs and co just ask for a block device, putting mtdx
>> syntax awareness in each of them just does not seem a good fit to me.
>
> Right. But the same arguments applies to do_mounts.c which you hack:
To a far lesser extent, moreover do_mounts has similar hacks in that
regard, plus doing
this into squashfs you would have to do it everywhere else (e.g cramfs).
I just cannot agree, sorry.

> mtdblock is just a block device, putting awareness of specific types of
> block devices is not a good thing to do.
>
That was my point when you suggested hacking into squashfs to add mtd
names awareness.

> May be this is why I'm not too happy about your patch?
>
Neither am I, hence the way I tackled the matter.
This tweak, as I introduced it, is here to get better feedback.

>> BTW 'for-2.6.37' got https://patchwork.kernel.org/patch/145681/.
>
> Yeah, it is better to have a generic approach which can be applied to
> all block devices. Make UUID of your mtdblock to be equivalent to the
> name of the underlying mtd device and you are done: use
> PARTUUID=<mtd_device_name>. Right?
>
I put that link to bring back the discussion around something I would
more stick to
 as I anticipated the taste of the reply.

Maybe we'll come easierly to a way of handling this matter with 'next-'s.
Though the mtd subsystem is probably one that would benefit from such
a name scheme,
the solution is on its way to come from another subsystem.

Thanks,
Artem Bityutskiy - Oct. 12, 2010, 3:27 p.m.
On Tue, 2010-10-12 at 17:11 +0200, Karl Beldan wrote:
> On Tue, Oct 12, 2010 at 4:27 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > On Tue, 2010-10-12 at 15:16 +0200, Karl Beldan wrote:
> >> On Mon, Oct 11, 2010 at 9:24 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> >> > On Wed, 2010-10-06 at 23:02 +0200, Karl Beldan wrote:
> >> >> Hi,
> >> >>
> >> >> I have been using this tweak for some time now, and I am getting tired of
> >> >> having to resort to it so often.
> >> >> It allows to pass such cmdline root as:
> >> >>  "root=mtdb:ubivolx rootfstype=squashfs"
> >> >> I am pretty sure many people use squashfs/cramfs filesystems on top of nand,
> >> >> for example, and I thought that this might initiate discussion.
> >> >>
> >> >> --
> >> >> Karl
> >> >>
> >> >> Signed-off-by: Karl Beldan <karl.beldan@gmail.com>
> >> >> ---
> >> >>  init/do_mounts.c |   15 +++++++++++++--
> >> >>  1 files changed, 13 insertions(+), 2 deletions(-)
> >> >
> >> > Dunno...
> >> >
> >> > First of all, if you have to do this, let it be mtdblock, not mtdb - no
> >> > need to breed aliases.
> >> >
> >> > But how about teaching squashfs to understand mtdX and mtd:name syntax
> >> > instead?
> >> >
> >>
> >> Hi Artem,
> >>
> >> I would not do that, squashfs and co just ask for a block device, putting mtdx
> >> syntax awareness in each of them just does not seem a good fit to me.
> >
> > Right. But the same arguments applies to do_mounts.c which you hack:
> To a far lesser extent, moreover do_mounts has similar hacks in that
> regard, plus doing
> this into squashfs you would have to do it everywhere else (e.g cramfs).
> I just cannot agree, sorry.

No, they are not similar. They are for devices of different type, not
for _block_ devices. So those are about adding base support for MTD and
UBI devices.

In your case this is about block devices, which are already supported.
What you do is you say - mtd block devices are very special, they should
have own hack. Why not scsii block devices or any other types?
karl beldan - Oct. 12, 2010, 3:51 p.m.
On Tue, Oct 12, 2010 at 5:27 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Tue, 2010-10-12 at 17:11 +0200, Karl Beldan wrote:
>> On Tue, Oct 12, 2010 at 4:27 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>> > On Tue, 2010-10-12 at 15:16 +0200, Karl Beldan wrote:
>> >> On Mon, Oct 11, 2010 at 9:24 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>> >> > On Wed, 2010-10-06 at 23:02 +0200, Karl Beldan wrote:
>> >> >> Hi,
>> >> >>
>> >> >> I have been using this tweak for some time now, and I am getting tired of
>> >> >> having to resort to it so often.
>> >> >> It allows to pass such cmdline root as:
>> >> >>  "root=mtdb:ubivolx rootfstype=squashfs"
>> >> >> I am pretty sure many people use squashfs/cramfs filesystems on top of nand,
>> >> >> for example, and I thought that this might initiate discussion.
>> >> >>
>> >> >> --
>> >> >> Karl
>> >> >>
>> >> >> Signed-off-by: Karl Beldan <karl.beldan@gmail.com>
>> >> >> ---
>> >> >>  init/do_mounts.c |   15 +++++++++++++--
>> >> >>  1 files changed, 13 insertions(+), 2 deletions(-)
>> >> >
>> >> > Dunno...
>> >> >
>> >> > First of all, if you have to do this, let it be mtdblock, not mtdb - no
>> >> > need to breed aliases.
>> >> >
>> >> > But how about teaching squashfs to understand mtdX and mtd:name syntax
>> >> > instead?
>> >> >
>> >>
>> >> Hi Artem,
>> >>
>> >> I would not do that, squashfs and co just ask for a block device, putting mtdx
>> >> syntax awareness in each of them just does not seem a good fit to me.
>> >
>> > Right. But the same arguments applies to do_mounts.c which you hack:
>> To a far lesser extent, moreover do_mounts has similar hacks in that
>> regard, plus doing
>> this into squashfs you would have to do it everywhere else (e.g cramfs).
>> I just cannot agree, sorry.
>
> No, they are not similar. They are for devices of different type, not
> for _block_ devices. So those are about adding base support for MTD and
> UBI devices.
>
Well, I don't disagree with this one.

> In your case this is about block devices, which are already supported.
> What you do is you say - mtd block devices are very special, they should
> have own hack. Why not scsii block devices or any other types?
>
I am just throwing light on _a_ blatant example of the need of such
name handling.
My pointing you towards https://patchwork.kernel.org/patch/145681/,
which discusses uuids,
is explicit enough to show the pb this tweak addresses goes beyond mtd
block devices.
Said otherwise, pointing to the above patchwork patch was like 'please
don't go ''you are
assuming mtd block devices are very special'', see I checked other
people had similar issues
with different devices'.
Please, assume my pointing to the above patchwork is very
intentionnal, so that you do not
assume an 'a priori' misunderstanding nor mistake what is being discussed.
Peter Korsgaard - Oct. 13, 2010, 10:35 a.m.
>>>>> "Karl" == Karl Beldan <karl.beldan@gmail.com> writes:

Hi,

 >> But how about teaching squashfs to understand mtdX and mtd:name syntax
 >> instead?

 Karl> I would not do that, squashfs and co just ask for a block device,
 Karl> putting mtdx syntax awareness in each of them just does not seem
 Karl> a good fit to me.

FYI, patches for direct mtd access in squashfs (E.G. for systems not
needing the block layer at all) were posted a while ago:

http://thread.gmane.org/gmane.linux.kernel/962226/focus=968074
karl beldan - Oct. 13, 2010, 11:16 a.m.
On Wed, Oct 13, 2010 at 12:35 PM, Peter Korsgaard <jacmet@sunsite.dk> wrote:
>>>>>> "Karl" == Karl Beldan <karl.beldan@gmail.com> writes:
>
> Hi,
>
>  >> But how about teaching squashfs to understand mtdX and mtd:name syntax
>  >> instead?
>
>  Karl> I would not do that, squashfs and co just ask for a block device,
>  Karl> putting mtdx syntax awareness in each of them just does not seem
>  Karl> a good fit to me.
>
> FYI, patches for direct mtd access in squashfs (E.G. for systems not
> needing the block layer at all) were posted a while ago:
>
> http://thread.gmane.org/gmane.linux.kernel/962226/focus=968074
>

Hi,

This is interesting work, I saw some time ago that a french company,
Free Electrons, was offering an internship on this very subject.
If squashfs would undergo such changes it could use something like
get_sb_mtd() as jffs2 does and do_mount would require no change to
understand mtd:name, yet the naming scheme would remain not-generic
and the mtd naming scheme would remain in mtd/mtdsuper.

Patch

diff --git a/init/do_mounts.c b/init/do_mounts.c
index 02e3ca4..20d9380 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -20,6 +20,7 @@ 
 #include <linux/nfs_fs.h>
 #include <linux/nfs_fs_sb.h>
 #include <linux/nfs_mount.h>
+#include <linux/mtd/mtd.h>
 
 #include "do_mounts.h"
 
@@ -386,8 +387,18 @@  void __init prepare_namespace(void)
 
 	if (saved_root_name[0]) {
 		root_device_name = saved_root_name;
-		if (!strncmp(root_device_name, "mtd", 3) ||
-		    !strncmp(root_device_name, "ubi", 3)) {
+		if (!strncmp(root_device_name, "mtdb:", 4)) {
+			struct mtd_info *mtd = get_mtd_device_nm(root_device_name + 4);
+			if (!IS_ERR(mtd))
+				sprintf(root_device_name, "/dev/mtdblock%d", mtd->index);
+			else {
+				printk(KERN_ERR "VFS: Unable to find mtd \"%s\"\n", root_device_name + 4);
+				printk("List of all partitions:\n");
+				printk_all_partitions();
+				panic("VFS: Unable to mount root fs");
+			}
+		} else if (!strncmp(root_device_name, "mtd", 3) ||
+			   !strncmp(root_device_name, "ubi", 3)) {
 			mount_block_root(root_device_name, root_mountflags);
 			goto out;
 		}