public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
@ 2016-08-27 19:43 Daniel Golle
  2016-08-27 20:43 ` Richard Weinberger
  2016-08-28 14:12 ` Ezequiel Garcia
  0 siblings, 2 replies; 27+ messages in thread
From: Daniel Golle @ 2016-08-27 19:43 UTC (permalink / raw)
  To: linux-mtd
  Cc: Richard Weinberger, Ralph Sennhauser, Zoltan HERPAI,
	Hauke Mehrtens, lede-dev, openwrt-devel

Hi!

In an attempts to fix the flaws of the current set of UBI-related
patches we are carrying in OpenWrt, I re-wrote the way mounting the
rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
which cannot be easily addressed using other means which are already
available in the kernel is the fact that UBIFS and squashfs-on-UBI
require different parameters to be set on the cmdline, e.g.
for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1 rootfstype=squashfs

The idea behind this patchset is to provide a single syntax which
allows mouting rootfs in both cases. To achieve that, the parsing of
the volume name string from UBIFS is moved to UBI, so it can be
reused by other in-kernel users. This is then used by init/do_mounts.c
to create a ubiblock device if the filesystem on the device is
non-UBIFS. To actually set this device to be ROOT_DEV, ubiblock_create
is extended to allow passing-back the created ubiblock device.

With those changes, a single set of cmdline parameters is
sufficient to mount either UBIFS or any other block filesystem
by creating a ubiblock device:
ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs

Please comment on the code, which is to considered a loose draft hacked
up during this evening -- it's untested and certainly still has a lot
of flaws. Yet, I felt it'd be good to discuss the general direction at
this early stage.

Cheers

Daniel


Daniel Golle (3):
  UBIFS, UBI: move volume string parser from UBIFS to UBI
  mtd: ubiblock: introduce ubiblock_create_dev
  init: auto-create ubiblock device for non-UBIFS rootfs on UBI

 drivers/mtd/ubi/block.c | 11 ++++++++-
 drivers/mtd/ubi/kapi.c  | 65 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/ubi/ubi.h   |  2 ++
 fs/ubifs/super.c        | 64 +-----------------------------------------------
 include/linux/mtd/ubi.h | 17 +++++++++++++
 init/do_mounts.c        | 62 ++++++++++++++++++++++++++++++++++++++++------
 6 files changed, 150 insertions(+), 71 deletions(-)

