[PATCH for-4.8] ubifs: silence early error messages if MS_SILENT is set

[PATCH for-4.8] ubifs: silence early error messages if MS_SILENT is set

[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

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 fs/ubifs/super.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

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;
 	}
 
-- 
2.9.0

Re: [PATCH for-4.8] ubifs: silence early error messages if MS_SILENT is set

[Richard Weinberger]

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

Re: [PATCH for-4.8] ubifs: silence early error messages if MS_SILENT is set

[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.

> 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/

Re: [PATCH for-4.8] ubifs: silence early error messages if MS_SILENT is set

[Richard Weinberger]

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

Re: [PATCH for-4.8] ubifs: silence early error messages if MS_SILENT is set

[Daniel Golle]

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

[PATCH] ubifs: update comment for ubifs_errc

[Daniel Golle]

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 fs/ubifs/ubifs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index ddf9f6b9..4617d45 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -1783,8 +1783,8 @@ void ubifs_err(const struct ubifs_info *c, const char *fmt, ...);
 __printf(2, 3)
 void ubifs_warn(const struct ubifs_info *c, const char *fmt, ...);
 /*
- * A variant of 'ubifs_err()' which takes the UBIFS file-sytem description
- * object as an argument.
+ * A conditional variant of 'ubifs_err()' which doesn't output anything
+ * if probing (ie. MS_SILENT set).
  */
 #define ubifs_errc(c, fmt, ...)						\
 do {									\
-- 
2.9.0

Re: [PATCH] ubifs: update comment for ubifs_errc

[Richard Weinberger]

Daniel,

Am 25.07.2016 um 13:44 schrieb Daniel Golle:
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  fs/ubifs/ubifs.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index ddf9f6b9..4617d45 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -1783,8 +1783,8 @@ void ubifs_err(const struct ubifs_info *c, const char *fmt, ...);
>  __printf(2, 3)
>  void ubifs_warn(const struct ubifs_info *c, const char *fmt, ...);
>  /*
> - * A variant of 'ubifs_err()' which takes the UBIFS file-sytem description
> - * object as an argument.
> + * A conditional variant of 'ubifs_err()' which doesn't output anything
> + * if probing (ie. MS_SILENT set).

Looks good. :-)

Thanks for improving this,
//richard