linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ubifs: respect MS_SILENT mount flag
@ 2014-05-17  1:55 Daniel Golle
  2014-05-27 12:18 ` Artem Bityutskiy
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Golle @ 2014-05-17  1:55 UTC (permalink / raw)
  To: dedekind1, linux-mtd

[-- Attachment #1: Type: text/plain, Size: 5735 bytes --]

When attempting to mount a non-ubifs formatted volume, lots of error
messages (including a stack dump) are thrown to the kernel log even if
the MS_SILENT mount flag is set.
Fix this by introducing an additional parameter in ubifs_read_node and
use it to pass down the MS_SILENT flag in ubifs_read_sb_node.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 fs/ubifs/commit.c   |  4 ++--
 fs/ubifs/io.c       | 23 ++++++++++++++---------
 fs/ubifs/sb.c       |  5 +++--
 fs/ubifs/tnc_misc.c |  4 ++--
 fs/ubifs/ubifs.h    |  2 +-
 5 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/fs/ubifs/commit.c b/fs/ubifs/commit.c
index ff82293..865d13f 100644
--- a/fs/ubifs/commit.c
+++ b/fs/ubifs/commit.c
@@ -542,7 +542,7 @@ int dbg_old_index_check_init(struct ubifs_info *c, struct ubifs_zbranch *zroot)
 	if (!idx)
 		return -ENOMEM;
 
-	err = ubifs_read_node(c, idx, UBIFS_IDX_NODE, len, lnum, offs);
+	err = ubifs_read_node(c, idx, UBIFS_IDX_NODE, len, lnum, offs, 0);
 	if (err)
 		goto out;
 
@@ -610,7 +610,7 @@ int dbg_check_old_index(struct ubifs_info *c, struct ubifs_zbranch *zroot)
 		list_add_tail(&i->list, &list);
 		/* Read the index node */
 		idx = &i->idx;
-		err = ubifs_read_node(c, idx, UBIFS_IDX_NODE, len, lnum, offs);
+		err = ubifs_read_node(c, idx, UBIFS_IDX_NODE, len, lnum, offs, 0);
 		if (err)
 			goto out_free;
 		/* Validate index node */
diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index e18b988..51c4072 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -912,7 +912,7 @@ int ubifs_read_node_wbuf(struct ubifs_wbuf *wbuf, void *buf, int type, int len,
 	if (!overlap) {
 		/* We may safely unlock the write-buffer and read the data */
 		spin_unlock(&wbuf->lock);
-		return ubifs_read_node(c, buf, type, len, lnum, offs);
+		return ubifs_read_node(c, buf, type, len, lnum, offs, 0);
 	}
 
 	/* Don't read under wbuf */
@@ -966,13 +966,14 @@ out:
  * @len: node length (not aligned)
  * @lnum: logical eraseblock number
  * @offs: offset within the logical eraseblock
+ * @silent: suppress error messages
  *
  * This function reads a node of known type and and length, checks it and
  * stores in @buf. Returns zero in case of success, %-EUCLEAN if CRC mismatched
  * and a negative error code in case of failure.
  */
 int ubifs_read_node(const struct ubifs_info *c, void *buf, int type, int len,
-		    int lnum, int offs)
+		    int lnum, int offs, int silent)
 {
 	int err, l;
 	struct ubifs_ch *ch = buf;
@@ -988,30 +989,34 @@ int ubifs_read_node(const struct ubifs_info *c, void *buf, int type, int len,
 		return err;
 
 	if (type != ch->node_type) {
-		ubifs_err("bad node type (%d but expected %d)",
+		if (!silent) ubifs_err("bad node type (%d but expected %d)",
 			  ch->node_type, type);
 		goto out;
 	}
 
 	err = ubifs_check_node(c, buf, lnum, offs, 0, 0);
 	if (err) {
-		ubifs_err("expected node type %d", type);
+		if (!silent)
+			ubifs_err("expected node type %d", type);
 		return err;
 	}
 
 	l = le32_to_cpu(ch->len);
 	if (l != len) {
-		ubifs_err("bad node length %d, expected %d", l, len);
+		if (!silent)
+			ubifs_err("bad node length %d, expected %d", l, len);
 		goto out;
 	}
 
 	return 0;
 
 out:
-	ubifs_err("bad node at LEB %d:%d, LEB mapping status %d", lnum, offs,
-		  ubi_is_mapped(c->ubi, lnum));
-	ubifs_dump_node(c, buf);
-	dump_stack();
+	if (!silent) {
+		ubifs_err("bad node at LEB %d:%d, LEB mapping status %d", lnum,
+			  offs, ubi_is_mapped(c->ubi, lnum));
+		ubifs_dump_node(c, buf);
+		dump_stack();
+	}
 	return -EINVAL;
 }
 
diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
index 4c37607..b46847d 100644
--- a/fs/ubifs/sb.c
+++ b/fs/ubifs/sb.c
@@ -482,14 +482,15 @@ failed:
 struct ubifs_sb_node *ubifs_read_sb_node(struct ubifs_info *c)
 {
 	struct ubifs_sb_node *sup;
-	int err;
+	int silent, err;
 
 	sup = kmalloc(ALIGN(UBIFS_SB_NODE_SZ, c->min_io_size), GFP_NOFS);
 	if (!sup)
 		return ERR_PTR(-ENOMEM);
 
+	silent = !!(c->vfs_sb->s_flags & MS_SILENT);
 	err = ubifs_read_node(c, sup, UBIFS_SB_NODE, UBIFS_SB_NODE_SZ,
-			      UBIFS_SB_LNUM, 0);
+			      UBIFS_SB_LNUM, 0, silent);
 	if (err) {
 		kfree(sup);
 		return ERR_PTR(err);
diff --git a/fs/ubifs/tnc_misc.c b/fs/ubifs/tnc_misc.c
index f6bf899..e128689 100644
--- a/fs/ubifs/tnc_misc.c
+++ b/fs/ubifs/tnc_misc.c
@@ -280,7 +280,7 @@ static int read_znode(struct ubifs_info *c, int lnum, int offs, int len,
 	if (!idx)
 		return -ENOMEM;
 
-	err = ubifs_read_node(c, idx, UBIFS_IDX_NODE, len, lnum, offs);
+	err = ubifs_read_node(c, idx, UBIFS_IDX_NODE, len, lnum, offs, 0);
 	if (err < 0) {
 		kfree(idx);
 		return err;
@@ -472,7 +472,7 @@ int ubifs_tnc_read_node(struct ubifs_info *c, struct ubifs_zbranch *zbr,
 					   zbr->lnum, zbr->offs);
 	else
 		err = ubifs_read_node(c, node, type, zbr->len, zbr->lnum,
-				      zbr->offs);
+				      zbr->offs, 0);
 
 	if (err) {
 		dbg_tnck(key, "key ");
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index e8c8cfe..85fdd11 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -1481,7 +1481,7 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len);
 int ubifs_wbuf_seek_nolock(struct ubifs_wbuf *wbuf, int lnum, int offs);
 int ubifs_wbuf_init(struct ubifs_info *c, struct ubifs_wbuf *wbuf);
 int ubifs_read_node(const struct ubifs_info *c, void *buf, int type, int len,
-		    int lnum, int offs);
+		    int lnum, int offs, int silent);
 int ubifs_read_node_wbuf(struct ubifs_wbuf *wbuf, void *buf, int type, int len,
 			 int lnum, int offs);
 int ubifs_write_node(struct ubifs_info *c, void *node, int len, int lnum,
-- 
1.9.2


[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

* Re: [PATCH] ubifs: respect MS_SILENT mount flag
  2014-05-17  1:55 [PATCH] ubifs: respect MS_SILENT mount flag Daniel Golle
@ 2014-05-27 12:18 ` Artem Bityutskiy
  2014-05-27 14:11   ` [PATCH v2] " Daniel Golle
  0 siblings, 1 reply; 19+ messages in thread
From: Artem Bityutskiy @ 2014-05-27 12:18 UTC (permalink / raw)
  To: Daniel Golle; +Cc: linux-mtd

On Sat, 2014-05-17 at 03:55 +0200, Daniel Golle wrote:
> When attempting to mount a non-ubifs formatted volume, lots of error
> messages (including a stack dump) are thrown to the kernel log even if
> the MS_SILENT mount flag is set.
> Fix this by introducing an additional parameter in ubifs_read_node and
> use it to pass down the MS_SILENT flag in ubifs_read_sb_node.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>

This looks reasonable. Could you please send v2 with checkpatch.pl
complaints addressed?

Thanks!

-- 
Best Regards,
Artem Bityutskiy

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

* [PATCH v2] ubifs: respect MS_SILENT mount flag
  2014-05-27 12:18 ` Artem Bityutskiy