-- 
2.9.3

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-27 19:43 [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter Daniel Golle
@ 2016-08-27 20:43 ` Richard Weinberger
  2016-08-27 21:06   ` Daniel Golle
  2016-08-28  7:10   ` Ralph Sennhauser
  2016-08-28 14:12 ` Ezequiel Garcia
  1 sibling, 2 replies; 27+ messages in thread
From: Richard Weinberger @ 2016-08-27 20:43 UTC (permalink / raw)
  To: Daniel Golle, linux-mtd
  Cc: Ralph Sennhauser, Zoltan HERPAI, Hauke Mehrtens, lede-dev,
	openwrt-devel, Boris Brezillon, Brian Norris, Ezequiel Garcia

Daniel,

On 27.08.2016 21:43, Daniel Golle wrote:
> Hi!
> 
> In an attempts to fix the flaws of the current set of UBI-related
> patches we are carrying in OpenWrt, I re-wrote the way mounting the
> rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
> which cannot be easily addressed using other means which are already
> available in the kernel is the fact that UBIFS and squashfs-on-UBI
> require different parameters to be set on the cmdline, e.g.
> for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1 rootfstype=squashfs
> 
> The idea behind this patchset is to provide a single syntax which
> allows mouting rootfs in both cases. To achieve that, the parsing of
> the volume name string from UBIFS is moved to UBI, so it can be
> reused by other in-kernel users. This is then used by init/do_mounts.c
> to create a ubiblock device if the filesystem on the device is
> non-UBIFS. To actually set this device to be ROOT_DEV, ubiblock_create
> is extended to allow passing-back the created ubiblock device.
> 
> With those changes, a single set of cmdline parameters is
> sufficient to mount either UBIFS or any other block filesystem
> by creating a ubiblock device:
> ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs

well, this all boils down to the point we have already been.
I will tell you do set the cmdline correctly, either via bootloader
or devicetree, or use an initramfs.

I can understand that OpenWrt/LEDE sometimes has to start from a hostile
bootloader which doesn't let you changing the cmdline.
But you can still set (and override) it either using CONFIG_CMDLINE or
in your device tree.
For advanced booting you can also use a inittamfs.

So, what do I miss?

Thanks,
//richard

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-27 20:43 ` Richard Weinberger
@ 2016-08-27 21:06   ` Daniel Golle
  2016-08-27 21:23     ` Richard Weinberger
  2016-08-28  7:10   ` Ralph Sennhauser
  1 sibling, 1 reply; 27+ messages in thread
From: Daniel Golle @ 2016-08-27 21:06 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: linux-mtd, Boris Brezillon, Brian Norris, Hauke Mehrtens,
	lede-dev, Ezequiel Garcia, Zoltan HERPAI, Ralph Sennhauser,
	openwrt-devel

Hi Richard,

thanks for the quick reply!

On Sat, Aug 27, 2016 at 10:43:45PM +0200, Richard Weinberger wrote:
> Daniel,
> 
> On 27.08.2016 21:43, Daniel Golle wrote:
> > Hi!
> > 
> > In an attempts to fix the flaws of the current set of UBI-related
> > patches we are carrying in OpenWrt, I re-wrote the way mounting the
> > rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
> > which cannot be easily addressed using other means which are already
> > available in the kernel is the fact that UBIFS and squashfs-on-UBI
> > require different parameters to be set on the cmdline, e.g.
> > for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> > for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1 rootfstype=squashfs
> > 
> > The idea behind this patchset is to provide a single syntax which
> > allows mouting rootfs in both cases. To achieve that, the parsing of
> > the volume name string from UBIFS is moved to UBI, so it can be
> > reused by other in-kernel users. This is then used by init/do_mounts.c
> > to create a ubiblock device if the filesystem on the device is
> > non-UBIFS. To actually set this device to be ROOT_DEV, ubiblock_create
> > is extended to allow passing-back the created ubiblock device.
> > 
> > With those changes, a single set of cmdline parameters is
> > sufficient to mount either UBIFS or any other block filesystem
> > by creating a ubiblock device:
> > ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs
> 
> well, this all boils down to the point we have already been.
> I will tell you do set the cmdline correctly, either via bootloader
> or devicetree, or use an initramfs.

I got that points and this patchset is meant to made exactly that
possible -- using the kernel cmdline to specify the rootfs.
However, usually the filesystem type doesn't need to be explicitely
taken into account by the bootloader.

> 
> I can understand that OpenWrt/LEDE sometimes has to start from a hostile
> bootloader which doesn't let you changing the cmdline.

On most systems, *users* do not have bootloader access. Surely, our
build-system could generate images with different built-in cmdlines
depending on the bundled filesystem types.

> But you can still set (and override) it either using CONFIG_CMDLINE or
> in your device tree.
> For advanced booting you can also use a inittamfs.
> 
> So, what do I miss?

On block devices, GRUB doesn't need to set any parameters differently
depending on the rootfstype being squashfs, xfs or ext4, right?
On UBI, this assumption no longer holds true, which imho is the
remaining core of the problem, which I meant to illustrate using the
example in the previous message I've sent.
Do you really explect that the bootloader should probe the rootfs-type
and based on that, re-write the cmdline parameters?
Having the option to mount other filesystems and transparently create
a ubiblock device using the same syntax as used for ubifs allows to
overcome that.
Also note the nasty details when using the current syntax which alone
should be a good reason to unify things:
ubi.mtd=ubi ubiblock=0,rootfs root=/dev/ubiblock0_1 rootfstype=squashfs
                       ^^^^^^                   ^^^
ubiblock allows creating a block device by volume name. However, in
order to specify the root= parameter, one would need to know the
vol_id of the volume previously addressed by its name.
Just having "ubi.mtd=ubi root=ubi0:rootfs rootfstype=squashfs,ubifs"
looks nicer than that, doesn't it?

> 
> Thanks,
> //richard
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-27 21:06   ` Daniel Golle
@ 2016-08-27 21:23     ` Richard Weinberger
  2016-08-27 23:13       ` Daniel Golle
  0 siblings, 1 reply; 27+ messages in thread
From: Richard Weinberger @ 2016-08-27 21:23 UTC (permalink / raw)
  To: Daniel Golle
  Cc: linux-mtd, Boris Brezillon, Brian Norris, Hauke Mehrtens,
	lede-dev, Ezequiel Garcia, Zoltan HERPAI, Ralph Sennhauser,
	openwrt-devel

Daniel,

On 27.08.2016 23:06, Daniel Golle wrote:
> Hi Richard,
> 
> thanks for the quick reply!
> 
> On Sat, Aug 27, 2016 at 10:43:45PM +0200, Richard Weinberger wrote:
>> Daniel,
>>
>> On 27.08.2016 21:43, Daniel Golle wrote:
>>> Hi!
>>>
>>> In an attempts to fix the flaws of the current set of UBI-related
>>> patches we are carrying in OpenWrt, I re-wrote the way mounting the
>>> rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
>>> which cannot be easily addressed using other means which are already
>>> available in the kernel is the fact that UBIFS and squashfs-on-UBI
>>> require different parameters to be set on the cmdline, e.g.
>>> for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
>>> for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1 rootfstype=squashfs
>>>
>>> The idea behind this patchset is to provide a single syntax which
>>> allows mouting rootfs in both cases. To achieve that, the parsing of
>>> the volume name string from UBIFS is moved to UBI, so it can be
>>> reused by other in-kernel users. This is then used by init/do_mounts.c
>>> to create a ubiblock device if the filesystem on the device is
>>> non-UBIFS. To actually set this device to be ROOT_DEV, ubiblock_create
>>> is extended to allow passing-back the created ubiblock device.
>>>
>>> With those changes, a single set of cmdline parameters is
>>> sufficient to mount either UBIFS or any other block filesystem
>>> by creating a ubiblock device:
>>> ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs
>>
>> well, this all boils down to the point we have already been.
>> I will tell you do set the cmdline correctly, either via bootloader
>> or devicetree, or use an initramfs.
> 
> I got that points and this patchset is meant to made exactly that
> possible -- using the kernel cmdline to specify the rootfs.
> However, usually the filesystem type doesn't need to be explicitely
> taken into account by the bootloader.
> 
>>
>> I can understand that OpenWrt/LEDE sometimes has to start from a hostile
>> bootloader which doesn't let you changing the cmdline.
> 
> On most systems, *users* do not have bootloader access. Surely, our
> build-system could generate images with different built-in cmdlines
> depending on the bundled filesystem types.

Since you have to provide anyways per-device images because the flash
type/geometric of each device are different it wouldn't hurt having
a per device cmdline too, right?

>> But you can still set (and override) it either using CONFIG_CMDLINE or
>> in your device tree.
>> For advanced booting you can also use a inittamfs.
>>
>> So, what do I miss?
> 
> On block devices, GRUB doesn't need to set any parameters differently
> depending on the rootfstype being squashfs, xfs or ext4, right?

No, this works only for trivial storage stacks on x86 when booting from a
disk.
For squashfs you need an initramfs anyways on most setups
since squashfs is stored as regular file.

For everything non-trivial, may it RAID, LVM or iSCSI, it won't work
automagically.

> On UBI, this assumption no longer holds true, which imho is the
> remaining core of the problem, which I meant to illustrate using the
> example in the previous message I've sent.

Having squashfs on top of ubiblock which is ont op of UBI is something
non-trivial IMHO.

> Do you really explect that the bootloader should probe the rootfs-type
> and based on that, re-write the cmdline parameters?

No, the bootloader should *know* on what device it runs on and has to set
the cmdline correctly. Exactly as we do on every embedded system for ever.
I don't think OpenWrt/LEDE is so special.

> Just having "ubi.mtd=ubi root=ubi0:rootfs rootfstype=squashfs,ubifs"
> looks nicer than that, doesn't it?

I don't agree. Adding more auto probe hacks to the kernel for the sake
of looking nice are not worth it.
Since you have to have *at least* a per device device tree it is perfectly
fine adding the correct rootfs cmdline to it.
You also have to specify per device in your device tree the correct MTD
layout for each device.

Thanks,
//richard

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-27 21:23     ` Richard Weinberger
@ 2016-08-27 23:13       ` Daniel Golle
  2016-08-27 23:33         ` Richard Weinberger
  0 siblings, 1 reply; 27+ messages in thread
From: Daniel Golle @ 2016-08-27 23:13 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Boris Brezillon, openwrt-devel, Hauke Mehrtens, lede-dev,
	linux-mtd, Ezequiel Garcia, Zoltan HERPAI, Ralph Sennhauser,
	Brian Norris

Hi Richard,

On Sat, Aug 27, 2016 at 11:23:14PM +0200, Richard Weinberger wrote:
> Daniel,
> 
> On 27.08.2016 23:06, Daniel Golle wrote:
> > Hi Richard,
> > 
> > thanks for the quick reply!
> > 
> > On Sat, Aug 27, 2016 at 10:43:45PM +0200, Richard Weinberger wrote:
> >> Daniel,
> >>
> >> On 27.08.2016 21:43, Daniel Golle wrote:
> >>> Hi!
> >>>
> >>> In an attempts to fix the flaws of the current set of UBI-related
> >>> patches we are carrying in OpenWrt, I re-wrote the way mounting the
> >>> rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
> >>> which cannot be easily addressed using other means which are already
> >>> available in the kernel is the fact that UBIFS and squashfs-on-UBI
> >>> require different parameters to be set on the cmdline, e.g.
> >>> for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> >>> for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1 rootfstype=squashfs
> >>>
> >>> The idea behind this patchset is to provide a single syntax which
> >>> allows mouting rootfs in both cases. To achieve that, the parsing of
> >>> the volume name string from UBIFS is moved to UBI, so it can be
> >>> reused by other in-kernel users. This is then used by init/do_mounts.c
> >>> to create a ubiblock device if the filesystem on the device is
> >>> non-UBIFS. To actually set this device to be ROOT_DEV, ubiblock_create
> >>> is extended to allow passing-back the created ubiblock device.
> >>>
> >>> With those changes, a single set of cmdline parameters is
> >>> sufficient to mount either UBIFS or any other block filesystem
> >>> by creating a ubiblock device:
> >>> ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs
> >>
> >> well, this all boils down to the point we have already been.
> >> I will tell you do set the cmdline correctly, either via bootloader
> >> or devicetree, or use an initramfs.
> > 
> > I got that points and this patchset is meant to made exactly that
> > possible -- using the kernel cmdline to specify the rootfs.
> > However, usually the filesystem type doesn't need to be explicitely
> > taken into account by the bootloader.
> > 
> >>
> >> I can understand that OpenWrt/LEDE sometimes has to start from a hostile
> >> bootloader which doesn't let you changing the cmdline.
> > 
> > On most systems, *users* do not have bootloader access. Surely, our
> > build-system could generate images with different built-in cmdlines
> > depending on the bundled filesystem types.
> 
> Since you have to provide anyways per-device images because the flash
> type/geometric of each device are different it wouldn't hurt having
> a per device cmdline too, right?

Right, but on the same device using the same kernel you might have
ubifs or squashfs as rootfs when using OpenWrt/LEDE and the like.
Why should the bootloader (or even the device-tree) contain any
knowledge about that?
I reckon we are both aware that re-installing or modifying the
bootloader is far less common on non-x86 targets and is usually avoided
to minimize risks and added complexity to OS installers.

> 
> >> But you can still set (and override) it either using CONFIG_CMDLINE or
> >> in your device tree.
> >> For advanced booting you can also use a inittamfs.
> >>
> >> So, what do I miss?
> > 
> > On block devices, GRUB doesn't need to set any parameters differently
> > depending on the rootfstype being squashfs, xfs or ext4, right?
> 
> No, this works only for trivial storage stacks on x86 when booting from a
> disk.
> For squashfs you need an initramfs anyways on most setups
> since squashfs is stored as regular file.

I've never seen that (squashfs loaded from a filesystem which was
mounted by what's running on the initramfs) on any small embedded
device in my life. *That's* a very complex setup.
Most commonly, squashfs is mounted by the kernel from /dev/mtdblock*.
In the past, most people who were force to use UBI then ended up using
gluebi and then mounting squashfs off the gluebi mtdblock device -- I
understood that ubiblock is intended to replace those legacy hacks and
meant to be the official way to mount non-UBIFS read-only filesystems
store on UBI volumes, right?

> 
> For everything non-trivial, may it RAID, LVM or iSCSI, it won't work
> automagically.

With that in mind, why should a simple system which anyway stores the
kernel and filesystem on the same physical media have an initramfs?

> 
> > On UBI, this assumption no longer holds true, which imho is the
> > remaining core of the problem, which I meant to illustrate using the
> > example in the previous message I've sent.
> 
> Having squashfs on top of ubiblock which is ont op of UBI is something
> non-trivial IMHO.

Ok, so that seems to be the essence of our disagreement. From my
perspective, this setup seems to be the most trivial possible on small
embeeded devices which come with NAND flash and thus do benefit from
using UBI.

> 
> > Do you really explect that the bootloader should probe the rootfs-type
> > and based on that, re-write the cmdline parameters?
> 
> No, the bootloader should *know* on what device it runs on and has to set
> the cmdline correctly. Exactly as we do on every embedded system for ever.
> I don't think OpenWrt/LEDE is so special.

I certainly agree about that. However, depending on the use-case and
stage of development (!), one might choose to use UBIFS as read-write
rootfs or to use squashfs and have a overlay on-top. Thus, the rootfs-
type used on the exact same device may vary, also during the device's
lifecycle.
Using UBIFS as rootfs allows granular rolling-release-style updates
and makes prototyping easy. On more robust production firmware, a
read-only image with either a read-write overlay or just some plain
configuration allows easily reverting to the factory-defaults and
updating the whole system with a replacement image.

> 
> > Just having "ubi.mtd=ubi root=ubi0:rootfs rootfstype=squashfs,ubifs"
> > looks nicer than that, doesn't it?
> 
> I don't agree. Adding more auto probe hacks to the kernel for the sake
> of looking nice are not worth it.

The value I advertise is functional, not just esthetical.
By the end of the day, probing the rootfs-type will to live somewhere.
I agree that for complex setups, an initramfs should be used. However,
an easy an generic way to boot also without mandatory initramfs should
still be possible, even on boards having NAND flash.
In the current (whacky) way we roll UBI support in the kernel, it also
allows the same binary kernel to be used for other distributions (eg.
Debian or archlinux with an UBIFS rootfs on hardware not officially
supported by those distributions)
This added value of having a universal kernel would be lost if high-
level decissions such as the filesystem type would be dictated by
either built-in kernel parameters, the device-tree or the bootloader.
My intention is to preserve that feature in a way which still shouldn't
hard-code the rootfs volume name (which it does, forcing more complex
bootloader setups to come up with a great variety of vendor-specific
implementations).

> Since you have to have *at least* a per device device tree it is perfectly
> fine adding the correct rootfs cmdline to it.
> You also have to specify per device in your device tree the correct MTD
> layout for each device.

Same argument as above: this doesn't imply that the filesystem type
used as a rootfs is the same for all use-cases (and even more so, all
other distributions to run on the same board, ideally without
modifications to the bootloader, it's evironment or the device-tree).

All that being said, I agree that none of the other patches are truely
needed. However, if the cmdline cannot be rootfstype-agnostic, I'll
have to work my way around that. I'll happily work on making that
possible and improve things where needed. I'm still hoping for
constructive criticism on how a generic and thus also rootfstype-
agnostic kernel can truly be achieved *without* using an initramfs on
simple devices.


Cheers


Daniel


> 
> Thanks,
> //richard
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-27 23:13       ` Daniel Golle
@ 2016-08-27 23:33         ` Richard Weinberger
  2016-08-28  6:47           ` Boris Brezillon
  0 siblings, 1 reply; 27+ messages in thread
From: Richard Weinberger @ 2016-08-27 23:33 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Boris Brezillon, openwrt-devel, Hauke Mehrtens, lede-dev,
	linux-mtd, Ezequiel Garcia, Zoltan HERPAI, Ralph Sennhauser,
	Brian Norris

Daniel,

On 28.08.2016 01:13, Daniel Golle wrote:
>> Since you have to provide anyways per-device images because the flash
>> type/geometric of each device are different it wouldn't hurt having
>> a per device cmdline too, right?
> 
> Right, but on the same device using the same kernel you might have
> ubifs or squashfs as rootfs when using OpenWrt/LEDE and the like.
> Why should the bootloader (or even the device-tree) contain any
> knowledge about that?

At some point you have to tell the kernel from where it has to obtain
the rootfs from.
Yes, for basic stuff we have auto probing but on a decent system you
always have to specify exactly what the rootfs is.

>> No, this works only for trivial storage stacks on x86 when booting from a
>> disk.
>> For squashfs you need an initramfs anyways on most setups
>> since squashfs is stored as regular file.
> 
> I've never seen that (squashfs loaded from a filesystem which was
> mounted by what's running on the initramfs) on any small embedded
> device in my life. *That's* a very complex setup.

It all depends on the use case.

> Most commonly, squashfs is mounted by the kernel from /dev/mtdblock*.
> In the past, most people who were force to use UBI then ended up using
> gluebi and then mounting squashfs off the gluebi mtdblock device -- I
> understood that ubiblock is intended to replace those legacy hacks and
> meant to be the official way to mount non-UBIFS read-only filesystems
> store on UBI volumes, right?

Yes. But not for magic autoprobing.

>>
>> For everything non-trivial, may it RAID, LVM or iSCSI, it won't work
>> automagically.
> 
> With that in mind, why should a simple system which anyway stores the
> kernel and filesystem on the same physical media have an initramfs?

You seem to hate initramfs, that's your problem, not mine :-)
But you don't depend on initramfs. It is just one more option to
solve your issue.

>>> Do you really explect that the bootloader should probe the rootfs-type
>>> and based on that, re-write the cmdline parameters?
>>
>> No, the bootloader should *know* on what device it runs on and has to set
>> the cmdline correctly. Exactly as we do on every embedded system for ever.
>> I don't think OpenWrt/LEDE is so special.
> 
> I certainly agree about that. However, depending on the use-case and
> stage of development (!), one might choose to use UBIFS as read-write
> rootfs or to use squashfs and have a overlay on-top. Thus, the rootfs-
> type used on the exact same device may vary, also during the device's
> lifecycle.
> Using UBIFS as rootfs allows granular rolling-release-style updates
> and makes prototyping easy. On more robust production firmware, a
> read-only image with either a read-write overlay or just some plain
> configuration allows easily reverting to the factory-defaults and
> updating the whole system with a replacement image.

I'll not accept such patches for the sake of supporting lazy development
style. Fix your buildsystem to produce the correct image for your target
device and the correct mode.
Development and production images are a common thing in embedded
world.

> All that being said, I agree that none of the other patches are truely
> needed. However, if the cmdline cannot be rootfstype-agnostic, I'll
> have to work my way around that. I'll happily work on making that
> possible and improve things where needed. I'm still hoping for
> constructive criticism on how a generic and thus also rootfstype-
> agnostic kernel can truly be achieved *without* using an initramfs on
> simple devices.

For your use case you are not forced to have an initramfs.
All you need is setting cmdline correctly. Either via DT, bootloader
or kernel .config.

So, yes, your patches are not needed at all.
They may support your current style of development but it does not
justify a kernel change.

Thanks,
//richard

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-27 23:33         ` Richard Weinberger
@ 2016-08-28  6:47           ` Boris Brezillon
  0 siblings, 0 replies; 27+ messages in thread
From: Boris Brezillon @ 2016-08-28  6:47 UTC (permalink / raw)
  To: Richard Weinberger, Daniel Golle
  Cc: Brian Norris, Hauke Mehrtens, lede-dev, linux-mtd,
	Ezequiel Garcia, Zoltan HERPAI, Ralph Sennhauser, openwrt-devel

Daniel, Richard,

On Sun, 28 Aug 2016 01:33:28 +0200
Richard Weinberger <richard@nod.at> wrote:

> Daniel,
> 
> On 28.08.2016 01:13, Daniel Golle wrote:
> >> Since you have to provide anyways per-device images because the flash
> >> type/geometric of each device are different it wouldn't hurt having
> >> a per device cmdline too, right?  
> > 
> > Right, but on the same device using the same kernel you might have
> > ubifs or squashfs as rootfs when using OpenWrt/LEDE and the like.
> > Why should the bootloader (or even the device-tree) contain any
> > knowledge about that?  
> 
> At some point you have to tell the kernel from where it has to obtain
> the rootfs from.
> Yes, for basic stuff we have auto probing but on a decent system you
> always have to specify exactly what the rootfs is.
> 
> >> No, this works only for trivial storage stacks on x86 when booting from a
> >> disk.
> >> For squashfs you need an initramfs anyways on most setups
> >> since squashfs is stored as regular file.  
> > 
> > I've never seen that (squashfs loaded from a filesystem which was
> > mounted by what's running on the initramfs) on any small embedded
> > device in my life. *That's* a very complex setup.  
> 
> It all depends on the use case.
> 
> > Most commonly, squashfs is mounted by the kernel from /dev/mtdblock*.
> > In the past, most people who were force to use UBI then ended up using
> > gluebi and then mounting squashfs off the gluebi mtdblock device -- I
> > understood that ubiblock is intended to replace those legacy hacks and
> > meant to be the official way to mount non-UBIFS read-only filesystems
> > store on UBI volumes, right?  
> 
> Yes. But not for magic autoprobing.
> 
> >>
> >> For everything non-trivial, may it RAID, LVM or iSCSI, it won't work
> >> automagically.  
> > 
> > With that in mind, why should a simple system which anyway stores the
> > kernel and filesystem on the same physical media have an initramfs?  
> 
> You seem to hate initramfs, that's your problem, not mine :-)
> But you don't depend on initramfs. It is just one more option to
> solve your issue.
> 
> >>> Do you really explect that the bootloader should probe the rootfs-type
> >>> and based on that, re-write the cmdline parameters?  
> >>
> >> No, the bootloader should *know* on what device it runs on and has to set
> >> the cmdline correctly. Exactly as we do on every embedded system for ever.
> >> I don't think OpenWrt/LEDE is so special.  
> > 
> > I certainly agree about that. However, depending on the use-case and
> > stage of development (!), one might choose to use UBIFS as read-write
> > rootfs or to use squashfs and have a overlay on-top. Thus, the rootfs-
> > type used on the exact same device may vary, also during the device's
> > lifecycle.
> > Using UBIFS as rootfs allows granular rolling-release-style updates
> > and makes prototyping easy. On more robust production firmware, a
> > read-only image with either a read-write overlay or just some plain
> > configuration allows easily reverting to the factory-defaults and
> > updating the whole system with a replacement image.  
> 
> I'll not accept such patches for the sake of supporting lazy development
> style. Fix your buildsystem to produce the correct image for your target
> device and the correct mode.
> Development and production images are a common thing in embedded
> world.
> 
> > All that being said, I agree that none of the other patches are truely
> > needed. However, if the cmdline cannot be rootfstype-agnostic, I'll
> > have to work my way around that. I'll happily work on making that
> > possible and improve things where needed. I'm still hoping for
> > constructive criticism on how a generic and thus also rootfstype-
> > agnostic kernel can truly be achieved *without* using an initramfs on
> > simple devices.  
> 
> For your use case you are not forced to have an initramfs.
> All you need is setting cmdline correctly. Either via DT, bootloader
> or kernel .config.
> 
> So, yes, your patches are not needed at all.
> They may support your current style of development but it does not
> justify a kernel change.

I agree with all Richard said so far. You're trying to add dirty hacks
in the kernel because you don't want to use the recommended way to
handle complex rootfs selection.

Think about the root= parameter. It's here to specify where your rootfs
will be mounted from, but you can only have one device specified here,
and if the system has to choose between different devices, then an
initramfs is used. In the UBI case, ubiblockX_Y and ubiX_Y are two
different devices from the kernel PoV, and it should stay like that.

Regards,

Boris

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-27 20:43 ` Richard Weinberger
  2016-08-27 21:06   ` Daniel Golle
@ 2016-08-28  7:10   ` Ralph Sennhauser
  2016-08-28  8:12     ` Richard Weinberger
  1 sibling, 1 reply; 27+ messages in thread
From: Ralph Sennhauser @ 2016-08-28  7:10 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Daniel Golle, linux-mtd, Zoltan HERPAI, Hauke Mehrtens, lede-dev,
	openwrt-devel, Boris Brezillon, Brian Norris, Ezequiel Garcia

Hi Richard,

On Sat, 27 Aug 2016 22:43:45 +0200
Richard Weinberger <richard@nod.at> wrote:

> Daniel,
> 
> On 27.08.2016 21:43, Daniel Golle wrote:
> > Hi!
> > 
> > In an attempts to fix the flaws of the current set of UBI-related
> > patches we are carrying in OpenWrt, I re-wrote the way mounting the
> > rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
> > which cannot be easily addressed using other means which are already
> > available in the kernel is the fact that UBIFS and squashfs-on-UBI
> > require different parameters to be set on the cmdline, e.g.
> > for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> > for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1
> > rootfstype=squashfs
> > 
> > The idea behind this patchset is to provide a single syntax which
> > allows mouting rootfs in both cases. To achieve that, the parsing of
> > the volume name string from UBIFS is moved to UBI, so it can be
> > reused by other in-kernel users. This is then used by
> > init/do_mounts.c to create a ubiblock device if the filesystem on
> > the device is non-UBIFS. To actually set this device to be
> > ROOT_DEV, ubiblock_create is extended to allow passing-back the
> > created ubiblock device.
> > 
> > With those changes, a single set of cmdline parameters is
> > sufficient to mount either UBIFS or any other block filesystem
> > by creating a ubiblock device:
> > ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs  
> 
> well, this all boils down to the point we have already been.
> I will tell you do set the cmdline correctly, either via bootloader
> or devicetree, or use an initramfs.
> 
> I can understand that OpenWrt/LEDE sometimes has to start from a
> hostile bootloader which doesn't let you changing the cmdline.
> But you can still set (and override) it either using CONFIG_CMDLINE or
> in your device tree.
> For advanced booting you can also use a inittamfs.
> 
> So, what do I miss?
> 
> Thanks,
> //richard

Using CONFIG_CMDLINE or the dtb isn't an option either for dual firmware
devices. You'd have to provide two images, one for each partition so
the rootfs belonging to the kernel gets mounted. Sounds like a recipe
for disaster.

On the other hand an initramfs can carry the logic to figure out which
to mount and is what I use for my self. The busybox based implementation
I use adds a tad over 300Kb to the uImage, perfectly acceptable in my
case.

Cheers
Ralph

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28  7:10   ` Ralph Sennhauser
@ 2016-08-28  8:12     ` Richard Weinberger
  2016-08-28  9:19       ` Ralph Sennhauser
  0 siblings, 1 reply; 27+ messages in thread
From: Richard Weinberger @ 2016-08-28  8:12 UTC (permalink / raw)
  To: Ralph Sennhauser
  Cc: Daniel Golle, linux-mtd, Zoltan HERPAI, Hauke Mehrtens, lede-dev,
	openwrt-devel, Boris Brezillon, Brian Norris, Ezequiel Garcia

Ralph,

On 28.08.2016 09:10, Ralph Sennhauser wrote:
> Using CONFIG_CMDLINE or the dtb isn't an option either for dual firmware
> devices. You'd have to provide two images, one for each partition so
> the rootfs belonging to the kernel gets mounted. Sounds like a recipe
> for disaster.

With "image" you mean the uImage?
Well, then you need to add the cmdline to the DT.
If your bootloader does not support DT loading and you need to append
it to uImage, yes, you'll have to two uImage for these devices.
Or a initramfs...

But IMHO it still does not justify adding these hacks to the kernel.

> On the other hand an initramfs can carry the logic to figure out which
> to mount and is what I use for my self. The busybox based implementation
> I use adds a tad over 300Kb to the uImage, perfectly acceptable in my
> case.

When your minimal initramfs consumes 300KiB you're doing something wrong.
As I said in another thread, for your special purpose you'd need to create
a minitmal userspace for initramfs, no fancy (eg)libc, just a bare minimum
/init program which does the mount probing. Shouldn’t be more than a few
system calls.

Thanks,
//richard

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28  8:12     ` Richard Weinberger
@ 2016-08-28  9:19       ` Ralph Sennhauser
  2016-08-28  9:28         ` Richard Weinberger
  0 siblings, 1 reply; 27+ messages in thread
From: Ralph Sennhauser @ 2016-08-28  9:19 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Daniel Golle, linux-mtd, Zoltan HERPAI, Hauke Mehrtens, lede-dev,
	openwrt-devel, Boris Brezillon, Brian Norris, Ezequiel Garcia

Hi Richard,

On Sun, 28 Aug 2016 10:12:31 +0200
Richard Weinberger <richard@nod.at> wrote:

> Ralph,
> 
> On 28.08.2016 09:10, Ralph Sennhauser wrote:
> > Using CONFIG_CMDLINE or the dtb isn't an option either for dual
> > firmware devices. You'd have to provide two images, one for each
> > partition so the rootfs belonging to the kernel gets mounted.
> > Sounds like a recipe for disaster.  
> 
> With "image" you mean the uImage?

uImage - padding to 6MB - ubinized rootfs. A firmware image as is
flashed by users.

Linksys set it up so that if one firmware image is broken there is a
good chance the bootloader will attempt to boot the other.

> Well, then you need to add the cmdline to the DT.
> If your bootloader does not support DT loading and you need to append
> it to uImage, yes, you'll have to two uImage for these devices.

Two uImages / firmware images is probably more problematic than asking
the user to fix the kernel parameters passed by u-boot.

> Or a initramfs...
> 
> But IMHO it still does not justify adding these hacks to the kernel.
> 

Those hacks can be justified if there is a case an initramfs or
CONFIG_CMDLINE/dtb doesn't work. I can't think of such a case right
now.

> > On the other hand an initramfs can carry the logic to figure out
> > which to mount and is what I use for my self. The busybox based
> > implementation I use adds a tad over 300Kb to the uImage, perfectly
> > acceptable in my case.  
> 
> When your minimal initramfs consumes 300KiB you're doing something
> wrong. As I said in another thread, for your special purpose you'd
> need to create a minitmal userspace for initramfs, no fancy (eg)libc,
> just a bare minimum /init program which does the mount probing.
> Shouldn’t be more than a few system calls.
> 
> Thanks,
> //richard

Well, I use busybox because I'm lazy and still get away with only 300Kb.
And as I said there is plenty space on my device. (6M per uImage OEM
firmware configuration)

Cheers
Ralph

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28  9:19       ` Ralph Sennhauser
@ 2016-08-28  9:28         ` Richard Weinberger
  2016-08-28 11:44           ` Daniel Golle
  0 siblings, 1 reply; 27+ messages in thread
From: Richard Weinberger @ 2016-08-28  9:28 UTC (permalink / raw)
  To: Ralph Sennhauser
  Cc: Daniel Golle, linux-mtd, Zoltan HERPAI, Hauke Mehrtens, lede-dev,
	openwrt-devel, Boris Brezillon, Brian Norris, Ezequiel Garcia

Ralph,

On 28.08.2016 11:19, Ralph Sennhauser wrote:
>>> On the other hand an initramfs can carry the logic to figure out
>>> which to mount and is what I use for my self. The busybox based
>>> implementation I use adds a tad over 300Kb to the uImage, perfectly
>>> acceptable in my case.  
>>
>> When your minimal initramfs consumes 300KiB you're doing something
>> wrong. As I said in another thread, for your special purpose you'd
>> need to create a minitmal userspace for initramfs, no fancy (eg)libc,
>> just a bare minimum /init program which does the mount probing.
>> Shouldn’t be more than a few system calls.
>>
>> Thanks,
>> //richard
> 
> Well, I use busybox because I'm lazy and still get away with only 300Kb.
> And as I said there is plenty space on my device. (6M per uImage OEM
> firmware configuration)

So, problem solved. Use an initramfs. :-)

</discussion>,
//richard

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28  9:28         ` Richard Weinberger
@ 2016-08-28 11:44           ` Daniel Golle
  2016-08-28 11:57             ` Richard Weinberger
                               ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Daniel Golle @ 2016-08-28 11:44 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Ralph Sennhauser, linux-mtd, Zoltan HERPAI, Hauke Mehrtens,
	lede-dev, openwrt-devel, Boris Brezillon, Brian Norris,
	Ezequiel Garcia

Hi Richard,

On Sun, Aug 28, 2016 at 11:28:18AM +0200, Richard Weinberger wrote:
> Ralph,
> 
> On 28.08.2016 11:19, Ralph Sennhauser wrote:
> >>> On the other hand an initramfs can carry the logic to figure out
> >>> which to mount and is what I use for my self. The busybox based
> >>> implementation I use adds a tad over 300Kb to the uImage, perfectly
> >>> acceptable in my case.  
> >>
> >> When your minimal initramfs consumes 300KiB you're doing something
> >> wrong. As I said in another thread, for your special purpose you'd
> >> need to create a minitmal userspace for initramfs, no fancy (eg)libc,
> >> just a bare minimum /init program which does the mount probing.
> >> Shouldn’t be more than a few system calls.

That would indeed be nice, however, I fail to see how this can work
with little effort *before* having devtmpfs ready. Also, we'll need a
non-standard kernel parameter (usually "real_root=") to pass the
selected rootfs down to our to-be-implemented micro-sized initramfs.

> >>
> >> Thanks,
> >> //richard
> > 
> > Well, I use busybox because I'm lazy and still get away with only 300Kb.
> > And as I said there is plenty space on my device. (6M per uImage OEM
> > firmware configuration)
> 
> So, problem solved. Use an initramfs. :-)

I agree this might be acceptable for some, but certainly not all
cases. Unlike to your previous statement, I'm not generally opposed
to having an initramfs included in the kernel as that can also provide
other nice features such as recovery/failsafe methods.
We've had this discussion before and my goal is, as I explained, to
make the kernel as reusable as possible and even allow people to use
OpenWrt/LEDE's kernel binary with other distributions. I'm sure
this is also possible when using an initramfs, however, I still fail to
see the necessity for that on simple devices.
Imho, using an initramfs shouldn't be mandatory which is why there are
patches (now down to about 80 LoC, resulting in probably less than 1kB
in added binary size) to mount the rootfs without the need of an
initramfs, simply because I do not consider that to be a
"complex setup" if there is no less complex and yet generic way
imaginable to boot on that platform at all.
Hence I'm not buying the argument that ubiX_Y and ubiblockX_Y are two
different devices and thus, this is a device selection problem and thus
initramfs is the recommended way -- in fact, all other filesystems
which do *not* build upon a block device provide some probing hacks in
early userspace. Take MTD as an example: mtdblock devices are
automagically provided and needed for block-based filesystems, no need
for initramfs or kernel parameters to achieve that. For UBI, ubiblock
(or gluebi...) is required to use UBI for anything else than UBIFS.
Anyway, I'm afraid you have made your decission and additional
arguments have no influence what-so-ever.

> 
> </discussion>,

If that's the whole answer ("Use initramfs. End of story."), that's
pretty disappointing. Dispite your previous invitation to discuss the
matter and collaborate to address the needs of all parties involved,
you are now offering only one option which is not agreeable by all
parties (which is the obvious reason for those nasty patches to exist
in first place). Carrying a few patches in our local overlay doesn't
truly hurt in a technical sense, however, it'd be nicer to discuss how
those features could be brought upstream or mitigate our local patches
in other ways.
In the case of not finding room for a debate and your answer is a
straight "we don't want this feature upstream" this is never the less
a good reference to remember when touching those patches in future:
falls into "we tried, they didn't want it" and thus we'll keep carrying
them around, at least as long as there are platforms needing them.


Cheers


Daniel

> //richard

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28 11:44           ` Daniel Golle
@ 2016-08-28 11:57             ` Richard Weinberger
  2016-08-28 13:47               ` Daniel Golle
  2016-08-28 12:10             ` Ralph Sennhauser
  2016-08-28 13:24             ` Boris Brezillon
  2 siblings, 1 reply; 27+ messages in thread
From: Richard Weinberger @ 2016-08-28 11:57 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Ralph Sennhauser, linux-mtd, Zoltan HERPAI, Hauke Mehrtens,
	lede-dev, openwrt-devel, Boris Brezillon, Brian Norris,
	Ezequiel Garcia

Daniel,

On 28.08.2016 13:44, Daniel Golle wrote:
> Hi Richard,
> 
> On Sun, Aug 28, 2016 at 11:28:18AM +0200, Richard Weinberger wrote:
>> Ralph,
>>
>> On 28.08.2016 11:19, Ralph Sennhauser wrote:
>>>>> On the other hand an initramfs can carry the logic to figure out
>>>>> which to mount and is what I use for my self. The busybox based
>>>>> implementation I use adds a tad over 300Kb to the uImage, perfectly
>>>>> acceptable in my case.  
>>>>
>>>> When your minimal initramfs consumes 300KiB you're doing something
>>>> wrong. As I said in another thread, for your special purpose you'd
>>>> need to create a minitmal userspace for initramfs, no fancy (eg)libc,
>>>> just a bare minimum /init program which does the mount probing.
>>>> Shouldn’t be more than a few system calls.
> 
> That would indeed be nice, however, I fail to see how this can work
> with little effort *before* having devtmpfs ready. Also, we'll need a
> non-standard kernel parameter (usually "real_root=") to pass the
> selected rootfs down to our to-be-implemented micro-sized initramfs.

devtmpfs is available in initramfs.

>>>>
>>>> Thanks,
>>>> //richard
>>>
>>> Well, I use busybox because I'm lazy and still get away with only 300Kb.
>>> And as I said there is plenty space on my device. (6M per uImage OEM
>>> firmware configuration)
>>
>> So, problem solved. Use an initramfs. :-)
> 
> I agree this might be acceptable for some, but certainly not all
> cases. Unlike to your previous statement, I'm not generally opposed
> to having an initramfs included in the kernel as that can also provide
> other nice features such as recovery/failsafe methods.
> We've had this discussion before and my goal is, as I explained, to
> make the kernel as reusable as possible and even allow people to use
> OpenWrt/LEDE's kernel binary with other distributions. I'm sure
> this is also possible when using an initramfs, however, I still fail to
> see the necessity for that on simple devices.
> Imho, using an initramfs shouldn't be mandatory which is why there are
> patches (now down to about 80 LoC, resulting in probably less than 1kB
> in added binary size) to mount the rootfs without the need of an
> initramfs, simply because I do not consider that to be a
> "complex setup" if there is no less complex and yet generic way
> imaginable to boot on that platform at all.
> Hence I'm not buying the argument that ubiX_Y and ubiblockX_Y are two
> different devices and thus, this is a device selection problem and thus
> initramfs is the recommended way -- in fact, all other filesystems
> which do *not* build upon a block device provide some probing hacks in
> early userspace. Take MTD as an example: mtdblock devices are
> automagically provided and needed for block-based filesystems, no need
> for initramfs or kernel parameters to achieve that. For UBI, ubiblock
> (or gluebi...) is required to use UBI for anything else than UBIFS.
> Anyway, I'm afraid you have made your decission and additional
> arguments have no influence what-so-ever.
> 
>>
>> </discussion>,
> 
> If that's the whole answer ("Use initramfs. End of story."), that's
> pretty disappointing. Dispite your previous invitation to discuss the
> matter and collaborate to address the needs of all parties involved,
> you are now offering only one option which is not agreeable by all
> parties (which is the obvious reason for those nasty patches to exist
> in first place). Carrying a few patches in our local overlay doesn't
> truly hurt in a technical sense, however, it'd be nicer to discuss how
> those features could be brought upstream or mitigate our local patches
> in other ways.
> In the case of not finding room for a debate and your answer is a
> straight "we don't want this feature upstream" this is never the less
> a good reference to remember when touching those patches in future:
> falls into "we tried, they didn't want it" and thus we'll keep carrying
> them around, at least as long as there are platforms needing them.

