| Message ID | 20160718222649.GA17394@makrotopia.org |
|---|---|
| State | Accepted |
| Delegated to: | Richard Weinberger |
| Headers | show |
Am 19.07.2016 um 00:26 schrieb Daniel Golle: > Probe-mounting a volume too small for UBIFS results in kernel log > polution which might irritate users. > Address this by silencing errors which may happen during boot if the > rootfs is e.g. squashfs (and thus rather small) stored on a UBI volume. > This commit should be seen in context with > commit 65fb47721925628bf85ec08c4302ac5e7224a66a > and > commit 90bea5a3f0bf680b87b90516f3c231997f4b8f3b The diff makes sense but not the commit message. Why would you mount a squashfs on top of UBI as UBIFS? We have rootfstype= for this. Thanks, //richard
Hi Richard, thanks for reviewing my patch! On Tue, Jul 19, 2016 at 07:38:28AM +0200, Richard Weinberger wrote: > Am 19.07.2016 um 00:26 schrieb Daniel Golle: > > Probe-mounting a volume too small for UBIFS results in kernel log > > polution which might irritate users. > > Address this by silencing errors which may happen during boot if the > > rootfs is e.g. squashfs (and thus rather small) stored on a UBI volume. > > This commit should be seen in context with > > commit 65fb47721925628bf85ec08c4302ac5e7224a66a > > and > > commit 90bea5a3f0bf680b87b90516f3c231997f4b8f3b > > The diff makes sense but not the commit message. More brief? Less reasoning? Please elaborate, so I know what to improve. > Why would you mount a squashfs on top of UBI as UBIFS? > We have rootfstype= for this. However, in OpenWrt/LEDE users may choose the fstype (UBIFS and squashfs) used for rootfs and the kernel should auto-probe just like it does for block or MTD devices (we support JFFS2 and squashfs on NOR and SPI flashes). In the case of squashfs being a read-only filesystem, an overlayfs is automatically created and used. Changing kernel command line parameters or anything which would require bootloader access is not an option for most of our users, due to the bootloaders being locked or because not everyone is into hooking up a serial console. Also, many stock bootloaders append stuff to the cmdline unwanted when using vanilla or OpenWrt's kernel. Thus, we often times ignore the bootloader's bootargs. The strategy of probe-mounting is working well as filesystems can easily be identified. The reason for this patch is that some users are getting confused about lines in in their bootlog indicating an error eventhough there wasn't anything bad happening. See also https://git.lede-project.org/?p=source.git;a=commit;h=fc90851df0810bf49283ed238a58641a819402bd Cheers Daniel > > Thanks, > //richard > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
Daniel, Am 19.07.2016 um 14:20 schrieb Daniel Golle: > Hi Richard, > > thanks for reviewing my patch! > > On Tue, Jul 19, 2016 at 07:38:28AM +0200, Richard Weinberger wrote: >> Am 19.07.2016 um 00:26 schrieb Daniel Golle: >>> Probe-mounting a volume too small for UBIFS results in kernel log >>> polution which might irritate users. >>> Address this by silencing errors which may happen during boot if the >>> rootfs is e.g. squashfs (and thus rather small) stored on a UBI volume. >>> This commit should be seen in context with >>> commit 65fb47721925628bf85ec08c4302ac5e7224a66a >>> and >>> commit 90bea5a3f0bf680b87b90516f3c231997f4b8f3b >> >> The diff makes sense but not the commit message. > > More brief? Less reasoning? Please elaborate, so I know what to > improve. It confused me a bit. :-) >> Why would you mount a squashfs on top of UBI as UBIFS? >> We have rootfstype= for this. > > However, in OpenWrt/LEDE users may choose the fstype (UBIFS and > squashfs) used for rootfs and the kernel should auto-probe just like > it does for block or MTD devices (we support JFFS2 and squashfs > on NOR and SPI flashes). In the case of squashfs being a read-only > filesystem, an overlayfs is automatically created and used. > Changing kernel command line parameters or anything which would > require bootloader access is not an option for most of our users, due > to the bootloaders being locked or because not everyone is into hooking > up a serial console. > Also, many stock bootloaders append stuff to the cmdline unwanted > when using vanilla or OpenWrt's kernel. Thus, we often times ignore > the bootloader's bootargs. > The strategy of probe-mounting is working well as filesystems can > easily be identified. The reason for this patch is that some users are > getting confused about lines in in their bootlog indicating an error > eventhough there wasn't anything bad happening. Ah that kind of kludges again... Anyway, patch looks good to me. BTW: We should rename ubifs_errc(), commit 235c362bd (UBIFS: extend debug/message capabilities) added the UBIFS context object to all logging functions. Thanks, //richard
Hi Richard, On Tue, Jul 19, 2016 at 02:28:50PM +0200, Richard Weinberger wrote: > ... > >> The diff makes sense but not the commit message. > > > > More brief? Less reasoning? Please elaborate, so I know what to > > improve. > > It confused me a bit. :-) Feel free to change it or let me know if I should reword it and resend the patch. > > >> Why would you mount a squashfs on top of UBI as UBIFS? > >> We have rootfstype= for this. > > > > However, in OpenWrt/LEDE users may choose the fstype (UBIFS and > > squashfs) used for rootfs and the kernel should auto-probe just like > > it does for block or MTD devices (we support JFFS2 and squashfs > > on NOR and SPI flashes). In the case of squashfs being a read-only > > filesystem, an overlayfs is automatically created and used. > > Changing kernel command line parameters or anything which would > > require bootloader access is not an option for most of our users, due > > to the bootloaders being locked or because not everyone is into hooking > > up a serial console. > > Also, many stock bootloaders append stuff to the cmdline unwanted > > when using vanilla or OpenWrt's kernel. Thus, we often times ignore > > the bootloader's bootargs. > > The strategy of probe-mounting is working well as filesystems can > > easily be identified. The reason for this patch is that some users are > > getting confused about lines in in their bootlog indicating an error > > eventhough there wasn't anything bad happening. > > Ah that kind of kludges again... > > Anyway, patch looks good to me. > > BTW: We should rename ubifs_errc(), commit 235c362bd (UBIFS: extend debug/message capabilities) > added the UBIFS context object to all logging functions. ... or at least the just comment above the macro definition. 'c' could stand for 'conditional' ;) Cheers Daniel
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 736dd58..4ec0510 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -520,19 +520,19 @@ static int init_constants_early(struct ubifs_info *c) c->max_write_shift = fls(c->max_write_size) - 1; if (c->leb_size < UBIFS_MIN_LEB_SZ) { - ubifs_err(c, "too small LEBs (%d bytes), min. is %d bytes", - c->leb_size, UBIFS_MIN_LEB_SZ); + ubifs_errc(c, "too small LEBs (%d bytes), min. is %d bytes", + c->leb_size, UBIFS_MIN_LEB_SZ); return -EINVAL; } if (c->leb_cnt < UBIFS_MIN_LEB_CNT) { - ubifs_err(c, "too few LEBs (%d), min. is %d", - c->leb_cnt, UBIFS_MIN_LEB_CNT); + ubifs_errc(c, "too few LEBs (%d), min. is %d", + c->leb_cnt, UBIFS_MIN_LEB_CNT); return -EINVAL; } if (!is_power_of_2(c->min_io_size)) { - ubifs_err(c, "bad min. I/O size %d", c->min_io_size); + ubifs_errc(c, "bad min. I/O size %d", c->min_io_size); return -EINVAL; } @@ -543,8 +543,8 @@ static int init_constants_early(struct ubifs_info *c) if (c->max_write_size < c->min_io_size || c->max_write_size % c->min_io_size || !is_power_of_2(c->max_write_size)) { - ubifs_err(c, "bad write buffer size %d for %d min. I/O unit", - c->max_write_size, c->min_io_size); + ubifs_errc(c, "bad write buffer size %d for %d min. I/O unit", + c->max_write_size, c->min_io_size); return -EINVAL; }
Probe-mounting a volume too small for UBIFS results in kernel log polution which might irritate users. Address this by silencing errors which may happen during boot if the rootfs is e.g. squashfs (and thus rather small) stored on a UBI volume. This commit should be seen in context with commit 65fb47721925628bf85ec08c4302ac5e7224a66a and commit 90bea5a3f0bf680b87b90516f3c231997f4b8f3b Signed-off-by: Daniel Golle <daniel@makrotopia.org> --- fs/ubifs/super.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)