@ 2014-05-27 14:11   ` Daniel Golle
  2014-05-27 14:56     ` Artem Bityutskiy
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Golle @ 2014-05-27 14:11 UTC (permalink / raw)
  To: Artem Bityutskiy; +Cc: linux-mtd

[-- Attachment #1: Type: text/plain, Size: 5932 bytes --]

When attempting to mount a non-ubifs formatted volume, lots of error
messages (including a stack dump) are thrown to the kernel log even if
the MS_SILENT mount flag is set.
Fix this by introducing an additional parameter in ubifs_read_node and
use it to pass down the MS_SILENT flag in ubifs_read_sb_node.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: fixed checkpatch.pl issues, not sure about the newline in
    fs/ubifs/commit.c:610 which leaves the single 0 looking a bit
    lonely...

 fs/ubifs/commit.c   |  5 +++--
 fs/ubifs/io.c       | 26 ++++++++++++++++----------
 fs/ubifs/sb.c       |  5 +++--
 fs/ubifs/tnc_misc.c |  4 ++--
 fs/ubifs/ubifs.h    |  2 +-
 5 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/fs/ubifs/commit.c b/fs/ubifs/commit.c
index ff82293..936617f 100644
--- a/fs/ubifs/commit.c
+++ b/fs/ubifs/commit.c
@@ -542,7 +542,7 @@ int dbg_old_index_check_init(struct ubifs_info *c, struct ubifs_zbranch *zroot)
 	if (!idx)
 		return -ENOMEM;
 
-	err = ubifs_read_node(c, idx, UBIFS_IDX_NODE, len, lnum, offs);
+	err = ubifs_read_node(c, idx, UBIFS_IDX_NODE, len, lnum, offs, 0);
 	if (err)
 		goto out;
 
@@ -610,7 +610,8 @@ int dbg_check_old_index(struct ubifs_info *c, struct ubifs_zbranch *zroot)
 		list_add_tail(&i->list, &list);
 		/* Read the index node */
 		idx = &i->idx;
-		err = ubifs_read_node(c, idx, UBIFS_IDX_NODE, len, lnum, offs);
+		err = ubifs_read_node(c, idx, UBIFS_IDX_NODE, len, lnum, offs
+				      0);
 		if (err)
 			goto out_free;
 		/* Validate index node */
diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index e18b988..8c5dea1 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -912,7 +912,7 @@ int ubifs_read_node_wbuf(struct ubifs_wbuf *wbuf, void *buf, int type, int len,
 	if (!overlap) {
 		/* We may safely unlock the write-buffer and read the data */
 		spin_unlock(&wbuf->lock);
-		return ubifs_read_node(c, buf, type, len, lnum, offs);
+		return ubifs_read_node(c, buf, type, len, lnum, offs, 0);
 	}
 
 	/* Don't read under wbuf */
@@ -966,13 +966,14 @@ out:
  * @len: node length (not aligned)
  * @lnum: logical eraseblock number
  * @offs: offset within the logical eraseblock
+ * @silent: suppress error messages
  *
  * This function reads a node of known type and and length, checks it and
  * stores in @buf. Returns zero in case of success, %-EUCLEAN if CRC mismatched
  * and a negative error code in case of failure.
  */
 int ubifs_read_node(const struct ubifs_info *c, void *buf, int type, int len,
-		    int lnum, int offs)
+		    int lnum, int offs, int silent)
 {
 	int err, l;
 	struct ubifs_ch *ch = buf;
@@ -988,30 +989,35 @@ int ubifs_read_node(const struct ubifs_info *c, void *buf, int type, int len,
 		return err;
 
 	if (type != ch->node_type) {
-		ubifs_err("bad node type (%d but expected %d)",
-			  ch->node_type, type);
+		if (!silent)
+			ubifs_err("bad node type (%d but expected %d)",
+				  ch->node_type, type);
 		goto out;
 	}
 
 	err = ubifs_check_node(c, buf, lnum, offs, 0, 0);
 	if (err) {
-		ubifs_err("expected node type %d", type);
+		if (!silent)
+			ubifs_err("expected node type %d", type);
 		return err;
 	}
 
 	l = le32_to_cpu(ch->len);
 	if (l != len) {
-		ubifs_err("bad node length %d, expected %d", l, len);
+		if (!silent)
+			ubifs_err("bad node length %d, expected %d", l, len);
 		goto out;
 	}
 
 	return 0;
 
 out:
-	ubifs_err("bad node at LEB %d:%d, LEB mapping status %d", lnum, offs,
-		  ubi_is_mapped(c->ubi, lnum));
-	ubifs_dump_node(c, buf);
-	dump_stack();
+	if (!silent) {
+		ubifs_err("bad node at LEB %d:%d, LEB mapping status %d", lnum,
+			  offs, ubi_is_mapped(c->ubi, lnum));
+		ubifs_dump_node(c, buf);
+		dump_stack();
+	}
 	return -EINVAL;
 }
 
diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
index 4c37607..b46847d 100644
--- a/fs/ubifs/sb.c
+++ b/fs/ubifs/sb.c
@@ -482,14 +482,15 @@ failed:
 struct ubifs_sb_node *ubifs_read_sb_node(struct ubifs_info *c)
 {
 	struct ubifs_sb_node *sup;
-	int err;
+	int silent, err;
 
 	sup = kmalloc(ALIGN(UBIFS_SB_NODE_SZ, c->min_io_size), GFP_NOFS);
 	if (!sup)
 		return ERR_PTR(-ENOMEM);
 
+	silent = !!(c->vfs_sb->s_flags & MS_SILENT);
 	err = ubifs_read_node(c, sup, UBIFS_SB_NODE, UBIFS_SB_NODE_SZ,
-			      UBIFS_SB_LNUM, 0);
+			      UBIFS_SB_LNUM, 0, silent);
 	if (err) {
 		kfree(sup);
 		return ERR_PTR(err);
diff --git a/fs/ubifs/tnc_misc.c b/fs/ubifs/tnc_misc.c
index f6bf899..e128689 100644
--- a/fs/ubifs/tnc_misc.c
+++ b/fs/ubifs/tnc_misc.c
@@ -280,7 +280,7 @@ static int read_znode(struct ubifs_info *c, int lnum, int offs, int len,
 	if (!idx)
 		return -ENOMEM;
 
-	err = ubifs_read_node(c, idx, UBIFS_IDX_NODE, len, lnum, offs);
+	err = ubifs_read_node(c, idx, UBIFS_IDX_NODE, len, lnum, offs, 0);
 	if (err < 0) {
 		kfree(idx);
 		return err;
@@ -472,7 +472,7 @@ int ubifs_tnc_read_node(struct ubifs_info *c, struct ubifs_zbranch *zbr,
 					   zbr->lnum, zbr->offs);
 	else
 		err = ubifs_read_node(c, node, type, zbr->len, zbr->lnum,
-				      zbr->offs);
+				      zbr->offs, 0);
 
 	if (err) {
 		dbg_tnck(key, "key ");
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index e8c8cfe..85fdd11 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -1481,7 +1481,7 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len);
 int ubifs_wbuf_seek_nolock(struct ubifs_wbuf *wbuf, int lnum, int offs);
 int ubifs_wbuf_init(struct ubifs_info *c, struct ubifs_wbuf *wbuf);
 int ubifs_read_node(const struct ubifs_info *c, void *buf, int type, int len,
-		    int lnum, int offs);
+		    int lnum, int offs, int silent);
 int ubifs_read_node_wbuf(struct ubifs_wbuf *wbuf, void *buf, int type, int len,
 			 int lnum, int offs);
 int ubifs_write_node(struct ubifs_info *c, void *node, int len, int lnum,
-- 
1.9.3


[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

* Re: [PATCH v2] ubifs: respect MS_SILENT mount flag
  2014-05-27 14:11   ` [PATCH v2] " Daniel Golle
@ 2014-05-27 14:56     ` Artem Bityutskiy
  2014-05-27 16:04       ` Daniel
  0 siblings, 1 reply; 19+ messages in thread
From: Artem Bityutskiy @ 2014-05-27 14:56 UTC (permalink / raw)
  To: Daniel Golle; +Cc: linux-mtd

On Tue, 2014-05-27 at 16:11 +0200, Daniel Golle wrote:
> When attempting to mount a non-ubifs formatted volume, lots of error
> messages (including a stack dump) are thrown to the kernel log even if
> the MS_SILENT mount flag is set.
> Fix this by introducing an additional parameter in ubifs_read_node and
> use it to pass down the MS_SILENT flag in ubifs_read_sb_node.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>

Thanks. How did you test this patch?

-- 
Best Regards,
Artem Bityutskiy

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

* Re: [PATCH v2] ubifs: respect MS_SILENT mount flag
  2014-05-27 14:56     ` Artem Bityutskiy
@ 2014-05-27 16:04       ` Daniel
  2014-05-28  2:11         ` hujianyang
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel @ 2014-05-27 16:04 UTC (permalink / raw)
  To: linux-mtd, Artem Bityutskiy

On 05/27/2014 04:56 PM, Artem Bityutskiy wrote:
> On Tue, 2014-05-27 at 16:11 +0200, Daniel Golle wrote:
>> When attempting to mount a non-ubifs formatted volume, lots of error
>> messages (including a stack dump) are thrown to the kernel log even if
>> the MS_SILENT mount flag is set.
>> Fix this by introducing an additional parameter in ubifs_read_node and
>> use it to pass down the MS_SILENT flag in ubifs_read_sb_node.
>>
>> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> 
> Thanks. How did you test this patch?

You can test this by trying to mount a non-empty volume which does not contain a
UBIFS superblock (but e.g. squashfs or a U-Boot environment) with
mount -t ubifs -o silent /dev/ubiX_Y /mnt
This should fail without creating any klog lines.

The reason that I want this is that I'm working on integration of UBI support in
OpenWrt, including auto-mounting the "rootfs" volume by default (if it exists)
in case the rootfs and/or rootfstype parameters are not passed-down by the
bootloader, see
https://gitorious.org/openwrt-oxnas/openwrt-oxnas/commit/e1306d7b9bee8a39a33147d93cb399a4621bf3aa

The idea is to have the same level of features and comfort also on devices where
UBI is being used, for MTD devices OpenWrt does something similar
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.14/480-mtd-set-rootfs-to-be-root-dev.patch

However, this is probably distribution-specific hackery, but independently of
that, UBIFS should still respect the MS_SILENT flag just like all other
filesystems do.

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

* Re: [PATCH v2] ubifs: respect MS_SILENT mount flag
  2014-05-27 16:04       ` Daniel
@ 2014-05-28  2:11         ` hujianyang
  2014-05-28  8:01           ` Artem Bityutskiy
  0 siblings, 1 reply; 19+ messages in thread
From: hujianyang @ 2014-05-28  2:11 UTC (permalink / raw)
  To: Daniel, Artem Bityutskiy; +Cc: linux-mtd

Hi Daniel and Artem,

> 
> You can test this by trying to mount a non-empty volume which does not contain a
> UBIFS superblock (but e.g. squashfs or a U-Boot environment) with
> mount -t ubifs -o silent /dev/ubiX_Y /mnt
> This should fail without creating any klog lines.
> 

I think disabling log message in this case is needed. But I'm sorry to say
I don't like just adding a new parameter to ubifs_read_node in this way
because this silent flag seems only used during mount. This adding makes the
parameters different in ubifs_read_node and ubifs_write_node, also not good
for reading code.

How about to add a separate func ubifs_read_node_silent to instead this
ubifs_read_node in ubifs_read_sb_node? I think we could do more proper work
in this new function.

> 
> However, this is probably distribution-specific hackery, but independently of
> that, UBIFS should still respect the MS_SILENT flag just like all other
> filesystems do.
> 

Do you get some more error messages not only in ubifs_read_node during mount?
I think this is a good suggestion. Current code do not use @silent in
ubifs_fill_super and we should do some work to enable it like other filesystems.

Thanks!

Hu

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

* Re: [PATCH v2] ubifs: respect MS_SILENT mount flag
  2014-05-28  2:11         ` hujianyang
@ 2014-05-28  8:01           ` Artem Bityutskiy
  2014-05-28  8:07             ` Bityutskiy, Artem
  0 siblings, 1 reply; 19+ messages in thread
From: Artem Bityutskiy @ 2014-05-28  8:01 UTC (permalink / raw)
  To: hujianyang; +Cc: linux-mtd, Daniel

On Wed, 2014-05-28 at 10:11 +0800, hujianyang wrote:
> Hi Daniel and Artem,
> 
> > 
> > You can test this by trying to mount a non-empty volume which does not contain a
> > UBIFS superblock (but e.g. squashfs or a U-Boot environment) with
> > mount -t ubifs -o silent /dev/ubiX_Y /mnt
> > This should fail without creating any klog lines.
> > 
> 
> I think disabling log message in this case is needed. But I'm sorry to say
> I don't like just adding a new parameter to ubifs_read_node in this way
> because this silent flag seems only used during mount. This adding makes the
> parameters different in ubifs_read_node and ubifs_write_node, also not good
> for reading code.

So you bring up 2 points.

1. This patch only makes the read part of the mount code-path silent,
everything else stays normal.
2. Another parameter makes code less readable.

WRT the former, do we really need more than that? IIUC, this is exactly
the meaning of the 'silent' flag, and exactly its purpose. IIUC, it
serves a single use-case: file-system probing. And the patch does
exactly that.

> How about to add a separate func ubifs_read_node_silent to instead this
> ubifs_read_node in ubifs_read_sb_node? I think we could do more proper work
> in this new function.

This would introduce code duplication, and I am not sure if this is much
better than what Daniel suggests.

How about introducing a per-file-system flag, like c->probing or
something, and make do something like this:

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index a81c7b5..a627476 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1210,7 +1210,12 @@ static int mount_ubifs(struct ubifs_info *c)
 
        c->mounting = 1;
 
+       /*
+        * Good commend describing what we are doing.
+        */
+       c->probing = 1
        err = ubifs_read_superblock(c);
+       c->probing = 0
        if (err)
                goto out_free;
 

After all, we already have a number of "state" flags like 'c->mounting',
so adding another one is not that big deal.

-- 
Best Regards,
Artem Bityutskiy

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

* Re: [PATCH v2] ubifs: respect MS_SILENT mount flag
  2014-05-28  8:01           ` Artem Bityutskiy
@ 2014-05-28  8:07             ` Bityutskiy, Artem
  2014-05-28  8:14               ` Artem Bityutskiy
  0 siblings, 1 reply; 19+ messages in thread
From: Bityutskiy, Artem @ 2014-05-28  8:07 UTC (permalink / raw)
  To: hujianyang@huawei.com
  Cc: linux-mtd@lists.infradead.org, daniel@makrotopia.org

On Wed, 2014-05-28 at 11:01 +0300, Artem Bityutskiy wrote: 
> +       /*
> +        * Good commend describing what we are doing.
> +        */
> +       c->probing = 1

Err, actually:

          c->probing = silent;

>         err = ubifs_read_superblock(c);
> +       c->probing = 0
>         if (err)
>                 goto out_free;

-- 
Best Regards,
Artem Bityutskiy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

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

* Re: [PATCH v2] ubifs: respect MS_SILENT mount flag
  2014-05-28  8:07             ` Bityutskiy, Artem
@ 2014-05-28  8:14               ` Artem Bityutskiy
  2014-05-28  8:28                 ` Artem Bityutskiy
  2014-05-30 22:01                 ` [PATCH v3] " Daniel Golle
  0 siblings, 2 replies; 19+ messages in thread
From: Artem Bityutskiy @ 2014-05-28  8:14 UTC (permalink / raw)
  To: hujianyang@huawei.com
  Cc: linux-mtd@lists.infradead.org, daniel@makrotopia.org

On Wed, 2014-05-28 at 08:07 +0000, Bityutskiy, Artem wrote:
> On Wed, 2014-05-28 at 11:01 +0300, Artem Bityutskiy wrote: 
> > +       /*
> > +        * Good commend describing what we are doing.
> > +        */
> > +       c->probing = 1
> 
> Err, actually:
> 
>           c->probing = silent;
> 
> >         err = ubifs_read_superblock(c);
> > +       c->probing = 0
> >         if (err)
> >                 goto out_free;

I guess you guys got the idea, but just in case, you also will need to
add "if (!c->probing) { ubifs_error() }" in the relevant places.

-- 
Best Regards,
Artem Bityutskiy

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

* Re: [PATCH v2] ubifs: respect MS_SILENT mount flag
  2014-05-28  8:14               ` Artem Bityutskiy
@ 2014-05-28  8:28                 ` Artem Bityutskiy
  2014-05-28  8:42                   ` hujianyang
  2014-05-30 22:01                 ` [PATCH v3] " Daniel Golle
  1 sibling, 1 reply; 19+ messages in thread
From: Artem Bityutskiy @ 2014-05-28  8:28 UTC (permalink / raw)
  To: hujianyang@huawei.com
  Cc: linux-mtd@lists.infradead.org, daniel@makrotopia.org

On Wed, 2014-05-28 at 11:14 +0300, Artem Bityutskiy wrote:
> On Wed, 2014-05-28 at 08:07 +0000, Bityutskiy, Artem wrote:
> > On Wed, 2014-05-28 at 11:01 +0300, Artem Bityutskiy wrote: 
> > > +       /*
> > > +        * Good commend describing what we are doing.
> > > +        */
> > > +       c->probing = 1
> > 
> > Err, actually:
> > 
> >           c->probing = silent;
> > 
> > >         err = ubifs_read_superblock(c);
> > > +       c->probing = 0
> > >         if (err)
> > >                 goto out_free;
> 
> I guess you guys got the idea, but just in case, you also will need to
> add "if (!c->probing) { ubifs_error() }" in the relevant places.

Or even introduce a new version of the error macro, something like
'ubifs_errc(), and use that in the relevant places. Not sure what is
going to look better, though. Here is a sketch:


diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index e8c8cfe..60cffa7 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -52,6 +52,14 @@
        pr_warn("UBIFS warning (pid %d): %s: " fmt "\n",            \
                current->pid, __func__, ##__VA_ARGS__)
 
+/*
+ * A variant of 'ubifs_err()' which takes the UBIFS file-sytem description
+ * object as an argument.
+ */
+#define ubifs_errc(c, fmt, ...)                                    \
+       if (!(c)->probing)                                          \
+               ubifs_err(fmt, ##__VA_ARGS__)
+
 /* UBIFS file system VFS magic number */
 #define UBIFS_SUPER_MAGIC 0x24051905




-- 
Best Regards,
Artem Bityutskiy

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

* Re: [PATCH v2] ubifs: respect MS_SILENT mount flag
  2014-05-28  8:28                 ` Artem Bityutskiy
@ 2014-05-28  8:42                   ` hujianyang
  2014-05-28  9:29                     ` Artem Bityutskiy
  0 siblings, 1 reply; 19+ messages in thread
From: hujianyang @ 2014-05-28  8:42 UTC (permalink / raw)
  To: dedekind1; +Cc: linux-mtd

Hi Artem,

> 
> Or even introduce a new version of the error macro, something like
> 'ubifs_errc(), and use that in the relevant places. Not sure what is
> going to look better, though. Here is a sketch:
> 
> 
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index e8c8cfe..60cffa7 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -52,6 +52,14 @@
>         pr_warn("UBIFS warning (pid %d): %s: " fmt "\n",            \
>                 current->pid, __func__, ##__VA_ARGS__)
>  
> +/*
> + * A variant of 'ubifs_err()' which takes the UBIFS file-sytem description
> + * object as an argument.
> + */
> +#define ubifs_errc(c, fmt, ...)                                    \
> +       if (!(c)->probing)                                          \
> +               ubifs_err(fmt, ##__VA_ARGS__)
> +
>  /* UBIFS file system VFS magic number */
>  #define UBIFS_SUPER_MAGIC 0x24051905
> 

I have to say I was just writing to you about adding a new marco to instead.
Ha~

In this way we can disable unnecessary messages during mount if we use "slient"
flag. I have a simple question,Is this @probing safe during mount? Are there
some probably race conditions of it? I have no experiences of these.

I think you should find out which ubifs_err should be replace into your
"ubifs_errc" in the next step.

Thank you very much!


Hu

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

* Re: [PATCH v2] ubifs: respect MS_SILENT mount flag
  2014-05-28  8:42                   ` hujianyang
@ 2014-05-28  9:29                     ` Artem Bityutskiy
  0 siblings, 0 replies; 19+ messages in thread
From: Artem Bityutskiy @ 2014-05-28  9:29 UTC (permalink / raw)
  To: hujianyang; +Cc: linux-mtd

On Wed, 2014-05-28 at 16:42 +0800, hujianyang wrote:
> In this way we can disable unnecessary messages during mount if we use "slient"
> flag. I have a simple question,Is this @probing safe during mount? Are there
> some probably race conditions of it? I have no experiences of these.

I do not think so, the mount path is "single-stream".


-- 
Best Regards,
Artem Bityutskiy

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

* [PATCH v3] ubifs: respect MS_SILENT mount flag
  2014-05-28  8:14               ` Artem Bityutskiy
  2014-05-28  8:28                 ` Artem Bityutskiy
@ 2014-05-30 22:01                 ` Daniel Golle
  2014-05-30 23:20                   ` Brian Norris
  1 sibling, 1 reply; 19+ messages in thread
From: Daniel Golle @ 2014-05-30 22:01 UTC (permalink / raw)
  To: dedekind1, hujianyang, linux-mtd

[-- Attachment #1: Type: text/plain, Size: 3308 bytes --]

When attempting to mount a non-ubifs formatted volume, lots of error
messages (including a stack dump) are thrown to the kernel log even if
the MS_SILENT mount flag is set.
Fix this by introducing adding an additional state-variable in
struct ubifs_info and suppress error messages in ubifs_read_node if
MS_SILENT is set.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v3: use state variable in ubifs_info instead of 
 fs/ubifs/io.c    | 14 ++++++++------
 fs/ubifs/super.c |  4 ++++
 fs/ubifs/ubifs.h |  9 +++++++++
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index e18b988..e6a31faa 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -988,30 +988,32 @@ int ubifs_read_node(const struct ubifs_info *c, void *buf, int type, int len,
 		return err;
 
 	if (type != ch->node_type) {
-		ubifs_err("bad node type (%d but expected %d)",
+		ubifs_errc(c, "bad node type (%d but expected %d)",
 			  ch->node_type, type);
 		goto out;
 	}
 
 	err = ubifs_check_node(c, buf, lnum, offs, 0, 0);
 	if (err) {
-		ubifs_err("expected node type %d", type);
+		ubifs_errc(c, "expected node type %d", type);
 		return err;
 	}
 
 	l = le32_to_cpu(ch->len);
 	if (l != len) {
-		ubifs_err("bad node length %d, expected %d", l, len);
+		ubifs_errc(c, "bad node length %d, expected %d", l, len);
 		goto out;
 	}
 
 	return 0;
 
 out:
-	ubifs_err("bad node at LEB %d:%d, LEB mapping status %d", lnum, offs,
+	ubifs_errc(c, "bad node at LEB %d:%d, LEB mapping status %d", lnum, offs,
 		  ubi_is_mapped(c->ubi, lnum));
-	ubifs_dump_node(c, buf);
-	dump_stack();
+	if (!c->probing) {
+		ubifs_dump_node(c, buf);
+		dump_stack();
+	}
 	return -EINVAL;
 }
 
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index a81c7b5..e88bbb6 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1149,6 +1149,8 @@ static int mount_ubifs(struct ubifs_info *c)
 	size_t sz;
 
 	c->ro_mount = !!(c->vfs_sb->s_flags & MS_RDONLY);
+	c->probing = !!(c->vfs_sb->s_flags & MS_SILENT);
+
 	err = init_constants_early(c);
 	if (err)
 		return err;
@@ -1214,6 +1216,8 @@ static int mount_ubifs(struct ubifs_info *c)
 	if (err)
 		goto out_free;
 
+	c->probing = 0;
+
 	/*
 	 * Make sure the compressor which is set as default in the superblock
 	 * or overridden by mount options is actually compiled in.
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index e8c8cfe..6aa1550 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -52,6 +52,14 @@
 	pr_warn("UBIFS warning (pid %d): %s: " fmt "\n",            \
 		current->pid, __func__, ##__VA_ARGS__)
 
+/*
+ * A variant of 'ubifs_err()' which takes the UBIFS file-sytem description
+ * object as an argument.
+ */
+#define ubifs_errc(c, fmt, ...)                                    \
+	if (!(c)->probing)                                         \
+		ubifs_err(fmt, ##__VA_ARGS__)
+
 /* UBIFS file system VFS magic number */
 #define UBIFS_SUPER_MAGIC 0x24051905
 
@@ -1441,6 +1449,7 @@ struct ubifs_info {
 	unsigned int replaying:1;
 	unsigned int mounting:1;
 	unsigned int remounting_rw:1;
+	unsigned int probing:1;
 	struct list_head replay_list;
 	struct list_head replay_buds;
 	unsigned long long cs_sqnum;
-- 
1.9.3


[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

* Re: [PATCH v3] ubifs: respect MS_SILENT mount flag
  2014-05-30 22:01                 ` [PATCH v3] " Daniel Golle
@ 2014-05-30 23:20                   ` Brian Norris
  2014-05-30 23:32                     ` [PATCH v4] " Daniel Golle
  2014-05-31  0:01                     ` [PATCH v5] " Daniel Golle
  0 siblings, 2 replies; 19+ messages in thread
From: Brian Norris @ 2014-05-30 23:20 UTC (permalink / raw)
  To: Daniel Golle; +Cc: hujianyang, linux-mtd, dedekind1

Hi Daniel,

On Sat, May 31, 2014 at 12:01:56AM +0200, Daniel Golle wrote:
> When attempting to mount a non-ubifs formatted volume, lots of error
> messages (including a stack dump) are thrown to the kernel log even if
> the MS_SILENT mount flag is set.
> Fix this by introducing adding an additional state-variable in
> struct ubifs_info and suppress error messages in ubifs_read_node if
> MS_SILENT is set.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> v3: use state variable in ubifs_info instead of 

Instead of what?? I'm dying to know!

(Just kidding; I can read your previous conversations. Hint: the answer
is "a function argument.")

>  fs/ubifs/io.c    | 14 ++++++++------
>  fs/ubifs/super.c |  4 ++++
>  fs/ubifs/ubifs.h |  9 +++++++++
>  3 files changed, 21 insertions(+), 6 deletions(-)
> 
[...]
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index e8c8cfe..6aa1550 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -52,6 +52,14 @@
>  	pr_warn("UBIFS warning (pid %d): %s: " fmt "\n",            \
>  		current->pid, __func__, ##__VA_ARGS__)
>  
> +/*
> + * A variant of 'ubifs_err()' which takes the UBIFS file-sytem description
> + * object as an argument.
> + */
> +#define ubifs_errc(c, fmt, ...)                                    \
> +	if (!(c)->probing)                                         \
> +		ubifs_err(fmt, ##__VA_ARGS__)

Nitpick: might you want to wrap the whole condition in do { } while (0),
so that a user can't shoot themselves in the foot with something like:

	if (condition)
		ubifs_errc(c, "hello world");
	else
		do_something_else();

which will expand to the following unexpected code:

	if (condition)
		if (!c->probing)
			ubifs_err("hello world");
		else
			do_something_else();

> +
>  /* UBIFS file system VFS magic number */
>  #define UBIFS_SUPER_MAGIC 0x24051905
>  

Brian

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

* [PATCH v4] ubifs: respect MS_SILENT mount flag
  2014-05-30 23:20                   ` Brian Norris
@ 2014-05-30 23:32                     ` Daniel Golle
  2014-05-31  0:01                     ` [PATCH v5] " Daniel Golle
  1 sibling, 0 replies; 19+ messages in thread
From: Daniel Golle @ 2014-05-30 23:32 UTC (permalink / raw)
  To: dedekind1, hujianyang, computersforpeace, linux-mtd

[-- Attachment #1: Type: text/plain, Size: 3486 bytes --]

When attempting to mount a non-ubifs formatted volume, lots of error
messages (including a stack dump) are thrown to the kernel log even if
the MS_SILENT mount flag is set.
Fix this by introducing adding an additional state-variable in
struct ubifs_info and suppress error messages in ubifs_read_node if
MS_SILENT is set.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v4: enclose conditional error printing macro by do { } while (0)
v3: use state variable in ubifs_info instead of a function parameter

 fs/ubifs/io.c    | 14 ++++++++------
 fs/ubifs/super.c |  4 ++++
 fs/ubifs/ubifs.h | 11 +++++++++++
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index e18b988..e6a31faa 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -988,30 +988,32 @@ int ubifs_read_node(const struct ubifs_info *c, void *buf, int type, int len,
 		return err;
 
 	if (type != ch->node_type) {
-		ubifs_err("bad node type (%d but expected %d)",
+		ubifs_errc(c, "bad node type (%d but expected %d)",
 			  ch->node_type, type);
 		goto out;
 	}
 
 	err = ubifs_check_node(c, buf, lnum, offs, 0, 0);
 	if (err) {
-		ubifs_err("expected node type %d", type);
+		ubifs_errc(c, "expected node type %d", type);
 		return err;
 	}
 
 	l = le32_to_cpu(ch->len);
 	if (l != len) {
-		ubifs_err("bad node length %d, expected %d", l, len);
+		ubifs_errc(c, "bad node length %d, expected %d", l, len);
 		goto out;
 	}
 
 	return 0;
 
 out:
-	ubifs_err("bad node at LEB %d:%d, LEB mapping status %d", lnum, offs,
+	ubifs_errc(c, "bad node at LEB %d:%d, LEB mapping status %d", lnum, offs,
 		  ubi_is_mapped(c->ubi, lnum));
-	ubifs_dump_node(c, buf);
-	dump_stack();
+	if (!c->probing) {
+		ubifs_dump_node(c, buf);
+		dump_stack();
+	}
 	return -EINVAL;
 }
 
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index a81c7b5..e88bbb6 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1149,6 +1149,8 @@ static int mount_ubifs(struct ubifs_info *c)
 	size_t sz;
 
 	c->ro_mount = !!(c->vfs_sb->s_flags & MS_RDONLY);
+	c->probing = !!(c->vfs_sb->s_flags & MS_SILENT);
+
 	err = init_constants_early(c);
 	if (err)
 		return err;
@@ -1214,6 +1216,8 @@ static int mount_ubifs(struct ubifs_info *c)
 	if (err)
 		goto out_free;
 
+	c->probing = 0;
+
 	/*
 	 * Make sure the compressor which is set as default in the superblock
 	 * or overridden by mount options is actually compiled in.
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index e8c8cfe..d944d4d 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -52,6 +52,16 @@
 	pr_warn("UBIFS warning (pid %d): %s: " fmt "\n",            \
 		current->pid, __func__, ##__VA_ARGS__)
 
+/*
+ * A variant of 'ubifs_err()' which takes the UBIFS file-sytem description
+ * object as an argument.
+ */
+#define ubifs_errc(c, fmt, ...)                                    \
+	do {                                                       \
+		if (!(c)->probing)                                 \
+			ubifs_err(fmt, ##__VA_ARGS__)              \
+	} while (0)
+
 /* UBIFS file system VFS magic number */
 #define UBIFS_SUPER_MAGIC 0x24051905
 
@@ -1441,6 +1451,7 @@ struct ubifs_info {
 	unsigned int replaying:1;
 	unsigned int mounting:1;
 	unsigned int remounting_rw:1;
+	unsigned int probing:1;
 	struct list_head replay_list;
 	struct list_head replay_buds;
 	unsigned long long cs_sqnum;
-- 
1.9.3


[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

* [PATCH v5] ubifs: respect MS_SILENT mount flag
  2014-05-30 23:20                   ` Brian Norris
  2014-05-30 23:32                     ` [PATCH v4] " Daniel Golle
@ 2014-05-31  0:01                     ` Daniel Golle
  2014-06-02  7:59                       ` Artem Bityutskiy
  1 sibling, 1 reply; 19+ messages in thread
From: Daniel Golle @ 2014-05-31  0:01 UTC (permalink / raw)
  To: dedekind1, hujianyang, computersforpeace, linux-mtd

[-- Attachment #1: Type: text/plain, Size: 3577 bytes --]

When attempting to mount a non-ubifs formatted volume, lots of error
messages (including a stack dump) are thrown to the kernel log even if
the MS_SILENT mount flag is set.
Fix this by introducing adding an additional state-variable in
struct ubifs_info and suppress error messages in ubifs_read_node if
MS_SILENT is set.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v5: added missing semi-colon and fixed a style issue
v4: enclose conditional error printing macro by do { } while (0)
v3: use state variable in ubifs_info instead of a function parameter

 fs/ubifs/io.c    | 16 +++++++++-------
 fs/ubifs/super.c |  4 ++++
 fs/ubifs/ubifs.h | 11 +++++++++++
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index e18b988..3205929 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -988,30 +988,32 @@ int ubifs_read_node(const struct ubifs_info *c, void *buf, int type, int len,
 		return err;
 
 	if (type != ch->node_type) {
-		ubifs_err("bad node type (%d but expected %d)",
+		ubifs_errc(c, "bad node type (%d but expected %d)",
 			  ch->node_type, type);
 		goto out;
 	}
 
 	err = ubifs_check_node(c, buf, lnum, offs, 0, 0);
 	if (err) {
-		ubifs_err("expected node type %d", type);
+		ubifs_errc(c, "expected node type %d", type);
 		return err;
 	}
 
 	l = le32_to_cpu(ch->len);
 	if (l != len) {
-		ubifs_err("bad node length %d, expected %d", l, len);
+		ubifs_errc(c, "bad node length %d, expected %d", l, len);
 		goto out;
 	}
 
 	return 0;
 
 out:
-	ubifs_err("bad node at LEB %d:%d, LEB mapping status %d", lnum, offs,
-		  ubi_is_mapped(c->ubi, lnum));
-	ubifs_dump_node(c, buf);
-	dump_stack();
+	ubifs_errc(c, "bad node at LEB %d:%d, LEB mapping status %d", lnum,
+		  offs, ubi_is_mapped(c->ubi, lnum));
+	if (!c->probing) {
+		ubifs_dump_node(c, buf);
+		dump_stack();
+	}
 	return -EINVAL;
 }
 
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index a81c7b5..e88bbb6 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1149,6 +1149,8 @@ static int mount_ubifs(struct ubifs_info *c)
 	size_t sz;
 
 	c->ro_mount = !!(c->vfs_sb->s_flags & MS_RDONLY);
+	c->probing = !!(c->vfs_sb->s_flags & MS_SILENT);
+
 	err = init_constants_early(c);
 	if (err)
 		return err;
@@ -1214,6 +1216,8 @@ static int mount_ubifs(struct ubifs_info *c)
 	if (err)
 		goto out_free;
 
+	c->probing = 0;
+
 	/*
 	 * Make sure the compressor which is set as default in the superblock
 	 * or overridden by mount options is actually compiled in.
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index e8c8cfe..145dc6d 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -52,6 +52,16 @@
 	pr_warn("UBIFS warning (pid %d): %s: " fmt "\n",            \
 		current->pid, __func__, ##__VA_ARGS__)
 
+/*
+ * A variant of 'ubifs_err()' which takes the UBIFS file-sytem description
+ * object as an argument.
+ */
+#define ubifs_errc(c, fmt, ...)                                    \
+	do {                                                       \
+		if (!(c)->probing)                                 \
+			ubifs_err(fmt, ##__VA_ARGS__);             \
+	} while (0)
+
 /* UBIFS file system VFS magic number */
 #define UBIFS_SUPER_MAGIC 0x24051905
 
@@ -1441,6 +1451,7 @@ struct ubifs_info {
 	unsigned int replaying:1;
 	unsigned int mounting:1;
 	unsigned int remounting_rw:1;
+	unsigned int probing:1;
 	struct list_head replay_list;
 	struct list_head replay_buds;
 	unsigned long long cs_sqnum;
-- 
1.9.3


[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

* Re: [PATCH v5] ubifs: respect MS_SILENT mount flag
  2014-05-31  0:01                     ` [PATCH v5] " Daniel Golle
@ 2014-06-02  7:59                       ` Artem Bityutskiy
  2014-06-02 13:51                         ` [PATCH v6] " Daniel Golle
  0 siblings, 1 reply; 19+ messages in thread
From: Artem Bityutskiy @ 2014-06-02  7:59 UTC (permalink / raw)
  To: Daniel Golle; +Cc: computersforpeace, linux-mtd, hujianyang

Thanks Daniel, this looks nicer than the original patch. Would you
please do few more cosmetic changes, though.

On Sat, 2014-05-31 at 02:01 +0200, Daniel Golle wrote: 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index a81c7b5..e88bbb6 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1149,6 +1149,8 @@ static int mount_ubifs(struct ubifs_info *c)
>  	size_t sz;
>  
>  	c->ro_mount = !!(c->vfs_sb->s_flags & MS_RDONLY);
> +	c->probing = !!(c->vfs_sb->s_flags & MS_SILENT);

Would you add a comment on top of this saying what MS_SILENT is about.
The kernel people who read the code do not necessary know the "FS
probing" use-case. E.g., I did not know about it.
> @@ -1441,6 +1451,7 @@ struct ubifs_info {
>  	unsigned int replaying:1;
>  	unsigned int mounting:1;
>  	unsigned int remounting_rw:1;
> +	unsigned int probing:1;

We have kerneldoc-style comment on top of this structure, could you
please document the new field there.

-- 
Best Regards,
Artem Bityutskiy

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

* [PATCH v6] ubifs: respect MS_SILENT mount flag
  2014-06-02  7:59                       ` Artem Bityutskiy
@ 2014-06-02 13:51                         ` Daniel Golle
  2014-06-02 15:01                           ` Artem Bityutskiy
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Golle @ 2014-06-02 13:51 UTC (permalink / raw)
  To: dedekind1, hujianyang, computersforpeace, linux-mtd

[-- Attachment #1: Type: text/plain, Size: 4094 bytes --]

When attempting to mount a non-ubifs formatted volume, lots of error
messages (including a stack dump) are thrown to the kernel log even if
the MS_SILENT mount flag is set.
Fix this by introducing adding an additional state-variable in
struct ubifs_info and suppress error messages in ubifs_read_node if
MS_SILENT is set.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v6: added comments
v5: added missing semi-colon and fixed a style issue
v4: enclose conditional error printing macro by do { } while (0)
v3: use state variable in ubifs_info instead of a function parameter

 fs/ubifs/io.c    | 16 +++++++++-------
 fs/ubifs/super.c |  5 +++++
 fs/ubifs/ubifs.h | 12 ++++++++++++
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index e18b988..3205929 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -988,30 +988,32 @@ int ubifs_read_node(const struct ubifs_info *c, void *buf, int type, int len,
 		return err;
 
 	if (type != ch->node_type) {
-		ubifs_err("bad node type (%d but expected %d)",
+		ubifs_errc(c, "bad node type (%d but expected %d)",
 			  ch->node_type, type);
 		goto out;
 	}
 
 	err = ubifs_check_node(c, buf, lnum, offs, 0, 0);
 	if (err) {
-		ubifs_err("expected node type %d", type);
+		ubifs_errc(c, "expected node type %d", type);
 		return err;
 	}
 
 	l = le32_to_cpu(ch->len);
 	if (l != len) {
-		ubifs_err("bad node length %d, expected %d", l, len);
+		ubifs_errc(c, "bad node length %d, expected %d", l, len);
 		goto out;
 	}
 
 	return 0;
 
 out:
-	ubifs_err("bad node at LEB %d:%d, LEB mapping status %d", lnum, offs,
-		  ubi_is_mapped(c->ubi, lnum));
-	ubifs_dump_node(c, buf);
-	dump_stack();
+	ubifs_errc(c, "bad node at LEB %d:%d, LEB mapping status %d", lnum,
+		  offs, ubi_is_mapped(c->ubi, lnum));
+	if (!c->probing) {
+		ubifs_dump_node(c, buf);
+		dump_stack();
+	}
 	return -EINVAL;
 }
 
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index a81c7b5..3904c85 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1149,6 +1149,9 @@ static int mount_ubifs(struct ubifs_info *c)
 	size_t sz;
 
 	c->ro_mount = !!(c->vfs_sb->s_flags & MS_RDONLY);
+	/* Suppress error messages while probing if MS_SILENT is set */
+	c->probing = !!(c->vfs_sb->s_flags & MS_SILENT);
+
 	err = init_constants_early(c);
 	if (err)
 		return err;
@@ -1214,6 +1217,8 @@ static int mount_ubifs(struct ubifs_info *c)
 	if (err)
 		goto out_free;
 
+	c->probing = 0;
+
 	/*
 	 * Make sure the compressor which is set as default in the superblock
 	 * or overridden by mount options is actually compiled in.
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index e8c8cfe..f87ab74 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -52,6 +52,16 @@
 	pr_warn("UBIFS warning (pid %d): %s: " fmt "\n",            \
 		current->pid, __func__, ##__VA_ARGS__)
 
+/*
+ * A variant of 'ubifs_err()' which takes the UBIFS file-sytem description
+ * object as an argument.
+ */
+#define ubifs_errc(c, fmt, ...)                                    \
+	do {                                                       \
+		if (!(c)->probing)                                 \
+			ubifs_err(fmt, ##__VA_ARGS__);             \
+	} while (0)
+
 /* UBIFS file system VFS magic number */
 #define UBIFS_SUPER_MAGIC 0x24051905
 
@@ -1209,6 +1219,7 @@ struct ubifs_debug_info;
  * @need_recovery: %1 if the file-system needs recovery
  * @replaying: %1 during journal replay
  * @mounting: %1 while mounting
+ * @probing: %1 while attempting to mount if MS_SILENT mount flag is set
  * @remounting_rw: %1 while re-mounting from R/O mode to R/W mode
  * @replay_list: temporary list used during journal replay
  * @replay_buds: list of buds to replay
@@ -1441,6 +1452,7 @@ struct ubifs_info {
 	unsigned int replaying:1;
 	unsigned int mounting:1;
 	unsigned int remounting_rw:1;
+	unsigned int probing:1;
 	struct list_head replay_list;
 	struct list_head replay_buds;
 	unsigned long long cs_sqnum;
-- 
1.9.3


[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

* Re: [PATCH v6] ubifs: respect MS_SILENT mount flag
  2014-06-02 13:51                         ` [PATCH v6] " Daniel Golle
@ 2014-06-02 15:01                           ` Artem Bityutskiy
  0 siblings, 0 replies; 19+ messages in thread
From: Artem Bityutskiy @ 2014-06-02 15:01 UTC (permalink / raw)
  To: Daniel Golle; +Cc: computersforpeace, linux-mtd, hujianyang

On Mon, 2014-06-02 at 15:51 +0200, Daniel Golle wrote:
> When attempting to mount a non-ubifs formatted volume, lots of error
> messages (including a stack dump) are thrown to the kernel log even if
> the MS_SILENT mount flag is set.
> Fix this by introducing adding an additional state-variable in
> struct ubifs_info and suppress error messages in ubifs_read_node if
> MS_SILENT is set.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>

Pushed to linux-ubifs.git with a couple of minor amendments.

Thanks!

-- 
Best Regards,
Artem Bityutskiy

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

end of thread, other threads:[~2014-06-02 15:02 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-17  1:55 [PATCH] ubifs: respect MS_SILENT mount flag Daniel Golle
2014-05-27 12:18 ` Artem Bityutskiy
2014-05-27 14:11   ` [PATCH v2] " Daniel Golle
2014-05-27 14:56     ` Artem Bityutskiy
2014-05-27 16:04       ` Daniel
2014-05-28  2:11         ` hujianyang
2014-05-28  8:01           ` Artem Bityutskiy
2014-05-28  8:07             ` Bityutskiy, Artem
2014-05-28  8:14               ` Artem Bityutskiy
2014-05-28  8:28                 ` Artem Bityutskiy
2014-05-28  8:42                   ` hujianyang
2014-05-28  9:29                     ` Artem Bityutskiy
2014-05-30 22:01                 ` [PATCH v3] " Daniel Golle
2014-05-30 23:20                   ` Brian Norris
2014-05-30 23:32                     ` [PATCH v4] " Daniel Golle
2014-05-31  0:01                     ` [PATCH v5] " Daniel Golle
2014-06-02  7:59                       ` Artem Bityutskiy
2014-06-02 13:51                         ` [PATCH v6] " Daniel Golle
2014-06-02 15:01                           ` Artem Bityutskiy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).