Well, it is more a "use initramfs or cmdline via bootloader or cmdline via DT"
but you refuse to use *all* of these.
I asked to discuss and explain your use case and patches on linux-mtd. You did.
We explained you how to solve your issues without the need of any kernel hack.
You refused.

You bring over and over the same arguments and we showed you every single time
that your kernel hacks are not needed and everything can be solved perfectly fine
using existing features. This is disappointing.
Discussing a patch to death does not get it merged.

So it is not a thing of not finding room for a debate. We debated all aspects
multiple times and provided solutions.

Thanks,
//richard

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28 11:44           ` Daniel Golle
  2016-08-28 11:57             ` Richard Weinberger
@ 2016-08-28 12:10             ` Ralph Sennhauser
  2016-08-28 13:24             ` Boris Brezillon
  2 siblings, 0 replies; 27+ messages in thread
From: Ralph Sennhauser @ 2016-08-28 12:10 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Richard Weinberger, linux-mtd, Zoltan HERPAI, Hauke Mehrtens,
	lede-dev, openwrt-devel, Boris Brezillon, Brian Norris,
	Ezequiel Garcia

On Sun, 28 Aug 2016 13:44:48 +0200
Daniel Golle <daniel@makrotopia.org> wrote:

> Hi Richard,
> 
> On Sun, Aug 28, 2016 at 11:28:18AM +0200, Richard Weinberger wrote:
> > Ralph,
> > 
> > On 28.08.2016 11:19, Ralph Sennhauser wrote:  
> > >>> On the other hand an initramfs can carry the logic to figure out
> > >>> which to mount and is what I use for my self. The busybox based
> > >>> implementation I use adds a tad over 300Kb to the uImage,
> > >>> perfectly acceptable in my case.    
> > >>
> > >> When your minimal initramfs consumes 300KiB you're doing
> > >> something wrong. As I said in another thread, for your special
> > >> purpose you'd need to create a minitmal userspace for initramfs,
> > >> no fancy (eg)libc, just a bare minimum /init program which does
> > >> the mount probing. Shouldn’t be more than a few system calls.  
> 
> That would indeed be nice, however, I fail to see how this can work
> with little effort *before* having devtmpfs ready. Also, we'll need a
> non-standard kernel parameter (usually "real_root=") to pass the
> selected rootfs down to our to-be-implemented micro-sized initramfs.

There is no such requirement, an initramfs lets you do with root= what
you want.

> 
> > >>
> > >> Thanks,
> > >> //richard  
> > > 
> > > Well, I use busybox because I'm lazy and still get away with only
> > > 300Kb. And as I said there is plenty space on my device. (6M per
> > > uImage OEM firmware configuration)  
> > 
> > So, problem solved. Use an initramfs. :-)  
> 
> I agree this might be acceptable for some, but certainly not all
> cases.

It's not guaranteed to be solved, but it looks a lot like it is. I
suggest you pick a target that can be solved with dtb and one that
needs an initramfs and experiment with it. The comments I came across
in the openwrt sources wrt initramfs are often misguided.

Once you have an example that can't be reasonably addressed with
either dtb or initramfs would be the time to resume here.

Cheers
Ralph

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28 11:44           ` Daniel Golle
  2016-08-28 11:57             ` Richard Weinberger
  2016-08-28 12:10             ` Ralph Sennhauser
@ 2016-08-28 13:24             ` Boris Brezillon
  2 siblings, 0 replies; 27+ messages in thread
From: Boris Brezillon @ 2016-08-28 13:24 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Richard Weinberger, Ralph Sennhauser, linux-mtd, Zoltan HERPAI,
	Hauke Mehrtens, lede-dev, openwrt-devel, Brian Norris,
	Ezequiel Garcia

On Sun, 28 Aug 2016 13:44:48 +0200
Daniel Golle <daniel@makrotopia.org> wrote:

> Hi Richard,
> 
> On Sun, Aug 28, 2016 at 11:28:18AM +0200, Richard Weinberger wrote:
> > Ralph,
> > 
> > On 28.08.2016 11:19, Ralph Sennhauser wrote:  
> > >>> On the other hand an initramfs can carry the logic to figure out
> > >>> which to mount and is what I use for my self. The busybox based
> > >>> implementation I use adds a tad over 300Kb to the uImage, perfectly
> > >>> acceptable in my case.    
> > >>
> > >> When your minimal initramfs consumes 300KiB you're doing something
> > >> wrong. As I said in another thread, for your special purpose you'd
> > >> need to create a minitmal userspace for initramfs, no fancy (eg)libc,
> > >> just a bare minimum /init program which does the mount probing.
> > >> Shouldn’t be more than a few system calls.  
> 
> That would indeed be nice, however, I fail to see how this can work
> with little effort *before* having devtmpfs ready. Also, we'll need a
> non-standard kernel parameter (usually "real_root=") to pass the
> selected rootfs down to our to-be-implemented micro-sized initramfs.

Sorry, but I don't get this argument? AFAICT, devtmpfs can definitely be
enabled in an initramfs, all you need is a /dev directory.
And for the extra kernel parameters, I fail to see why it's required at
all: your initramfs should contain scripts/binaries that know which
config should be tested (for example, first try to create a ubiblock
dev on ubi0:rootfs, if it fails, fallback to ubifs).

So really, I think you're making a big deal of something that is not so
complicated to setup.

> 
> > >>
> > >> Thanks,
> > >> //richard  
> > > 
> > > Well, I use busybox because I'm lazy and still get away with only 300Kb.
> > > And as I said there is plenty space on my device. (6M per uImage OEM
> > > firmware configuration)  
> > 
> > So, problem solved. Use an initramfs. :-)  
> 
> I agree this might be acceptable for some, but certainly not all
> cases. Unlike to your previous statement, I'm not generally opposed
> to having an initramfs included in the kernel as that can also provide
> other nice features such as recovery/failsafe methods.
> We've had this discussion before and my goal is, as I explained, to
> make the kernel as reusable as possible and even allow people to use
> OpenWrt/LEDE's kernel binary with other distributions. I'm sure
> this is also possible when using an initramfs, however, I still fail to
> see the necessity for that on simple devices.
> Imho, using an initramfs shouldn't be mandatory which is why there are
> patches (now down to about 80 LoC, resulting in probably less than 1kB
> in added binary size) to mount the rootfs without the need of an
> initramfs, simply because I do not consider that to be a
> "complex setup" if there is no less complex and yet generic way
> imaginable to boot on that platform at all.
> Hence I'm not buying the argument that ubiX_Y and ubiblockX_Y are two
> different devices and thus, this is a device selection problem and thus
> initramfs is the recommended way -- in fact, all other filesystems
> which do *not* build upon a block device provide some probing hacks in
> early userspace. Take MTD as an example: mtdblock devices are
> automagically provided and needed for block-based filesystems, no need
> for initramfs or kernel parameters to achieve that. For UBI, ubiblock
> (or gluebi...) is required to use UBI for anything else than UBIFS.
> Anyway, I'm afraid you have made your decission and additional
> arguments have no influence what-so-ever.

Except you're making a big mistake here: UBI is not a FS, it's an
intermediate layer which only purpose is to take care of wear-leveling
(and other nasty stuff that have to be handled on unreliable devices
such as NANDs). UBIFS is the de-facto FS used on top of UBIFS right
now, but it will not necessarily be the case in the future. Other FS
that are usually used on top of UBI are all the RO block filesystems
(like squashfs), and this can only work thanks to the ubiblock (and
before it exists, gluebi) layer, which is emulating a block device on
top of a UBI volume.

ITOH, JFFS2 is a FS, and as such can directly register itself to the VFS
layer, and be called by do_mount().

Now, the ubi volume vs ubiblock device difference is real, and even if
you're not buying the argument, there's nothing much we can do, because
UBI volumes are not (and will never) directly expose a bdev interface.

So now, let's suppose we provide a way to automagically bind all RO UBI
volumes to ubiblock, so that ubiblock devices are automatically
populated. It wouldn't solve your problem: 'how to ask the kernel to try
booting from ubiX_Y and fallback to ubiblockX_Y?'. Because those devices
are really not the same (the latter can be manipulated using standard
blockdev I/O requests, while the former needs to be accessed through
the in kernel UBI API (see include/linux/mtd/ubi.h)).

The last option would be to provide a hack in UBIFS to be able to turn
a block device rootfs name (ubiblockX_Y) into a ubi one (ubiX_Y).
Definitely doable, but why is this hack more legitimate than the
previous ones.

You have to accept this fact, a UBI volume is one thing, a UBIBLOCK
device is a different thing, and Linux does not allow you to pass
several devices to your root= argument (see this attempt to add rootfs
recovery mechanism [1]). So, either you know in advanced where your
rootfs is hosted, or you have to used advanced mechanisms to do the
'trial-and-error' probing.

> 
> > 
> > </discussion>,  
> 
> If that's the whole answer ("Use initramfs. End of story."), that's
> pretty disappointing. Dispite your previous invitation to discuss the
> matter and collaborate to address the needs of all parties involved,
> you are now offering only one option which is not agreeable by all
> parties (which is the obvious reason for those nasty patches to exist
> in first place). Carrying a few patches in our local overlay doesn't
> truly hurt in a technical sense, however, it'd be nicer to discuss how
> those features could be brought upstream or mitigate our local patches
> in other ways.
> In the case of not finding room for a debate and your answer is a
> straight "we don't want this feature upstream" this is never the less
> a good reference to remember when touching those patches in future:
> falls into "we tried, they didn't want it" and thus we'll keep carrying
> them around, at least as long as there are platforms needing them.

That's clearly not what's happening here. Richard proposed several
solutions, but you decided that they were not appropriate without
giving enough convincing arguments to make us change our mind.

That's how things work: if you come with a real use case that can't be
solved using command-line tweaking or initramfs Richard may reconsider
adding a hack to the UBI/UBIFS layer to do what you need to do. Until
then, it's a status quo.

Regards,

Boris

[1]https://lkml.org/lkml/2016/8/16/77

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28 11:57             ` Richard Weinberger
@ 2016-08-28 13:47               ` Daniel Golle
  2016-08-28 14:17                 ` Boris Brezillon
  0 siblings, 1 reply; 27+ messages in thread
From: Daniel Golle @ 2016-08-28 13:47 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Boris Brezillon, Brian Norris, Hauke Mehrtens, lede-dev,
	linux-mtd, Ezequiel Garcia, Zoltan HERPAI, Ralph Sennhauser,
	openwrt-devel

Hi Richard,

On Sun, Aug 28, 2016 at 01:57:40PM +0200, Richard Weinberger wrote:
> Daniel,
> 
> On 28.08.2016 13:44, Daniel Golle wrote:
> > Hi Richard,
> > 
> > On Sun, Aug 28, 2016 at 11:28:18AM +0200, Richard Weinberger wrote:
> >> Ralph,
> >>
> >> On 28.08.2016 11:19, Ralph Sennhauser wrote:
> >>>>> On the other hand an initramfs can carry the logic to figure out
> >>>>> which to mount and is what I use for my self. The busybox based
> >>>>> implementation I use adds a tad over 300Kb to the uImage, perfectly
> >>>>> acceptable in my case.  
> >>>>
> >>>> When your minimal initramfs consumes 300KiB you're doing something
> >>>> wrong. As I said in another thread, for your special purpose you'd
> >>>> need to create a minitmal userspace for initramfs, no fancy (eg)libc,
> >>>> just a bare minimum /init program which does the mount probing.
> >>>> Shouldn’t be more than a few system calls.
> > 
> > That would indeed be nice, however, I fail to see how this can work
> > with little effort *before* having devtmpfs ready. Also, we'll need a
> > non-standard kernel parameter (usually "real_root=") to pass the
> > selected rootfs down to our to-be-implemented micro-sized initramfs.
> 
> devtmpfs is available in initramfs.

Yes, but you'll need to mount it (or tell the kernel to do so
automatically, which will then again cause problems on non-initramfs
systems where the init process is taking care of that).
Once you use device from that devtmpfs, you'll no longer be able to
umount it (because it's then busy), resulting in the whole initfamfs
to eat up RAM even after the real rootfs has been mounted.
As I said, I'm not generally against that, I just think it should not
be mandatory for the most simple possible case.

> 
> >>>>
> >>>> Thanks,
> >>>> //richard
> >>>
> >>> Well, I use busybox because I'm lazy and still get away with only 300Kb.
> >>> And as I said there is plenty space on my device. (6M per uImage OEM
> >>> firmware configuration)
> >>
> >> So, problem solved. Use an initramfs. :-)
> > 
> > I agree this might be acceptable for some, but certainly not all
> > cases. Unlike to your previous statement, I'm not generally opposed
> > to having an initramfs included in the kernel as that can also provide
> > other nice features such as recovery/failsafe methods.
> > We've had this discussion before and my goal is, as I explained, to
> > make the kernel as reusable as possible and even allow people to use
> > OpenWrt/LEDE's kernel binary with other distributions. I'm sure
> > this is also possible when using an initramfs, however, I still fail to
> > see the necessity for that on simple devices.
> > Imho, using an initramfs shouldn't be mandatory which is why there are
> > patches (now down to about 80 LoC, resulting in probably less than 1kB
> > in added binary size) to mount the rootfs without the need of an
> > initramfs, simply because I do not consider that to be a
> > "complex setup" if there is no less complex and yet generic way
> > imaginable to boot on that platform at all.
> > Hence I'm not buying the argument that ubiX_Y and ubiblockX_Y are two
> > different devices and thus, this is a device selection problem and thus
> > initramfs is the recommended way -- in fact, all other filesystems
> > which do *not* build upon a block device provide some probing hacks in
> > early userspace. Take MTD as an example: mtdblock devices are
> > automagically provided and needed for block-based filesystems, no need
> > for initramfs or kernel parameters to achieve that. For UBI, ubiblock
> > (or gluebi...) is required to use UBI for anything else than UBIFS.
> > Anyway, I'm afraid you have made your decission and additional
> > arguments have no influence what-so-ever.
> > 
> >>
> >> </discussion>,
> > 
> > If that's the whole answer ("Use initramfs. End of story."), that's
> > pretty disappointing. Dispite your previous invitation to discuss the
> > matter and collaborate to address the needs of all parties involved,
> > you are now offering only one option which is not agreeable by all
> > parties (which is the obvious reason for those nasty patches to exist
> > in first place). Carrying a few patches in our local overlay doesn't
> > truly hurt in a technical sense, however, it'd be nicer to discuss how
> > those features could be brought upstream or mitigate our local patches
> > in other ways.
> > In the case of not finding room for a debate and your answer is a
> > straight "we don't want this feature upstream" this is never the less
> > a good reference to remember when touching those patches in future:
> > falls into "we tried, they didn't want it" and thus we'll keep carrying
> > them around, at least as long as there are platforms needing them.
> 
> Well, it is more a "use initramfs or cmdline via bootloader or cmdline via DT"
> but you refuse to use *all* of these.

I'll happily use cmdline if it would be possible in a filesystem-type
agnostic way. The problem I see is that currently, when following that
suggesting, I'll end up with information about the filesystem-type in
either the bootloader or device-tree, which really shouldn't be. Hence
my suggestion I was intending to discuss here.

> I asked to discuss and explain your use case and patches on linux-mtd. You did.
> We explained you how to solve your issues without the need of any kernel hack.
> You refused.

I went through your suggestions and tried to follow them as closely as
possible given the use-case. What I ended up with is a completely new
and very different set of patches which is required to actually be able
to do so.

> 
> You bring over and over the same arguments and we showed you every single time
> that your kernel hacks are not needed and everything can be solved perfectly fine
> using existing features. This is disappointing.

All suggestions include the problem named above (needing to be aware
of the rootfs-type in a place where I shouldn't). If I don't feel that
this is being understood, repeating and explainign it more deeply is a
quite natural reaction.

> Discussing a patch to death does not get it merged.

I'm also happy to come up with alternative implementations or accept
any suggestions which would actually resolve the issue.
Requirements:
 - the bootloader may be proprietary, so we cannot require users to touch it
   (I feel that this was understood, no need to discuss again)
 - run the exact same kernel + device-tree with different filesystems,
   eg. UBIFS and SQUASHFS. This is intended in the kernel, ie. one can
   specify a list of filesystems like rootfstype=squashfs,jffs2
   or rootfstype=squashfs,ext4 (which is what we do on NOR-flash or
   block-based devices)

> 
> So it is not a thing of not finding room for a debate. We debated all aspects
> multiple times and provided solutions.

Have you actually looked into the patches rather than just replied
to the cover-letter?

Let's make a check-list:
 OK   attach UBI during boot
      -> use ubi.mtd=ubi in cmdline instead
 OK   probe-mount UBIFS
      -> patches merged upstream (MS_SILENT)
HACK  move volume name parser from UBIFS to ubi_open_volume_str()
      -> refused, no debate, no good alternative
HACK  introduce ubiblock_create_dev() passing back dev_t of the created
      ubiblock device
      -> patch refused, no debate, no good alternative
HACK  set ROOT_DEV for created ubiblock device
      -> not discussed here, but that's ok :)

If there was an easy solution which would not require either the
bootloader or device-tree to be aware of the filesystem type (which is
stored on the same volume on the same board and may vary depending
on use-case and during the boards life-cycle) I wouldn't bother you
with that at all. Including information on the filesystem-type inside
DTS is explicitely prohibited (I followed the debate) and doing it
inside the bootloader seems even more wrong to me.

To make it clear: I'd really like to use cmdline (which may or may
not be built into the DTS), I'm trying to follow that suggestion. The
only remaining problem I see it with that is that it'd need to be
possible to mount either squashfs or UBIFS using the same set of
parameters -- which is possible on all other devices we support which
either use NOR-flash or block storage for their root filesystems.

Now, ubi is already being treated as a special case in init/do_mounts.c
I fail to see why it would be bad to extend that so mounting non-UBIFS
and UBIFS volumes could work using the same cmdline root=... syntax.

I reckon the remaining difference is truely that you define that
set of requirements to be "complex" while I'd define it as "as simple
as it can possibly get on that hardware", hence our different
conclusion with regard to whether or not to use an initramfs for that
purpose.


Cheers


Daniel

> 
> Thanks,
> //richard
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-27 19:43 [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter Daniel Golle
  2016-08-27 20:43 ` Richard Weinberger
@ 2016-08-28 14:12 ` Ezequiel Garcia
  2016-08-28 14:20   ` Boris Brezillon
  2016-08-28 14:27   ` Daniel Golle
  1 sibling, 2 replies; 27+ messages in thread
From: Ezequiel Garcia @ 2016-08-28 14:12 UTC (permalink / raw)
  To: Daniel Golle
  Cc: linux-mtd@lists.infradead.org, Richard Weinberger, lede-dev,
	Zoltan HERPAI, Hauke Mehrtens, Ralph Sennhauser, openwrt-devel,
	Boris Brezillon, Brian Norris

Daniel,

Let's try to tackle this from a different angle.

On 27 August 2016 at 16:43, Daniel Golle <daniel@makrotopia.org> wrote:
> Hi!
>
> In an attempts to fix the flaws of the current set of UBI-related
> patches we are carrying in OpenWrt, I re-wrote the way mounting the
> rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
> which cannot be easily addressed using other means which are already
> available in the kernel is the fact that UBIFS and squashfs-on-UBI
> require different parameters to be set on the cmdline, e.g.
> for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1 rootfstype=squashfs
>

Can you help me understand the problem you are solving here?

So you currently need to do:

* for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
* for squashfs: ubi.mtd=ubi ubi.block=0,1 root=/dev/ubiblock0_1
rootfstype=squashfs

[..]
>
> With those changes, a single set of cmdline parameters is
> sufficient to mount either UBIFS or any other block filesystem
> by creating a ubiblock device:
> ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs
>

And you would like to do:

* for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
* for squashfs: ubi.mtd=ubi root=/dev/ubiblock0_1 rootfstype=squashfs

So the only thing you are trying to acomplish, is removal
of the ubi.block parameter needed to create ubi block devices.

Is that correct?

Thanks,
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28 13:47               ` Daniel Golle
@ 2016-08-28 14:17                 ` Boris Brezillon
  0 siblings, 0 replies; 27+ messages in thread
From: Boris Brezillon @ 2016-08-28 14:17 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Richard Weinberger, openwrt-devel, Hauke Mehrtens, lede-dev,
	linux-mtd, Ezequiel Garcia, Zoltan HERPAI, Ralph Sennhauser,
	Brian Norris

On Sun, 28 Aug 2016 15:47:56 +0200
Daniel Golle <daniel@makrotopia.org> wrote:

> Hi Richard,
> 
> On Sun, Aug 28, 2016 at 01:57:40PM +0200, Richard Weinberger wrote:
> > Daniel,
> > 
> > On 28.08.2016 13:44, Daniel Golle wrote:  
> > > Hi Richard,
> > > 
> > > On Sun, Aug 28, 2016 at 11:28:18AM +0200, Richard Weinberger wrote:  
> > >> Ralph,
> > >>
> > >> On 28.08.2016 11:19, Ralph Sennhauser wrote:  
> > >>>>> On the other hand an initramfs can carry the logic to figure out
> > >>>>> which to mount and is what I use for my self. The busybox based
> > >>>>> implementation I use adds a tad over 300Kb to the uImage, perfectly
> > >>>>> acceptable in my case.    
> > >>>>
> > >>>> When your minimal initramfs consumes 300KiB you're doing something
> > >>>> wrong. As I said in another thread, for your special purpose you'd
> > >>>> need to create a minitmal userspace for initramfs, no fancy (eg)libc,
> > >>>> just a bare minimum /init program which does the mount probing.
> > >>>> Shouldn’t be more than a few system calls.  
> > > 
> > > That would indeed be nice, however, I fail to see how this can work
> > > with little effort *before* having devtmpfs ready. Also, we'll need a
> > > non-standard kernel parameter (usually "real_root=") to pass the
> > > selected rootfs down to our to-be-implemented micro-sized initramfs.  
> > 
> > devtmpfs is available in initramfs.  
> 
> Yes, but you'll need to mount it (or tell the kernel to do so
> automatically, which will then again cause problems on non-initramfs
> systems where the init process is taking care of that).
> Once you use device from that devtmpfs, you'll no longer be able to
> umount it (because it's then busy), resulting in the whole initfamfs
> to eat up RAM even after the real rootfs has been mounted.
> As I said, I'm not generally against that, I just think it should not
> be mandatory for the most simple possible case.
> 
> >   
> > >>>>
> > >>>> Thanks,
> > >>>> //richard  
> > >>>
> > >>> Well, I use busybox because I'm lazy and still get away with only 300Kb.
> > >>> And as I said there is plenty space on my device. (6M per uImage OEM
> > >>> firmware configuration)  
> > >>
> > >> So, problem solved. Use an initramfs. :-)  
> > > 
> > > I agree this might be acceptable for some, but certainly not all
> > > cases. Unlike to your previous statement, I'm not generally opposed
> > > to having an initramfs included in the kernel as that can also provide
> > > other nice features such as recovery/failsafe methods.
> > > We've had this discussion before and my goal is, as I explained, to
> > > make the kernel as reusable as possible and even allow people to use
> > > OpenWrt/LEDE's kernel binary with other distributions. I'm sure
> > > this is also possible when using an initramfs, however, I still fail to
> > > see the necessity for that on simple devices.
> > > Imho, using an initramfs shouldn't be mandatory which is why there are
> > > patches (now down to about 80 LoC, resulting in probably less than 1kB
> > > in added binary size) to mount the rootfs without the need of an
> > > initramfs, simply because I do not consider that to be a
> > > "complex setup" if there is no less complex and yet generic way
> > > imaginable to boot on that platform at all.
> > > Hence I'm not buying the argument that ubiX_Y and ubiblockX_Y are two
> > > different devices and thus, this is a device selection problem and thus
> > > initramfs is the recommended way -- in fact, all other filesystems
> > > which do *not* build upon a block device provide some probing hacks in
> > > early userspace. Take MTD as an example: mtdblock devices are
> > > automagically provided and needed for block-based filesystems, no need
> > > for initramfs or kernel parameters to achieve that. For UBI, ubiblock
> > > (or gluebi...) is required to use UBI for anything else than UBIFS.
> > > Anyway, I'm afraid you have made your decission and additional
> > > arguments have no influence what-so-ever.
> > >   
> > >>
> > >> </discussion>,  
> > > 
> > > If that's the whole answer ("Use initramfs. End of story."), that's
> > > pretty disappointing. Dispite your previous invitation to discuss the
> > > matter and collaborate to address the needs of all parties involved,
> > > you are now offering only one option which is not agreeable by all
> > > parties (which is the obvious reason for those nasty patches to exist
> > > in first place). Carrying a few patches in our local overlay doesn't
> > > truly hurt in a technical sense, however, it'd be nicer to discuss how
> > > those features could be brought upstream or mitigate our local patches
> > > in other ways.
> > > In the case of not finding room for a debate and your answer is a
> > > straight "we don't want this feature upstream" this is never the less
> > > a good reference to remember when touching those patches in future:
> > > falls into "we tried, they didn't want it" and thus we'll keep carrying
> > > them around, at least as long as there are platforms needing them.  
> > 
> > Well, it is more a "use initramfs or cmdline via bootloader or cmdline via DT"
> > but you refuse to use *all* of these.  
> 
> I'll happily use cmdline if it would be possible in a filesystem-type
> agnostic way. The problem I see is that currently, when following that
> suggesting, I'll end up with information about the filesystem-type in
> either the bootloader or device-tree, which really shouldn't be. Hence
> my suggestion I was intending to discuss here.
> 
> > I asked to discuss and explain your use case and patches on linux-mtd. You did.
> > We explained you how to solve your issues without the need of any kernel hack.
> > You refused.  
> 
> I went through your suggestions and tried to follow them as closely as
> possible given the use-case. What I ended up with is a completely new
> and very different set of patches which is required to actually be able
> to do so.
> 
> > 
> > You bring over and over the same arguments and we showed you every single time
> > that your kernel hacks are not needed and everything can be solved perfectly fine
> > using existing features. This is disappointing.  
> 
> All suggestions include the problem named above (needing to be aware
> of the rootfs-type in a place where I shouldn't). If I don't feel that
> this is being understood, repeating and explainign it more deeply is a
> quite natural reaction.
> 
> > Discussing a patch to death does not get it merged.  
> 
> I'm also happy to come up with alternative implementations or accept
> any suggestions which would actually resolve the issue.
> Requirements:
>  - the bootloader may be proprietary, so we cannot require users to touch it
>    (I feel that this was understood, no need to discuss again)
>  - run the exact same kernel + device-tree with different filesystems,
>    eg. UBIFS and SQUASHFS. This is intended in the kernel, ie. one can
>    specify a list of filesystems like rootfstype=squashfs,jffs2
>    or rootfstype=squashfs,ext4 (which is what we do on NOR-flash or
>    block-based devices)
> 
> > 
> > So it is not a thing of not finding room for a debate. We debated all aspects
> > multiple times and provided solutions.  
> 
> Have you actually looked into the patches rather than just replied
> to the cover-letter?
> 
> Let's make a check-list:
>  OK   attach UBI during boot
>       -> use ubi.mtd=ubi in cmdline instead  
>  OK   probe-mount UBIFS
>       -> patches merged upstream (MS_SILENT)  
> HACK  move volume name parser from UBIFS to ubi_open_volume_str()
>       -> refused, no debate, no good alternative  
> HACK  introduce ubiblock_create_dev() passing back dev_t of the created
>       ubiblock device
>       -> patch refused, no debate, no good alternative  
> HACK  set ROOT_DEV for created ubiblock device
>       -> not discussed here, but that's ok :)  

I'm just reacting at your implementation proposal, so don't take this
as a 'yes, Boris seems to accept the concept of trial-and-error FS probe
on top of UBI vol in the kernel'.

But wouldn't something like that [1] be both simpler and self-contained?
Don't know if that works, but I think it's better to have a single
place to change (here UBIFS), than touching core kernel code along with
UBI and UBIFS.

Of course, this does not solve the auto ubi-to-ubiblock attach, but
that can be done with another parameter, like
ubi.block=attach_all_ro_vols.


[1]http://code.bulix.org/fkxrgt-105392

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28 14:12 ` Ezequiel Garcia
@ 2016-08-28 14:20   ` Boris Brezillon
  2016-08-28 14:25     ` Ezequiel Garcia
  2016-08-28 14:32     ` Daniel Golle
  2016-08-28 14:27   ` Daniel Golle
  1 sibling, 2 replies; 27+ messages in thread
From: Boris Brezillon @ 2016-08-28 14:20 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Daniel Golle, Brian Norris, Richard Weinberger, lede-dev,
	linux-mtd@lists.infradead.org, Zoltan HERPAI, Hauke Mehrtens,
	Ralph Sennhauser, openwrt-devel

On Sun, 28 Aug 2016 11:12:50 -0300
Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:

> Daniel,
> 
> Let's try to tackle this from a different angle.
> 
> On 27 August 2016 at 16:43, Daniel Golle <daniel@makrotopia.org> wrote:
> > Hi!
> >
> > In an attempts to fix the flaws of the current set of UBI-related
> > patches we are carrying in OpenWrt, I re-wrote the way mounting the
> > rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
> > which cannot be easily addressed using other means which are already
> > available in the kernel is the fact that UBIFS and squashfs-on-UBI
> > require different parameters to be set on the cmdline, e.g.
> > for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> > for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1 rootfstype=squashfs
> >  
> 
> Can you help me understand the problem you are solving here?
> 
> So you currently need to do:
> 
> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> * for squashfs: ubi.mtd=ubi ubi.block=0,1 root=/dev/ubiblock0_1
> rootfstype=squashfs
> 
> [..]
> >
> > With those changes, a single set of cmdline parameters is
> > sufficient to mount either UBIFS or any other block filesystem
> > by creating a ubiblock device:
> > ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs
> >  
> 
> And you would like to do:
> 
> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> * for squashfs: ubi.mtd=ubi root=/dev/ubiblock0_1 rootfstype=squashfs

I think Daniel wants something like:

ubi.mtd=1 root=ubi0:rootfs

or

ubi.mtd=1 root=/dev/ubiblock0_1

to work for both the UBIFS and squashfs cases.

> 
> So the only thing you are trying to acomplish, is removal
> of the ubi.block parameter needed to create ubi block devices.
> 
> Is that correct?
> 
> Thanks,

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28 14:20   ` Boris Brezillon
@ 2016-08-28 14:25     ` Ezequiel Garcia
  2016-08-28 14:40       ` Daniel Golle
  2016-08-28 14:32     ` Daniel Golle
  1 sibling, 1 reply; 27+ messages in thread
From: Ezequiel Garcia @ 2016-08-28 14:25 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Daniel Golle, Brian Norris, Richard Weinberger, lede-dev,
	linux-mtd@lists.infradead.org, Zoltan HERPAI, Hauke Mehrtens,
	Ralph Sennhauser, openwrt-devel

On 28 August 2016 at 11:20, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Sun, 28 Aug 2016 11:12:50 -0300
> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
>
>> Daniel,
>>
>> Let's try to tackle this from a different angle.
>>
>> On 27 August 2016 at 16:43, Daniel Golle <daniel@makrotopia.org> wrote:
>> > Hi!
>> >
>> > In an attempts to fix the flaws of the current set of UBI-related
>> > patches we are carrying in OpenWrt, I re-wrote the way mounting the
>> > rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
>> > which cannot be easily addressed using other means which are already
>> > available in the kernel is the fact that UBIFS and squashfs-on-UBI
>> > require different parameters to be set on the cmdline, e.g.
>> > for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
>> > for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1 rootfstype=squashfs
>> >
>>
>> Can you help me understand the problem you are solving here?
>>
>> So you currently need to do:
>>
>> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
>> * for squashfs: ubi.mtd=ubi ubi.block=0,1 root=/dev/ubiblock0_1
>> rootfstype=squashfs
>>
>> [..]
>> >
>> > With those changes, a single set of cmdline parameters is
>> > sufficient to mount either UBIFS or any other block filesystem
>> > by creating a ubiblock device:
>> > ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs
>> >
>>
>> And you would like to do:
>>
>> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
>> * for squashfs: ubi.mtd=ubi root=/dev/ubiblock0_1 rootfstype=squashfs
>
> I think Daniel wants something like:
>
> ubi.mtd=1 root=ubi0:rootfs
>
> or
>
> ubi.mtd=1 root=/dev/ubiblock0_1
>
> to work for both the UBIFS and squashfs cases.
>

Right. In which case, I was going to propose the same thing
you just did! It would be simple, and uninvasive to introduce
something like the parameter you suggested
"ubi.block=attach_all_ro_vols".

After all, this is what mtdblock does, so it wouldn't be
a too revolutionary approach.
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28 14:12 ` Ezequiel Garcia
  2016-08-28 14:20   ` Boris Brezillon
@ 2016-08-28 14:27   ` Daniel Golle
  2016-08-28 14:54     ` Ezequiel Garcia
  1 sibling, 1 reply; 27+ messages in thread
From: Daniel Golle @ 2016-08-28 14:27 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Boris Brezillon, Brian Norris, Richard Weinberger, lede-dev,
	linux-mtd@lists.infradead.org, Zoltan HERPAI, Hauke Mehrtens,
	Ralph Sennhauser, openwrt-devel

Hi Ezequiel,

On Sun, Aug 28, 2016 at 11:12:50AM -0300, Ezequiel Garcia wrote:
> Daniel,
> 
> Let's try to tackle this from a different angle.
> 
> On 27 August 2016 at 16:43, Daniel Golle <daniel@makrotopia.org> wrote:
> > Hi!
> >
> > In an attempts to fix the flaws of the current set of UBI-related
> > patches we are carrying in OpenWrt, I re-wrote the way mounting the
> > rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
> > which cannot be easily addressed using other means which are already
> > available in the kernel is the fact that UBIFS and squashfs-on-UBI
> > require different parameters to be set on the cmdline, e.g.
> > for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> > for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1 rootfstype=squashfs
> >
> 
> Can you help me understand the problem you are solving here?
> 
> So you currently need to do:
> 
> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> * for squashfs: ubi.mtd=ubi ubi.block=0,1 root=/dev/ubiblock0_1
> rootfstype=squashfs
> 
> [..]
> >
> > With those changes, a single set of cmdline parameters is
> > sufficient to mount either UBIFS or any other block filesystem
> > by creating a ubiblock device:
> > ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     this is the goal state :)

> >
> 
> And you would like to do:
> 
> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> * for squashfs: ubi.mtd=ubi root=/dev/ubiblock0_1 rootfstype=squashfs

I do NOT want to pass different cmdlines depending on the filesystem
type. The lines quote above were to illustrate the current
inconsistency of cmdline parameters.

> 
> So the only thing you are trying to acomplish, is removal
> of the ubi.block parameter needed to create ubi block devices.
> 
> Is that correct?

Almost. I want a single syntax for rootfs= to refer to the rootfs
volume, no matter what the filesystem type is. Hence, in case of
non-UBIFS filesystem, a ubiblock needs to be created and mounted.


Cheers


Daniel


> 
> Thanks,
> -- 
> Ezequiel García, VanguardiaSur
> www.vanguardiasur.com.ar
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28 14:20   ` Boris Brezillon
  2016-08-28 14:25     ` Ezequiel Garcia
@ 2016-08-28 14:32     ` Daniel Golle
  1 sibling, 0 replies; 27+ messages in thread
From: Daniel Golle @ 2016-08-28 14:32 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Ezequiel Garcia, openwrt-devel, Richard Weinberger, lede-dev,
	linux-mtd@lists.infradead.org, Zoltan HERPAI, Hauke Mehrtens,
	Ralph Sennhauser, Brian Norris

On Sun, Aug 28, 2016 at 04:20:59PM +0200, Boris Brezillon wrote:
> I think Daniel wants something like:
> 
> ubi.mtd=1 root=ubi0:rootfs

Yes, this is what I want :)

> 
> or
> 
> ubi.mtd=1 root=/dev/ubiblock0_1

This would already be a bit worse, as then I need to refer to the
rootfs volume using the vol_id rather than its name...

> 
> to work for both the UBIFS and squashfs cases.
> 
> > 
> > So the only thing you are trying to acomplish, is removal
> > of the ubi.block parameter needed to create ubi block devices.
> > 
> > Is that correct?
> > 
> > Thanks,
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28 14:25     ` Ezequiel Garcia
@ 2016-08-28 14:40       ` Daniel Golle
  2016-08-28 15:00         ` Boris Brezillon
  0 siblings, 1 reply; 27+ messages in thread
From: Daniel Golle @ 2016-08-28 14:40 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Boris Brezillon, openwrt-devel, Richard Weinberger, lede-dev,
	linux-mtd@lists.infradead.org, Zoltan HERPAI, Hauke Mehrtens,
	Ralph Sennhauser, Brian Norris

On Sun, Aug 28, 2016 at 11:25:54AM -0300, Ezequiel Garcia wrote:
> On 28 August 2016 at 11:20, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Sun, 28 Aug 2016 11:12:50 -0300
> > Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
> >
> >> Daniel,
> >>
> >> Let's try to tackle this from a different angle.
> >>
> >> On 27 August 2016 at 16:43, Daniel Golle <daniel@makrotopia.org> wrote:
> >> > Hi!
> >> >
> >> > In an attempts to fix the flaws of the current set of UBI-related
> >> > patches we are carrying in OpenWrt, I re-wrote the way mounting the
> >> > rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
> >> > which cannot be easily addressed using other means which are already
> >> > available in the kernel is the fact that UBIFS and squashfs-on-UBI
> >> > require different parameters to be set on the cmdline, e.g.
> >> > for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> >> > for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1 rootfstype=squashfs
> >> >
> >>
> >> Can you help me understand the problem you are solving here?
> >>
> >> So you currently need to do:
> >>
> >> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> >> * for squashfs: ubi.mtd=ubi ubi.block=0,1 root=/dev/ubiblock0_1
> >> rootfstype=squashfs
> >>
> >> [..]
> >> >
> >> > With those changes, a single set of cmdline parameters is
> >> > sufficient to mount either UBIFS or any other block filesystem
> >> > by creating a ubiblock device:
> >> > ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs
> >> >
> >>
> >> And you would like to do:
> >>
> >> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> >> * for squashfs: ubi.mtd=ubi root=/dev/ubiblock0_1 rootfstype=squashfs
> >
> > I think Daniel wants something like:
> >
> > ubi.mtd=1 root=ubi0:rootfs
> >
> > or
> >
> > ubi.mtd=1 root=/dev/ubiblock0_1
> >
> > to work for both the UBIFS and squashfs cases.
> >
> 
> Right. In which case, I was going to propose the same thing
> you just did! It would be simple, and uninvasive to introduce
> something like the parameter you suggested
> "ubi.block=attach_all_ro_vols".

That in addition with the patch referred to in an earlier mail
http://code.bulix.org/fkxrgt-105392
would indeed solve the problem, with the limitation that one needs to
use the {ubi_num}_{vol_id} syntax instead of being able to refer to
the volume name.
Currently, we do use 'dynamic' (ie. read-write) volumes even for
squashfs, as otherwise boot takes much longer as the CRC for the
whole volumes needs to be calculated. Having *any* 'static' volumes
also breaks some older versions of U-Boot already supporting UBI.
I'd rather say "attach_all_non_ubifs_vols" and probe the filesystem
type, though that's also not very clean.
Also, one might not want to attach *all* volumes, ie. for a ubootenv
volume, there should not be a ubiblock.
Re-using the volume-name parser from UBIFS to also create the ubiblock
device needed to mount the rootfs seemed to be the most transparent
approach to me.

> 
> After all, this is what mtdblock does, so it wouldn't be
> a too revolutionary approach.

True.

> -- 
> Ezequiel García, VanguardiaSur
> www.vanguardiasur.com.ar
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28 14:27   ` Daniel Golle
@ 2016-08-28 14:54     ` Ezequiel Garcia
  0 siblings, 0 replies; 27+ messages in thread
From: Ezequiel Garcia @ 2016-08-28 14:54 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Boris Brezillon, Brian Norris, Richard Weinberger, lede-dev,
	linux-mtd@lists.infradead.org, Zoltan HERPAI, Hauke Mehrtens,
	Ralph Sennhauser, openwrt-devel

On 28 August 2016 at 11:27, Daniel Golle <daniel@makrotopia.org> wrote:
> Hi Ezequiel,
>
> On Sun, Aug 28, 2016 at 11:12:50AM -0300, Ezequiel Garcia wrote:
>> Daniel,
>>
>> Let's try to tackle this from a different angle.
>>
>> On 27 August 2016 at 16:43, Daniel Golle <daniel@makrotopia.org> wrote:
>> > Hi!
>> >
>> > In an attempts to fix the flaws of the current set of UBI-related
>> > patches we are carrying in OpenWrt, I re-wrote the way mounting the
>> > rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
>> > which cannot be easily addressed using other means which are already
>> > available in the kernel is the fact that UBIFS and squashfs-on-UBI
>> > require different parameters to be set on the cmdline, e.g.
>> > for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
>> > for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1 rootfstype=squashfs
>> >
>>
>> Can you help me understand the problem you are solving here?
>>
>> So you currently need to do:
>>
>> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
>> * for squashfs: ubi.mtd=ubi ubi.block=0,1 root=/dev/ubiblock0_1
>> rootfstype=squashfs
>>
>> [..]
>> >
>> > With those changes, a single set of cmdline parameters is
>> > sufficient to mount either UBIFS or any other block filesystem
>> > by creating a ubiblock device:
>> > ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs
>     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>      this is the goal state :)
>
>> >
>>
>> And you would like to do:
>>
>> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
>> * for squashfs: ubi.mtd=ubi root=/dev/ubiblock0_1 rootfstype=squashfs
>
> I do NOT want to pass different cmdlines depending on the filesystem
> type. The lines quote above were to illustrate the current
> inconsistency of cmdline parameters.
>
>>
>> So the only thing you are trying to acomplish, is removal
>> of the ubi.block parameter needed to create ubi block devices.
>>
>> Is that correct?
>
> Almost. I want a single syntax for rootfs= to refer to the rootfs
> volume, no matter what the filesystem type is. Hence, in case of
> non-UBIFS filesystem, a ubiblock needs to be created and mounted.
>

Hm, OK. So, if you _actually_ want to do something like:

ubi.mtd=ubi root=ubi0:rootfs rootfstype=squashfs

Then you are ** doomed **

I guess you already understand this, but just for the
sake of it, bare with me going through the obvious:

The squashfs works on top of a block device, and so it has
absolutely NO idea about the block device belonging
to a given volume.

The "root=ubi0:rootfs" is understood by the filesystem,
in this case ubifs, so it can't be understood by squashfs.

And you can add any amount of pretty hacks to squashfs
or to init/do_mounts.c, but as far as I can see, they will
always be hacks and they will always be rejected.

The reason for this is that kenel code would be very
hard to maintain if each $user that wanted some
special thing to work for a given use case would be
allowed to hack its own special rule.

The kernel is already hard to maintain as it is.
I don't even want to think what it would look like
if maintainers would accept proposals like this one.

Sorry,
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28 14:40       ` Daniel Golle
@ 2016-08-28 15:00         ` Boris Brezillon
  2016-08-28 15:24           ` Daniel Golle
  0 siblings, 1 reply; 27+ messages in thread
From: Boris Brezillon @ 2016-08-28 15:00 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Ezequiel Garcia, openwrt-devel, Richard Weinberger, lede-dev,
	linux-mtd@lists.infradead.org, Zoltan HERPAI, Hauke Mehrtens,
	Ralph Sennhauser, Brian Norris

On Sun, 28 Aug 2016 16:40:14 +0200
Daniel Golle <daniel@makrotopia.org> wrote:

> On Sun, Aug 28, 2016 at 11:25:54AM -0300, Ezequiel Garcia wrote:
> > On 28 August 2016 at 11:20, Boris Brezillon
> > <boris.brezillon@free-electrons.com> wrote:  
> > > On Sun, 28 Aug 2016 11:12:50 -0300
> > > Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
> > >  
> > >> Daniel,
> > >>
> > >> Let's try to tackle this from a different angle.
> > >>
> > >> On 27 August 2016 at 16:43, Daniel Golle <daniel@makrotopia.org> wrote:  
> > >> > Hi!
> > >> >
> > >> > In an attempts to fix the flaws of the current set of UBI-related
> > >> > patches we are carrying in OpenWrt, I re-wrote the way mounting the
> > >> > rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
> > >> > which cannot be easily addressed using other means which are already
> > >> > available in the kernel is the fact that UBIFS and squashfs-on-UBI
> > >> > require different parameters to be set on the cmdline, e.g.
> > >> > for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> > >> > for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1 rootfstype=squashfs
> > >> >  
> > >>
> > >> Can you help me understand the problem you are solving here?
> > >>
> > >> So you currently need to do:
> > >>
> > >> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> > >> * for squashfs: ubi.mtd=ubi ubi.block=0,1 root=/dev/ubiblock0_1
> > >> rootfstype=squashfs
> > >>
> > >> [..]  
> > >> >
> > >> > With those changes, a single set of cmdline parameters is
> > >> > sufficient to mount either UBIFS or any other block filesystem
> > >> > by creating a ubiblock device:
> > >> > ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs
> > >> >  
> > >>
> > >> And you would like to do:
> > >>
> > >> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> > >> * for squashfs: ubi.mtd=ubi root=/dev/ubiblock0_1 rootfstype=squashfs  
> > >
> > > I think Daniel wants something like:
> > >
> > > ubi.mtd=1 root=ubi0:rootfs
> > >
> > > or
> > >
> > > ubi.mtd=1 root=/dev/ubiblock0_1
> > >
> > > to work for both the UBIFS and squashfs cases.
> > >  
> > 
> > Right. In which case, I was going to propose the same thing
> > you just did! It would be simple, and uninvasive to introduce
> > something like the parameter you suggested
> > "ubi.block=attach_all_ro_vols".  
> 
> That in addition with the patch referred to in an earlier mail
> http://code.bulix.org/fkxrgt-105392
> would indeed solve the problem, with the limitation that one needs to
> use the {ubi_num}_{vol_id} syntax instead of being able to refer to
> the volume name.

Let me clarify my previous answer: I'm still strongly opposed to this
approach (including my less invasive patch). I was just trying to show
that your implementation was invasive and could be made less invasive,
but it remains a hack, which, unless proven otherwise, is not needed.

> Currently, we do use 'dynamic' (ie. read-write) volumes even for
> squashfs, as otherwise boot takes much longer as the CRC for the
> whole volumes needs to be calculated. Having *any* 'static' volumes
> also breaks some older versions of U-Boot already supporting UBI.
> I'd rather say "attach_all_non_ubifs_vols" and probe the filesystem
> type, though that's also not very clean.
> Also, one might not want to attach *all* volumes, ie. for a ubootenv
> volume, there should not be a ubiblock.
> Re-using the volume-name parser from UBIFS to also create the ubiblock
> device needed to mount the rootfs seemed to be the most transparent
> approach to me.

So each time you're asking more. AFAIK, JFFS2 was only supporting
the /dev/mtdblockX format, without any way to use MTD names. But now you
want an improved version that is able to use the UBI format, which is
not at all suitable for block-device probing.

To be very clear, on my side this is a NACK.

Regards,

Boris

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28 15:00         ` Boris Brezillon
@ 2016-08-28 15:24           ` Daniel Golle
  2016-08-28 16:35             ` Boris Brezillon
  0 siblings, 1 reply; 27+ messages in thread
From: Daniel Golle @ 2016-08-28 15:24 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Brian Norris, Richard Weinberger, lede-dev,
	linux-mtd@lists.infradead.org, Ezequiel Garcia, Zoltan HERPAI,
	Hauke Mehrtens, Ralph Sennhauser, openwrt-devel

Hi Boris,

On Sun, Aug 28, 2016 at 05:00:54PM +0200, Boris Brezillon wrote:
> On Sun, 28 Aug 2016 16:40:14 +0200
> Daniel Golle <daniel@makrotopia.org> wrote:
> 
> > On Sun, Aug 28, 2016 at 11:25:54AM -0300, Ezequiel Garcia wrote:
> > > On 28 August 2016 at 11:20, Boris Brezillon
> > > <boris.brezillon@free-electrons.com> wrote:  
> > > > On Sun, 28 Aug 2016 11:12:50 -0300
> > > > Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
> > > >  
> > > >> Daniel,
> > > >>
> > > >> Let's try to tackle this from a different angle.
> > > >>
> > > >> On 27 August 2016 at 16:43, Daniel Golle <daniel@makrotopia.org> wrote:  
> > > >> > Hi!
> > > >> >
> > > >> > In an attempts to fix the flaws of the current set of UBI-related
> > > >> > patches we are carrying in OpenWrt, I re-wrote the way mounting the
> > > >> > rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
> > > >> > which cannot be easily addressed using other means which are already
> > > >> > available in the kernel is the fact that UBIFS and squashfs-on-UBI
> > > >> > require different parameters to be set on the cmdline, e.g.
> > > >> > for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> > > >> > for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1 rootfstype=squashfs
> > > >> >  
> > > >>
> > > >> Can you help me understand the problem you are solving here?
> > > >>
> > > >> So you currently need to do:
> > > >>
> > > >> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> > > >> * for squashfs: ubi.mtd=ubi ubi.block=0,1 root=/dev/ubiblock0_1
> > > >> rootfstype=squashfs
> > > >>
> > > >> [..]  
> > > >> >
> > > >> > With those changes, a single set of cmdline parameters is
> > > >> > sufficient to mount either UBIFS or any other block filesystem
> > > >> > by creating a ubiblock device:
> > > >> > ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs
> > > >> >  
> > > >>
> > > >> And you would like to do:
> > > >>
> > > >> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> > > >> * for squashfs: ubi.mtd=ubi root=/dev/ubiblock0_1 rootfstype=squashfs  
> > > >
> > > > I think Daniel wants something like:
> > > >
> > > > ubi.mtd=1 root=ubi0:rootfs
> > > >
> > > > or
> > > >
> > > > ubi.mtd=1 root=/dev/ubiblock0_1
> > > >
> > > > to work for both the UBIFS and squashfs cases.
> > > >  
> > > 
> > > Right. In which case, I was going to propose the same thing
> > > you just did! It would be simple, and uninvasive to introduce
> > > something like the parameter you suggested
> > > "ubi.block=attach_all_ro_vols".  
> > 
> > That in addition with the patch referred to in an earlier mail
> > http://code.bulix.org/fkxrgt-105392
> > would indeed solve the problem, with the limitation that one needs to
> > use the {ubi_num}_{vol_id} syntax instead of being able to refer to
> > the volume name.
> 
> Let me clarify my previous answer: I'm still strongly opposed to this
> approach (including my less invasive patch). I was just trying to show
> that your implementation was invasive and could be made less invasive,
> but it remains a hack, which, unless proven otherwise, is not needed.

I agree and merely wanted to discuss the vague direction rather than
details of the implementation. Using your patch, if it was merged,
would indeed at least provide a partial alternative.

Anyway, What more of a proof for the need of such a hack would be needed?
Facts:
[1] the bootloader should not need to be aware of the filesystem-type
    used for Linux' rootfs
[2] the device-tree explicitely prohibits information on fstypes
[3] disitributions and users use different filesystem types on top of
    UBI inside the same volume on the very same board
[4] two entirely different sets of parameters are needed to mount UBIFS
    or e.g. SQUASHFS (or any other read-only block filesystem)
[5] in initramfs should be used for complex boot setups

>From fact [3] and [4] you can imply that either the cmdline needs to be
adapted based on the filesystem type [6] or a new probing mechanism
needs to be introduced, either in the kernel [7] or by using an
initramfs [8].
[1] and [2] contradict with [6].
[8] seems the way to go if you consider [5] to be true in that case
(ie. "this is a complex setup"), which is what this discussion boils
down to imho.


> 
> > Currently, we do use 'dynamic' (ie. read-write) volumes even for
> > squashfs, as otherwise boot takes much longer as the CRC for the
> > whole volumes needs to be calculated. Having *any* 'static' volumes
> > also breaks some older versions of U-Boot already supporting UBI.
> > I'd rather say "attach_all_non_ubifs_vols" and probe the filesystem
> > type, though that's also not very clean.
> > Also, one might not want to attach *all* volumes, ie. for a ubootenv
> > volume, there should not be a ubiblock.
> > Re-using the volume-name parser from UBIFS to also create the ubiblock
> > device needed to mount the rootfs seemed to be the most transparent
> > approach to me.
> 
> So each time you're asking more. AFAIK, JFFS2 was only supporting
> the /dev/mtdblockX format, without any way to use MTD names. But now you
> want an improved version that is able to use the UBI format, which is
> not at all suitable for block-device probing.

I've been asking for that ("move the volume name parser from UBIFS to
UBI for it to be reused elsewhere") from the beginning of this debate.
To get to your remark: Please look at the actual patches I submitted
for discussion at the beginning of this thread.
For MTD, refering to partitions by numbers is fine, because those
numbers are static and merely a result of the partitioning scheme given
in the device-tree or cmdline mtdparts.
For UBI, volumes are dynamic and users my delete and create volumes
during the life-time of a device as they like. Thus, it's not asking
for more, it's just asking for having the same degree of reliability
and transparency as when using other storage subsystems.

> 
> To be very clear, on my side this is a NACK.

So, for the record, I conclude that those patches will remain
OpenWrt/LEDE specific and you and others on linux-mtd are opposed to
the general *approach* (and not just the actual implementation).

> 
> Regards,
> 
> Boris
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter
  2016-08-28 15:24           ` Daniel Golle
@ 2016-08-28 16:35             ` Boris Brezillon
  0 siblings, 0 replies; 27+ messages in thread
From: Boris Brezillon @ 2016-08-28 16:35 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Brian Norris, Richard Weinberger, lede-dev,
	linux-mtd@lists.infradead.org, Ezequiel Garcia, Zoltan HERPAI,
	Hauke Mehrtens, Ralph Sennhauser, openwrt-devel

On Sun, 28 Aug 2016 17:24:51 +0200
Daniel Golle <daniel@makrotopia.org> wrote:

> Hi Boris,
> 
> On Sun, Aug 28, 2016 at 05:00:54PM +0200, Boris Brezillon wrote:
> > On Sun, 28 Aug 2016 16:40:14 +0200
> > Daniel Golle <daniel@makrotopia.org> wrote:
> >   
> > > On Sun, Aug 28, 2016 at 11:25:54AM -0300, Ezequiel Garcia wrote:  
> > > > On 28 August 2016 at 11:20, Boris Brezillon
> > > > <boris.brezillon@free-electrons.com> wrote:    
> > > > > On Sun, 28 Aug 2016 11:12:50 -0300
> > > > > Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
> > > > >    
> > > > >> Daniel,
> > > > >>
> > > > >> Let's try to tackle this from a different angle.
> > > > >>
> > > > >> On 27 August 2016 at 16:43, Daniel Golle <daniel@makrotopia.org> wrote:    
> > > > >> > Hi!
> > > > >> >
> > > > >> > In an attempts to fix the flaws of the current set of UBI-related
> > > > >> > patches we are carrying in OpenWrt, I re-wrote the way mounting the
> > > > >> > rootfs from UBI in OpenWrt/LEDE works. The main requirement I face
> > > > >> > which cannot be easily addressed using other means which are already
> > > > >> > available in the kernel is the fact that UBIFS and squashfs-on-UBI
> > > > >> > require different parameters to be set on the cmdline, e.g.
> > > > >> > for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> > > > >> > for squashfs: ubi.mtd=ubi ubiblock=0,1 root=/dev/ubiblock0_1 rootfstype=squashfs
> > > > >> >    
> > > > >>
> > > > >> Can you help me understand the problem you are solving here?
> > > > >>
> > > > >> So you currently need to do:
> > > > >>
> > > > >> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> > > > >> * for squashfs: ubi.mtd=ubi ubi.block=0,1 root=/dev/ubiblock0_1
> > > > >> rootfstype=squashfs
> > > > >>
> > > > >> [..]    
> > > > >> >
> > > > >> > With those changes, a single set of cmdline parameters is
> > > > >> > sufficient to mount either UBIFS or any other block filesystem
> > > > >> > by creating a ubiblock device:
> > > > >> > ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs,squashfs
> > > > >> >    
> > > > >>
> > > > >> And you would like to do:
> > > > >>
> > > > >> * for UBIFS: ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs
> > > > >> * for squashfs: ubi.mtd=ubi root=/dev/ubiblock0_1 rootfstype=squashfs    
> > > > >
> > > > > I think Daniel wants something like:
> > > > >
> > > > > ubi.mtd=1 root=ubi0:rootfs
> > > > >
> > > > > or
> > > > >
> > > > > ubi.mtd=1 root=/dev/ubiblock0_1
> > > > >
> > > > > to work for both the UBIFS and squashfs cases.
> > > > >    
> > > > 
> > > > Right. In which case, I was going to propose the same thing
> > > > you just did! It would be simple, and uninvasive to introduce
> > > > something like the parameter you suggested
> > > > "ubi.block=attach_all_ro_vols".    
> > > 
> > > That in addition with the patch referred to in an earlier mail
> > > http://code.bulix.org/fkxrgt-105392
> > > would indeed solve the problem, with the limitation that one needs to
> > > use the {ubi_num}_{vol_id} syntax instead of being able to refer to
> > > the volume name.  
> > 
> > Let me clarify my previous answer: I'm still strongly opposed to this
> > approach (including my less invasive patch). I was just trying to show
> > that your implementation was invasive and could be made less invasive,
> > but it remains a hack, which, unless proven otherwise, is not needed.  
> 
> I agree and merely wanted to discuss the vague direction rather than
> details of the implementation. Using your patch, if it was merged,
> would indeed at least provide a partial alternative.
> 
> Anyway, What more of a proof for the need of such a hack would be needed?
> Facts:
> [1] the bootloader should not need to be aware of the filesystem-type
>     used for Linux' rootfs
> [2] the device-tree explicitely prohibits information on fstypes
> [3] disitributions and users use different filesystem types on top of
>     UBI inside the same volume on the very same board
> [4] two entirely different sets of parameters are needed to mount UBIFS
>     or e.g. SQUASHFS (or any other read-only block filesystem)
> [5] in initramfs should be used for complex boot setups
> 
> From fact [3] and [4] you can imply that either the cmdline needs to be
> adapted based on the filesystem type [6] or a new probing mechanism
> needs to be introduced, either in the kernel [7] or by using an
> initramfs [8].
> [1] and [2] contradict with [6].
> [8] seems the way to go if you consider [5] to be true in that case
> (ie. "this is a complex setup"), which is what this discussion boils
> down to imho.

I think Richard and I agree that [8] is the right answer, because your
use case is way more complex than "mount a FS from this device" (as
explained earlier).

> 
> 
> >   
> > > Currently, we do use 'dynamic' (ie. read-write) volumes even for
> > > squashfs, as otherwise boot takes much longer as the CRC for the
> > > whole volumes needs to be calculated. Having *any* 'static' volumes
> > > also breaks some older versions of U-Boot already supporting UBI.
> > > I'd rather say "attach_all_non_ubifs_vols" and probe the filesystem
> > > type, though that's also not very clean.
> > > Also, one might not want to attach *all* volumes, ie. for a ubootenv
> > > volume, there should not be a ubiblock.
> > > Re-using the volume-name parser from UBIFS to also create the ubiblock
> > > device needed to mount the rootfs seemed to be the most transparent
> > > approach to me.  
> > 
> > So each time you're asking more. AFAIK, JFFS2 was only supporting
> > the /dev/mtdblockX format, without any way to use MTD names. But now you
> > want an improved version that is able to use the UBI format, which is
> > not at all suitable for block-device probing.  
> 
> I've been asking for that ("move the volume name parser from UBIFS to
> UBI for it to be reused elsewhere") from the beginning of this debate.
> To get to your remark: Please look at the actual patches I submitted
> for discussion at the beginning of this thread.

How do you think I could comment on your implementation? Of course I
reviewed your patches, just didn't answer to the patches directly,
because the general approach seemed wrong to me.

I'll comment on them individually if you really want.

> For MTD, refering to partitions by numbers is fine, because those
> numbers are static and merely a result of the partitioning scheme given
> in the device-tree or cmdline mtdparts.

That's only true if you have a single MTD device. But the probe order
is not guaranteed if you mix different devices, so relying on dev
numbers for MTD is just as broken as with UBI devices.

> For UBI, volumes are dynamic and users my delete and create volumes
> during the life-time of a device as they like. Thus, it's not asking
> for more, it's just asking for having the same degree of reliability
> and transparency as when using other storage subsystems.

See above.

> 
> > 
> > To be very clear, on my side this is a NACK.  
> 
> So, for the record, I conclude that those patches will remain
> OpenWrt/LEDE specific and you and others on linux-mtd are opposed to
> the general *approach* (and not just the actual implementation).

How about evaluating the initrd approach? You didn't give any real
arguments beside 'it's more complicated than a command-line parameter'.
But, according to your explanation, you want to do advanced selection of
the rootfs source, so it seems a good candidate for an initrd setup to
me.

Please test that, and come back to us if it's not working.

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

end of thread, other threads:[~2016-08-28 16:35 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-27 19:43 [PATCH/RFC 0/3] UBI: unify mouting rootfs based on cmdline parameter Daniel Golle
2016-08-27 20:43 ` Richard Weinberger
2016-08-27 21:06   ` Daniel Golle
2016-08-27 21:23     ` Richard Weinberger
2016-08-27 23:13       ` Daniel Golle
2016-08-27 23:33         ` Richard Weinberger
2016-08-28  6:47           ` Boris Brezillon
2016-08-28  7:10   ` Ralph Sennhauser
2016-08-28  8:12     ` Richard Weinberger
2016-08-28  9:19       ` Ralph Sennhauser
2016-08-28  9:28         ` Richard Weinberger
2016-08-28 11:44           ` Daniel Golle
2016-08-28 11:57             ` Richard Weinberger
2016-08-28 13:47               ` Daniel Golle
2016-08-28 14:17                 ` Boris Brezillon
2016-08-28 12:10             ` Ralph Sennhauser
2016-08-28 13:24             ` Boris Brezillon
2016-08-28 14:12 ` Ezequiel Garcia
2016-08-28 14:20   ` Boris Brezillon
2016-08-28 14:25     ` Ezequiel Garcia
2016-08-28 14:40       ` Daniel Golle
2016-08-28 15:00         ` Boris Brezillon
2016-08-28 15:24           ` Daniel Golle
2016-08-28 16:35             ` Boris Brezillon
2016-08-28 14:32     ` Daniel Golle
2016-08-28 14:27   ` Daniel Golle
2016-08-28 14:54     ` Ezequiel Garcia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox