From 5ffef88ffeb730e1bf2da56a39a55e03d57a66c9 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Wed, 4 Aug 2010 10:14:47 +0300 Subject: [PATCH 01/20] UBIFS: switch to RO mode after synchronizing In 'ubifs_garbage_collect()' on error path, we first switch to R/O mode, and then synchronize write-buffers (to make sure no data are lost). But the GC write-buffer synchronization will fail, because we are already in R/O mode. This patch re-orders this and makes sure we first synchronize the write-buffer, and then switch to R/O mode. Signed-off-by: Artem Bityutskiy --- fs/ubifs/gc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index 918d1582ca05..f89a422ca395 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -774,8 +774,8 @@ out_unlock: out: ubifs_assert(ret < 0); ubifs_assert(ret != -ENOSPC && ret != -EAGAIN); - ubifs_ro_mode(c, ret); ubifs_wbuf_sync_nolock(wbuf); + ubifs_ro_mode(c, ret); mutex_unlock(&wbuf->io_mutex); ubifs_return_leb(c, lp.lnum); return ret; From efe1881f5482f94f5e5e6cb74bf3ea72f2b5b9ce Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Wed, 4 Aug 2010 14:06:06 +0300 Subject: [PATCH 02/20] UBIFS: do not treat ENOSPC specially 'ubifs_garbage_collect_leb()' should never return '-ENOSPC', and if it does, this is an error. Thus, do not treat this error code specially. '-EAGAIN' is a special error code, but not '-ENOSPC'. Signed-off-by: Artem Bityutskiy --- fs/ubifs/gc.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index f89a422ca395..ee4b05de4d48 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -677,14 +677,12 @@ int ubifs_garbage_collect(struct ubifs_info *c, int anyway) ret = ubifs_garbage_collect_leb(c, &lp); if (ret < 0) { - if (ret == -EAGAIN || ret == -ENOSPC) { + if (ret == -EAGAIN) { /* - * These codes are not errors, so we have to - * return the LEB to lprops. But if the - * 'ubifs_return_leb()' function fails, its - * failure code is propagated to the caller - * instead of the original '-EAGAIN' or - * '-ENOSPC'. + * This is not error, so we have to return the + * LEB to lprops. But if 'ubifs_return_leb()' + * fails, its failure code is propagated to the + * caller instead of the original '-EAGAIN'. */ err = ubifs_return_leb(c, lp.lnum); if (err) From e3408ad4cbed6ec6990efad4c2ef0856bcd3c712 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Wed, 4 Aug 2010 13:50:51 +0300 Subject: [PATCH 03/20] UBIFS: fix assertion warning This patch fixes the following false assertion warning: UBIFS assert failed in data_nodes_cmp at 130 (pid 15107) The assertion was wrong because it did not take into account that the node can be an xentry. Signed-off-by: Artem Bityutskiy --- fs/ubifs/gc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index ee4b05de4d48..98b2c3c8725a 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -178,7 +178,8 @@ int nondata_nodes_cmp(void *priv, struct list_head *a, struct list_head *b) if (typeb == UBIFS_INO_KEY) return 1; - ubifs_assert(typea == UBIFS_DENT_KEY && typeb == UBIFS_DENT_KEY); + ubifs_assert(typea == UBIFS_DENT_KEY || typea == UBIFS_XENT_KEY); + ubifs_assert(typeb == UBIFS_DENT_KEY || typeb == UBIFS_XENT_KEY); inuma = key_inum(c, &sa->key); inumb = key_inum(c, &sb->key); From 44ec83b8bd05d323998031f141c310127721acae Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sat, 7 Aug 2010 08:44:13 +0300 Subject: [PATCH 04/20] UBIFS: do not look up truncation nodes When moving nodes in GC, do not try to look up truncation nodes in TNC, because they do not exist there. This would be harmless, because the TNC look-up would fail, if we did not have bug 'ubifs_add_snod()' which reads garbage into 'snod->key'. But in any case, it is less error prone to explicitly ignore everything but inode, data, dentry and xentry nodes. Signed-off-by: Artem Bityutskiy --- fs/ubifs/gc.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index 98b2c3c8725a..9dbbc5c88940 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -233,9 +233,26 @@ static int sort_nodes(struct ubifs_info *c, struct ubifs_scan_leb *sleb, list_for_each_entry_safe(snod, tmp, &sleb->nodes, list) { int err; - ubifs_assert(snod->type != UBIFS_IDX_NODE); - ubifs_assert(snod->type != UBIFS_REF_NODE); - ubifs_assert(snod->type != UBIFS_CS_NODE); + ubifs_assert(snod->type == UBIFS_INO_NODE || + snod->type == UBIFS_DATA_NODE || + snod->type == UBIFS_DENT_NODE || + snod->type == UBIFS_XENT_NODE || + snod->type == UBIFS_TRUN_NODE); + + if (snod->type != UBIFS_INO_NODE && + snod->type != UBIFS_DATA_NODE && + snod->type != UBIFS_DENT_NODE && + snod->type != UBIFS_XENT_NODE) { + /* Probably truncation node, zap it */ + list_del(&snod->list); + kfree(snod); + continue; + } + + ubifs_assert(key_type(c, &snod->key) == UBIFS_DATA_KEY || + key_type(c, &snod->key) == UBIFS_INO_KEY || + key_type(c, &snod->key) == UBIFS_DENT_KEY || + key_type(c, &snod->key) == UBIFS_XENT_KEY); err = ubifs_tnc_has_node(c, &snod->key, 0, sleb->lnum, snod->offs, 0); From ab87118d717467cbcd9648692c2a9708d55193bc Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 8 Aug 2010 12:25:33 +0300 Subject: [PATCH 05/20] UBIFS: do not use key type in list_sort In comparison function for 'list_sort()' we use key type to distinguish between node types. However, we have a bit simper way to detect node type - 'snod->type'. This more logical to use, comparing to decoding key types. Also allows to get rid of 2 local variables. Signed-off-by: Artem Bityutskiy --- fs/ubifs/gc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index 9dbbc5c88940..27815bb91125 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -157,7 +157,6 @@ int data_nodes_cmp(void *priv, struct list_head *a, struct list_head *b) */ int nondata_nodes_cmp(void *priv, struct list_head *a, struct list_head *b) { - int typea, typeb; ino_t inuma, inumb; struct ubifs_info *c = priv; struct ubifs_scan_node *sa, *sb; @@ -165,21 +164,22 @@ int nondata_nodes_cmp(void *priv, struct list_head *a, struct list_head *b) cond_resched(); sa = list_entry(a, struct ubifs_scan_node, list); sb = list_entry(b, struct ubifs_scan_node, list); - typea = key_type(c, &sa->key); - typeb = key_type(c, &sb->key); - ubifs_assert(typea != UBIFS_DATA_KEY && typeb != UBIFS_DATA_KEY); + ubifs_assert(sa->type != UBIFS_DATA_NODE && + sb->type != UBIFS_DATA_NODE); /* Inodes go before directory entries */ - if (typea == UBIFS_INO_KEY) { - if (typeb == UBIFS_INO_KEY) + if (sa->type == UBIFS_INO_NODE) { + if (sb->type == UBIFS_INO_NODE) return sb->len - sa->len; return -1; } - if (typeb == UBIFS_INO_KEY) + if (sb->type == UBIFS_INO_NODE) return 1; - ubifs_assert(typea == UBIFS_DENT_KEY || typea == UBIFS_XENT_KEY); - ubifs_assert(typeb == UBIFS_DENT_KEY || typeb == UBIFS_XENT_KEY); + ubifs_assert(sa->type == UBIFS_DENT_NODE || + sa->type == UBIFS_XENT_NODE); + ubifs_assert(sb->type == UBIFS_DENT_NODE || + sb->type == UBIFS_XENT_NODE); inuma = key_inum(c, &sa->key); inumb = key_inum(c, &sb->key); From 66576833f5396af34c52160b16d7b8573199282a Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 8 Aug 2010 12:29:58 +0300 Subject: [PATCH 06/20] UBIFS: improve assertion in node comparison functions Improve assertions in gc.c in the comparison functions for 'list_sort()': check key types _and_ node types. Signed-off-by: Artem Bityutskiy --- fs/ubifs/gc.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index 27815bb91125..dafef3d195d3 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -127,8 +127,11 @@ int data_nodes_cmp(void *priv, struct list_head *a, struct list_head *b) cond_resched(); sa = list_entry(a, struct ubifs_scan_node, list); sb = list_entry(b, struct ubifs_scan_node, list); + ubifs_assert(key_type(c, &sa->key) == UBIFS_DATA_KEY); ubifs_assert(key_type(c, &sb->key) == UBIFS_DATA_KEY); + ubifs_assert(sa->type == UBIFS_DATA_NODE); + ubifs_assert(sb->type == UBIFS_DATA_NODE); inuma = key_inum(c, &sa->key); inumb = key_inum(c, &sb->key); @@ -164,6 +167,9 @@ int nondata_nodes_cmp(void *priv, struct list_head *a, struct list_head *b) cond_resched(); sa = list_entry(a, struct ubifs_scan_node, list); sb = list_entry(b, struct ubifs_scan_node, list); + + ubifs_assert(key_type(c, &sa->key) != UBIFS_DATA_KEY && + key_type(c, &sb->key) != UBIFS_DATA_KEY); ubifs_assert(sa->type != UBIFS_DATA_NODE && sb->type != UBIFS_DATA_NODE); @@ -176,10 +182,15 @@ int nondata_nodes_cmp(void *priv, struct list_head *a, struct list_head *b) if (sb->type == UBIFS_INO_NODE) return 1; + ubifs_assert(key_type(c, &sa->key) == UBIFS_DENT_KEY || + key_type(c, &sa->key) == UBIFS_XENT_KEY); + ubifs_assert(key_type(c, &sb->key) == UBIFS_DENT_KEY || + key_type(c, &sb->key) == UBIFS_XENT_KEY); ubifs_assert(sa->type == UBIFS_DENT_NODE || sa->type == UBIFS_XENT_NODE); ubifs_assert(sb->type == UBIFS_DENT_NODE || sb->type == UBIFS_XENT_NODE); + inuma = key_inum(c, &sa->key); inumb = key_inum(c, &sb->key); From 5b7a3a2e1b0cbc7d5410a8da60dac266a3e19268 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 8 Aug 2010 12:32:00 +0300 Subject: [PATCH 07/20] UBIFS: do not write rubbish into truncation scanning node In the scanning code, in 'ubifs_add_snod()', we write rubbish into 'snod->key', because we assume that on-flash truncation nodes have a key, but they do not. If the other parts of UBIFS then mistakenly try to look-up the truncation node key (they should not do this, but may do because of a bug), we can succeed and corrupt TNC. It looks like we did have such a situation in 'sort_nodes()' in gc.c. Signed-off-by: Artem Bityutskiy --- fs/ubifs/scan.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/ubifs/scan.c b/fs/ubifs/scan.c index 96c525384191..a0a305ca61af 100644 --- a/fs/ubifs/scan.c +++ b/fs/ubifs/scan.c @@ -212,7 +212,6 @@ int ubifs_add_snod(const struct ubifs_info *c, struct ubifs_scan_leb *sleb, case UBIFS_DENT_NODE: case UBIFS_XENT_NODE: case UBIFS_DATA_NODE: - case UBIFS_TRUN_NODE: /* * The key is in the same place in all keyed * nodes. From ba2f48f70efcf4d82deafb2be327ed64b1f043a5 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 22 Aug 2010 07:10:12 +0300 Subject: [PATCH 08/20] UBIFS: mark unused key objects as invalid When scanning the flash, UBIFS builds a list of flash nodes of type 'struct ubifs_scan_node'. Each scanned node has a 'snod->key' field. This field is valid for most of the nodes, but invalid for some node type, e.g., truncation nodes. It is safer to explicitly initialize such keys to something invalid, rather than leaving them initialized to all zeros, which has key type of UBIFS_INO_KEY. This patch introduces new "fake" key type UBIFS_INVALID_KEY and initializes unused 'snod->key' objects to this type. It also adds debugging assertions in the TNC code to make sure no one ever tries to look these nodes up in the TNC. Signed-off-by: Artem Bityutskiy --- fs/ubifs/key.h | 14 ++++++++++++++ fs/ubifs/scan.c | 5 ++++- fs/ubifs/tnc.c | 5 ++++- fs/ubifs/ubifs.h | 6 +++++- 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/fs/ubifs/key.h b/fs/ubifs/key.h index 0f530c684f0b..92a8491a8f8c 100644 --- a/fs/ubifs/key.h +++ b/fs/ubifs/key.h @@ -305,6 +305,20 @@ static inline void trun_key_init(const struct ubifs_info *c, key->u32[1] = UBIFS_TRUN_KEY << UBIFS_S_KEY_BLOCK_BITS; } +/** + * invalid_key_init - initialize invalid node key. + * @c: UBIFS file-system description object + * @key: key to initialize + * + * This is a helper function which marks a @key object as invalid. + */ +static inline void invalid_key_init(const struct ubifs_info *c, + union ubifs_key *key) +{ + key->u32[0] = 0xDEADBEAF; + key->u32[1] = UBIFS_INVALID_KEY; +} + /** * key_type - get key type. * @c: UBIFS file-system description object diff --git a/fs/ubifs/scan.c b/fs/ubifs/scan.c index a0a305ca61af..3e1ee57dbeaa 100644 --- a/fs/ubifs/scan.c +++ b/fs/ubifs/scan.c @@ -197,7 +197,7 @@ int ubifs_add_snod(const struct ubifs_info *c, struct ubifs_scan_leb *sleb, struct ubifs_ino_node *ino = buf; struct ubifs_scan_node *snod; - snod = kzalloc(sizeof(struct ubifs_scan_node), GFP_NOFS); + snod = kmalloc(sizeof(struct ubifs_scan_node), GFP_NOFS); if (!snod) return -ENOMEM; @@ -218,6 +218,9 @@ int ubifs_add_snod(const struct ubifs_info *c, struct ubifs_scan_leb *sleb, */ key_read(c, &ino->key, &snod->key); break; + default: + invalid_key_init(c, &snod->key); + break; } list_add_tail(&snod->list, &sleb->nodes); sleb->nodes_cnt += 1; diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c index 2194915220e5..ad9cf0133622 100644 --- a/fs/ubifs/tnc.c +++ b/fs/ubifs/tnc.c @@ -1177,6 +1177,7 @@ int ubifs_lookup_level0(struct ubifs_info *c, const union ubifs_key *key, unsigned long time = get_seconds(); dbg_tnc("search key %s", DBGKEY(key)); + ubifs_assert(key_type(c, key) < UBIFS_INVALID_KEY); znode = c->zroot.znode; if (unlikely(!znode)) { @@ -2966,7 +2967,7 @@ static struct ubifs_znode *right_znode(struct ubifs_info *c, * * This function searches an indexing node by its first key @key and its * address @lnum:@offs. It looks up the indexing tree by pulling all indexing - * nodes it traverses to TNC. This function is called fro indexing nodes which + * nodes it traverses to TNC. This function is called for indexing nodes which * were found on the media by scanning, for example when garbage-collecting or * when doing in-the-gaps commit. This means that the indexing node which is * looked for does not have to have exactly the same leftmost key @key, because @@ -2988,6 +2989,8 @@ static struct ubifs_znode *lookup_znode(struct ubifs_info *c, struct ubifs_znode *znode, *zn; int n, nn; + ubifs_assert(key_type(c, key) < UBIFS_INVALID_KEY); + /* * The arguments have probably been read off flash, so don't assume * they are valid. diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 0c9876b396dd..c4dc9b18f73e 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -119,8 +119,12 @@ * in TNC. However, when replaying, it is handy to introduce fake "truncation" * keys for truncation nodes because the code becomes simpler. So we define * %UBIFS_TRUN_KEY type. + * + * But otherwise, out of the journal reply scope, the truncation keys are + * invalid. */ -#define UBIFS_TRUN_KEY UBIFS_KEY_TYPES_CNT +#define UBIFS_TRUN_KEY UBIFS_KEY_TYPES_CNT +#define UBIFS_INVALID_KEY UBIFS_KEY_TYPES_CNT /* * How much a directory entry/extended attribute entry adds to the parent/host From 1a9476a77083354005750c9df45ba9d71ad12c8c Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 8 Aug 2010 12:45:23 +0300 Subject: [PATCH 09/20] UBIFS: fix assertion warnings in comparison function When running the integrity test ('integck' from mtd-utils) on current UBIFS on 2.6.35, I see that assertions in UBIFS 'list_sort()' comparison functions trigger sometimes, e.g.: UBIFS assert failed in data_nodes_cmp at 132 (pid 28311) My investigation showed that this happens when 'list_sort()' calls the 'cmp()' function with equivalent arguments. In this case, the 'struct list_head' parameter, passed to 'cmp()' is bogus, and it does not belong to any element in the original list. And this issue seems to be introduced by commit: commit 835cc0c8477fdbc59e0217891d6f11061b1ac4e2 Author: Don Mullis Date: Fri Mar 5 13:43:15 2010 -0800 It is easy to work around the issue by doing: if (a == b) return 0; in UBIFS. It works, but 'lib_sort()' should nevertheless be fixed. Although it is harmless to have this piece of code in UBIFS. This patch adds that code to both UBIFS 'cmp()' functions: 'data_nodes_cmp()' and 'nondata_nodes_cmp()'. Signed-off-by: Artem Bityutskiy --- fs/ubifs/gc.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index dafef3d195d3..4fc31ca0e3b1 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -125,6 +125,9 @@ int data_nodes_cmp(void *priv, struct list_head *a, struct list_head *b) struct ubifs_scan_node *sa, *sb; cond_resched(); + if (a == b) + return 0; + sa = list_entry(a, struct ubifs_scan_node, list); sb = list_entry(b, struct ubifs_scan_node, list); @@ -165,6 +168,9 @@ int nondata_nodes_cmp(void *priv, struct list_head *a, struct list_head *b) struct ubifs_scan_node *sa, *sb; cond_resched(); + if (a == b) + return 0; + sa = list_entry(a, struct ubifs_scan_node, list); sb = list_entry(b, struct ubifs_scan_node, list); From 3bb66b47a4268a4419594b4c4aec58dbeb6b58d2 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sat, 7 Aug 2010 10:06:11 +0300 Subject: [PATCH 10/20] UBIFS: introduce list sorting debugging checks The UBIFS bug in the GC list sorting comparison functions inspired me to write internal debugging check functions which verify that the list of nodes is sorted properly. So, this patch implements 2 new debugging functions: o 'dbg_check_data_nodes_order()' - check order of data nodes list o 'dbg_check_nondata_nodes_order()' - check order of non-data nodes list The debugging functions are executed only if general UBIFS debugging checks are enabled. And they are compiled out if UBIFS debugging is disabled. Signed-off-by: Artem Bityutskiy --- fs/ubifs/debug.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++ fs/ubifs/debug.h | 4 ++ fs/ubifs/gc.c | 10 ++- 3 files changed, 168 insertions(+), 2 deletions(-) diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c index c2a68baa782f..c664f10aec6c 100644 --- a/fs/ubifs/debug.c +++ b/fs/ubifs/debug.c @@ -2239,6 +2239,162 @@ out_free: return err; } +/** + * dbg_check_data_nodes_order - check that list of data nodes is sorted. + * @c: UBIFS file-system description object + * @head: the list of nodes ('struct ubifs_scan_node' objects) + * + * This function returns zero if the list of data nodes is sorted correctly, + * and %-EINVAL if not. + */ +int dbg_check_data_nodes_order(struct ubifs_info *c, struct list_head *head) +{ + struct list_head *cur; + struct ubifs_scan_node *sa, *sb; + + if (!(ubifs_chk_flags & UBIFS_CHK_GEN)) + return 0; + + for (cur = head->next; cur->next != head; cur = cur->next) { + ino_t inuma, inumb; + uint32_t blka, blkb; + + cond_resched(); + sa = container_of(cur, struct ubifs_scan_node, list); + sb = container_of(cur->next, struct ubifs_scan_node, list); + + if (sa->type != UBIFS_DATA_NODE) { + ubifs_err("bad node type %d", sa->type); + dbg_dump_node(c, sa->node); + return -EINVAL; + } + if (sb->type != UBIFS_DATA_NODE) { + ubifs_err("bad node type %d", sb->type); + dbg_dump_node(c, sb->node); + return -EINVAL; + } + + inuma = key_inum(c, &sa->key); + inumb = key_inum(c, &sb->key); + + if (inuma < inumb) + continue; + if (inuma > inumb) { + ubifs_err("larger inum %lu goes before inum %lu", + (unsigned long)inuma, (unsigned long)inumb); + goto error_dump; + } + + blka = key_block(c, &sa->key); + blkb = key_block(c, &sb->key); + + if (blka > blkb) { + ubifs_err("larger block %u goes before %u", blka, blkb); + goto error_dump; + } + if (blka == blkb) { + ubifs_err("two data nodes for the same block"); + goto error_dump; + } + } + + return 0; + +error_dump: + dbg_dump_node(c, sa->node); + dbg_dump_node(c, sb->node); + return -EINVAL; +} + +/** + * dbg_check_nondata_nodes_order - check that list of data nodes is sorted. + * @c: UBIFS file-system description object + * @head: the list of nodes ('struct ubifs_scan_node' objects) + * + * This function returns zero if the list of non-data nodes is sorted correctly, + * and %-EINVAL if not. + */ +int dbg_check_nondata_nodes_order(struct ubifs_info *c, struct list_head *head) +{ + struct list_head *cur; + struct ubifs_scan_node *sa, *sb; + + if (!(ubifs_chk_flags & UBIFS_CHK_GEN)) + return 0; + + for (cur = head->next; cur->next != head; cur = cur->next) { + ino_t inuma, inumb; + uint32_t hasha, hashb; + + cond_resched(); + sa = container_of(cur, struct ubifs_scan_node, list); + sb = container_of(cur->next, struct ubifs_scan_node, list); + + if (sa->type != UBIFS_INO_NODE && sa->type != UBIFS_DENT_NODE && + sa->type != UBIFS_XENT_NODE) { + ubifs_err("bad node type %d", sa->type); + dbg_dump_node(c, sa->node); + return -EINVAL; + } + if (sa->type != UBIFS_INO_NODE && sa->type != UBIFS_DENT_NODE && + sa->type != UBIFS_XENT_NODE) { + ubifs_err("bad node type %d", sb->type); + dbg_dump_node(c, sb->node); + return -EINVAL; + } + + if (sa->type != UBIFS_INO_NODE && sb->type == UBIFS_INO_NODE) { + ubifs_err("non-inode node goes before inode node"); + goto error_dump; + } + + if (sa->type == UBIFS_INO_NODE && sb->type != UBIFS_INO_NODE) + continue; + + if (sa->type == UBIFS_INO_NODE && sb->type == UBIFS_INO_NODE) { + /* Inode nodes are sorted in descending size order */ + if (sa->len < sb->len) { + ubifs_err("smaller inode node goes first"); + goto error_dump; + } + continue; + } + + /* + * This is either a dentry or xentry, which should be sorted in + * ascending (parent ino, hash) order. + */ + inuma = key_inum(c, &sa->key); + inumb = key_inum(c, &sb->key); + + if (inuma < inumb) + continue; + if (inuma > inumb) { + ubifs_err("larger inum %lu goes before inum %lu", + (unsigned long)inuma, (unsigned long)inumb); + goto error_dump; + } + + hasha = key_block(c, &sa->key); + hashb = key_block(c, &sb->key); + + if (hasha > hashb) { + ubifs_err("larger hash %u goes before %u", hasha, hashb); + goto error_dump; + } + } + + return 0; + +error_dump: + ubifs_msg("dumping first node"); + dbg_dump_node(c, sa->node); + ubifs_msg("dumping second node"); + dbg_dump_node(c, sb->node); + return -EINVAL; + return 0; +} + static int invocation_cnt; int dbg_force_in_the_gaps(void) diff --git a/fs/ubifs/debug.h b/fs/ubifs/debug.h index 29d960101ea6..69ebe4729151 100644 --- a/fs/ubifs/debug.h +++ b/fs/ubifs/debug.h @@ -324,6 +324,8 @@ int dbg_check_lpt_nodes(struct ubifs_info *c, struct ubifs_cnode *cnode, int row, int col); int dbg_check_inode_size(struct ubifs_info *c, const struct inode *inode, loff_t size); +int dbg_check_data_nodes_order(struct ubifs_info *c, struct list_head *head); +int dbg_check_nondata_nodes_order(struct ubifs_info *c, struct list_head *head); /* Force the use of in-the-gaps method for testing */ @@ -465,6 +467,8 @@ void dbg_debugfs_exit_fs(struct ubifs_info *c); #define dbg_check_lprops(c) 0 #define dbg_check_lpt_nodes(c, cnode, row, col) 0 #define dbg_check_inode_size(c, inode, size) 0 +#define dbg_check_data_nodes_order(c, head) 0 +#define dbg_check_nondata_nodes_order(c, head) 0 #define dbg_force_in_the_gaps_enabled 0 #define dbg_force_in_the_gaps() 0 #define dbg_failure_mode 0 diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index 4fc31ca0e3b1..396f24a30af9 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -242,14 +242,13 @@ int nondata_nodes_cmp(void *priv, struct list_head *a, struct list_head *b) static int sort_nodes(struct ubifs_info *c, struct ubifs_scan_leb *sleb, struct list_head *nondata, int *min) { + int err; struct ubifs_scan_node *snod, *tmp; *min = INT_MAX; /* Separate data nodes and non-data nodes */ list_for_each_entry_safe(snod, tmp, &sleb->nodes, list) { - int err; - ubifs_assert(snod->type == UBIFS_INO_NODE || snod->type == UBIFS_DATA_NODE || snod->type == UBIFS_DENT_NODE || @@ -293,6 +292,13 @@ static int sort_nodes(struct ubifs_info *c, struct ubifs_scan_leb *sleb, /* Sort data and non-data nodes */ list_sort(c, &sleb->nodes, &data_nodes_cmp); list_sort(c, nondata, &nondata_nodes_cmp); + + err = dbg_check_data_nodes_order(c, &sleb->nodes); + if (err) + return err; + err = dbg_check_nondata_nodes_order(c, nondata); + if (err) + return err; return 0; } From 3a8fa0ed3e1a8dc54f9297d2a2292b87e03e21b5 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 22 Aug 2010 21:27:30 +0300 Subject: [PATCH 11/20] UBIFS: improve error reporting when reading bad node When an error happens during validation of read node, the typical situation is that the LEB we read is unmapped (due to some bug). It is handy to include the mapping status into the error message. Signed-off-by: Artem Bityutskiy --- fs/ubifs/io.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c index bcf5a16f30bb..9432431bf595 100644 --- a/fs/ubifs/io.c +++ b/fs/ubifs/io.c @@ -815,7 +815,8 @@ int ubifs_read_node(const struct ubifs_info *c, void *buf, int type, int len, return 0; out: - ubifs_err("bad node at LEB %d:%d", lnum, offs); + ubifs_err("bad node at LEB %d:%d, LEB mapping status %d", lnum, offs, + ubi_is_mapped(c->ubi, lnum)); dbg_dump_node(c, buf); dbg_dump_stack(); return -EINVAL; From 0e54c8992a35ab8126e8a7661007ddc1745a2597 Mon Sep 17 00:00:00 2001 From: Vasiliy Kulikov Date: Sun, 5 Sep 2010 22:33:00 +0400 Subject: [PATCH 12/20] UBIFS: check return code of ubifs_lpt_lookup Function ubifs_lpt_lookup may return ERR_PTR(...). Check for it. [Tweaked by Artem Bityutskiy] Signed-off-by: Vasiliy Kulikov Signed-off-by: Artem Bityutskiy --- fs/ubifs/lpt.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/ubifs/lpt.c b/fs/ubifs/lpt.c index 0084a33c4c69..72775d35b99e 100644 --- a/fs/ubifs/lpt.c +++ b/fs/ubifs/lpt.c @@ -1363,6 +1363,7 @@ static int read_lsave(struct ubifs_info *c) goto out; for (i = 0; i < c->lsave_cnt; i++) { int lnum = c->lsave[i]; + struct ubifs_lprops *lprops; /* * Due to automatic resizing, the values in the lsave table @@ -1370,7 +1371,11 @@ static int read_lsave(struct ubifs_info *c) */ if (lnum >= c->leb_cnt) continue; - ubifs_lpt_lookup(c, lnum); + lprops = ubifs_lpt_lookup(c, lnum); + if (IS_ERR(lprops)) { + err = PTR_ERR(lprops); + goto out; + } } out: vfree(buf); From 8c893a5545ca772744376295690723dcb0b47d96 Mon Sep 17 00:00:00 2001 From: Vasiliy Kulikov Date: Sun, 5 Sep 2010 22:33:04 +0400 Subject: [PATCH 13/20] UBIFS: check return code of pnode_lookup Function pnode_lookup may return ERR_PTR(...). Check for it. Signed-off-by: Vasiliy Kulikov Signed-off-by: Artem Bityutskiy --- fs/ubifs/lpt_commit.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/ubifs/lpt_commit.c b/fs/ubifs/lpt_commit.c index d12535b7fc78..5c90dec5db0b 100644 --- a/fs/ubifs/lpt_commit.c +++ b/fs/ubifs/lpt_commit.c @@ -705,6 +705,9 @@ static int make_tree_dirty(struct ubifs_info *c) struct ubifs_pnode *pnode; pnode = pnode_lookup(c, 0); + if (IS_ERR(pnode)) + return PTR_ERR(pnode); + while (pnode) { do_make_pnode_dirty(c, pnode); pnode = next_pnode_to_dirty(c, pnode); From 2680d722bf2c5f75225dd9acb3ec9e5a9e2652f4 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Fri, 17 Sep 2010 16:44:28 +0300 Subject: [PATCH 14/20] UBIFS: introduce new flag for RO due to errors The R/O state may have various reasons: 1. The UBI volume is R/O 2. The FS is mounted R/O 3. The FS switched to R/O mode because of an error However, in UBIFS we have only one variable which represents cases 1 and 3 - 'c->ro_media'. Indeed, we set this to 1 if we switch to R/O mode due to an error, and then we test it in many places to make sure that we stop writing as soon as the error happens. But this is very unclean. One consequence of this, for example, is that in 'ubifs_remount_fs()' we use 'c->ro_media' to check whether we are in R/O mode because on an error, and we print a message in this case. However, if we are in R/O mode because the media is R/O, our message is bogus. This patch introduces new flag - 'c->ro_error' which is set when we switch to R/O mode because of an error. It also changes all "if (c->ro_media)" checks to "if (c->ro_error)" checks, because this is what the checks actually mean. We do not need to check for 'c->ro_media' because if the UBI volume is in R/O mode, we do not allow R/W mounting, and now writes can happen. This is guaranteed by VFS. But it is good to double-check this, so this patch also adds many "ubifs_assert(!c->ro_media)" checks. In the 'ubifs_remount_fs()' function this patch makes a bit more changes - it fixes the error messages as well. Signed-off-by: Artem Bityutskiy --- fs/ubifs/commit.c | 4 +++- fs/ubifs/file.c | 6 ++++-- fs/ubifs/gc.c | 3 ++- fs/ubifs/io.c | 16 ++++++++++------ fs/ubifs/journal.c | 3 ++- fs/ubifs/log.c | 4 ++-- fs/ubifs/master.c | 3 ++- fs/ubifs/misc.h | 9 ++++++--- fs/ubifs/shrinker.c | 2 +- fs/ubifs/super.c | 14 +++++++++----- fs/ubifs/ubifs.h | 4 +++- 11 files changed, 44 insertions(+), 24 deletions(-) diff --git a/fs/ubifs/commit.c b/fs/ubifs/commit.c index 37fa7ed062d8..712432789fb8 100644 --- a/fs/ubifs/commit.c +++ b/fs/ubifs/commit.c @@ -63,7 +63,9 @@ static int do_commit(struct ubifs_info *c) struct ubifs_lp_stats lst; dbg_cmt("start"); - if (c->ro_media) { + ubifs_assert(!c->ro_media); + + if (c->ro_error) { err = -EROFS; goto out_up; } diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c index 03ae894c45de..c6bc51c9f07c 100644 --- a/fs/ubifs/file.c +++ b/fs/ubifs/file.c @@ -433,8 +433,9 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping, struct page *page; ubifs_assert(ubifs_inode(inode)->ui_size == inode->i_size); + ubifs_assert(!c->ro_media); - if (unlikely(c->ro_media)) + if (unlikely(c->ro_error)) return -EROFS; /* Try out the fast-path part first */ @@ -1440,8 +1441,9 @@ static int ubifs_vm_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vm dbg_gen("ino %lu, pg %lu, i_size %lld", inode->i_ino, page->index, i_size_read(inode)); ubifs_assert(!(inode->i_sb->s_flags & MS_RDONLY)); + ubifs_assert(!c->ro_media); - if (unlikely(c->ro_media)) + if (unlikely(c->ro_error)) return VM_FAULT_SIGBUS; /* -EROFS */ /* diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index 396f24a30af9..d927196d730b 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -616,13 +616,14 @@ int ubifs_garbage_collect(struct ubifs_info *c, int anyway) struct ubifs_wbuf *wbuf = &c->jheads[GCHD].wbuf; ubifs_assert_cmt_locked(c); + ubifs_assert(!c->ro_media); if (ubifs_gc_should_commit(c)) return -EAGAIN; mutex_lock_nested(&wbuf->io_mutex, wbuf->jhead); - if (c->ro_media) { + if (c->ro_error) { ret = -EROFS; goto out_unlock; } diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c index 9432431bf595..18a4b8d7c844 100644 --- a/fs/ubifs/io.c +++ b/fs/ubifs/io.c @@ -61,8 +61,8 @@ */ void ubifs_ro_mode(struct ubifs_info *c, int err) { - if (!c->ro_media) { - c->ro_media = 1; + if (!c->ro_error) { + c->ro_error = 1; c->no_chk_data_crc = 0; c->vfs_sb->s_flags |= MS_RDONLY; ubifs_warn("switched to read-only mode, error %d", err); @@ -359,8 +359,9 @@ int ubifs_wbuf_sync_nolock(struct ubifs_wbuf *wbuf) ubifs_assert(!(c->vfs_sb->s_flags & MS_RDONLY)); ubifs_assert(!(wbuf->avail & 7)); ubifs_assert(wbuf->offs + c->min_io_size <= c->leb_size); + ubifs_assert(!c->ro_media); - if (c->ro_media) + if (c->ro_error) return -EROFS; ubifs_pad(c, wbuf->buf + wbuf->used, wbuf->avail); @@ -440,11 +441,12 @@ int ubifs_bg_wbufs_sync(struct ubifs_info *c) { int err, i; + ubifs_assert(!c->ro_media); if (!c->need_wbuf_sync) return 0; c->need_wbuf_sync = 0; - if (c->ro_media) { + if (c->ro_error) { err = -EROFS; goto out_timers; } @@ -519,6 +521,7 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len) ubifs_assert(!(wbuf->offs & 7) && wbuf->offs <= c->leb_size); ubifs_assert(wbuf->avail > 0 && wbuf->avail <= c->min_io_size); ubifs_assert(mutex_is_locked(&wbuf->io_mutex)); + ubifs_assert(!c->ro_media); if (c->leb_size - wbuf->offs - wbuf->used < aligned_len) { err = -ENOSPC; @@ -527,7 +530,7 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len) cancel_wbuf_timer_nolock(wbuf); - if (c->ro_media) + if (c->ro_error) return -EROFS; if (aligned_len <= wbuf->avail) { @@ -663,8 +666,9 @@ int ubifs_write_node(struct ubifs_info *c, void *buf, int len, int lnum, buf_len); ubifs_assert(lnum >= 0 && lnum < c->leb_cnt && offs >= 0); ubifs_assert(offs % c->min_io_size == 0 && offs < c->leb_size); + ubifs_assert(!c->ro_media); - if (c->ro_media) + if (c->ro_error) return -EROFS; ubifs_prepare_node(c, buf, len, 1); diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c index d321baeca68d..a6da8aa68f37 100644 --- a/fs/ubifs/journal.c +++ b/fs/ubifs/journal.c @@ -122,11 +122,12 @@ static int reserve_space(struct ubifs_info *c, int jhead, int len) * better to try to allocate space at the ends of eraseblocks. This is * what the squeeze parameter does. */ + ubifs_assert(!c->ro_media); squeeze = (jhead == BASEHD); again: mutex_lock_nested(&wbuf->io_mutex, wbuf->jhead); - if (c->ro_media) { + if (c->ro_error) { err = -EROFS; goto out_unlock; } diff --git a/fs/ubifs/log.c b/fs/ubifs/log.c index c345e125f42c..a41713e2fbb3 100644 --- a/fs/ubifs/log.c +++ b/fs/ubifs/log.c @@ -223,8 +223,8 @@ int ubifs_add_bud_to_log(struct ubifs_info *c, int jhead, int lnum, int offs) } mutex_lock(&c->log_mutex); - - if (c->ro_media) { + ubifs_assert(!c->ro_media); + if (c->ro_error) { err = -EROFS; goto out_unlock; } diff --git a/fs/ubifs/master.c b/fs/ubifs/master.c index 28beaeedadc0..0c818e855f59 100644 --- a/fs/ubifs/master.c +++ b/fs/ubifs/master.c @@ -361,7 +361,8 @@ int ubifs_write_master(struct ubifs_info *c) { int err, lnum, offs, len; - if (c->ro_media) + ubifs_assert(!c->ro_media); + if (c->ro_error) return -EROFS; lnum = UBIFS_MST_LNUM; diff --git a/fs/ubifs/misc.h b/fs/ubifs/misc.h index 4fa81d867e41..5d476ba184e6 100644 --- a/fs/ubifs/misc.h +++ b/fs/ubifs/misc.h @@ -132,7 +132,8 @@ static inline int ubifs_leb_unmap(const struct ubifs_info *c, int lnum) { int err; - if (c->ro_media) + ubifs_assert(!c->ro_media); + if (c->ro_error) return -EROFS; err = ubi_leb_unmap(c->ubi, lnum); if (err) { @@ -159,7 +160,8 @@ static inline int ubifs_leb_write(const struct ubifs_info *c, int lnum, { int err; - if (c->ro_media) + ubifs_assert(!c->ro_media); + if (c->ro_error) return -EROFS; err = ubi_leb_write(c->ubi, lnum, buf, offs, len, dtype); if (err) { @@ -186,7 +188,8 @@ static inline int ubifs_leb_change(const struct ubifs_info *c, int lnum, { int err; - if (c->ro_media) + ubifs_assert(!c->ro_media); + if (c->ro_error) return -EROFS; err = ubi_leb_change(c->ubi, lnum, buf, len, dtype); if (err) { diff --git a/fs/ubifs/shrinker.c b/fs/ubifs/shrinker.c index 0b201114a5ad..10eec8778438 100644 --- a/fs/ubifs/shrinker.c +++ b/fs/ubifs/shrinker.c @@ -250,7 +250,7 @@ static int kick_a_thread(void) dirty_zn_cnt = atomic_long_read(&c->dirty_zn_cnt); if (!dirty_zn_cnt || c->cmt_state == COMMIT_BROKEN || - c->ro_media) { + c->ro_media || c->ro_error) { mutex_unlock(&c->umount_mutex); continue; } diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index cd5900b85d38..1cfeec56df91 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -1751,10 +1751,10 @@ static void ubifs_put_super(struct super_block *sb) ubifs_wbuf_sync(&c->jheads[i].wbuf); /* - * On fatal errors c->ro_media is set to 1, in which case we do + * On fatal errors c->ro_error is set to 1, in which case we do * not write the master node. */ - if (!c->ro_media) { + if (!c->ro_error) { /* * We are being cleanly unmounted which means the * orphans were killed - indicate this in the master @@ -1798,16 +1798,20 @@ static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data) } if ((sb->s_flags & MS_RDONLY) && !(*flags & MS_RDONLY)) { + if (c->ro_error) { + ubifs_msg("cannot re-mount R/W due to prior errors"); + return -EROFS; + } if (c->ro_media) { - ubifs_msg("cannot re-mount due to prior errors"); + ubifs_msg("cannot re-mount R/W - UBI volume is R/O"); return -EROFS; } err = ubifs_remount_rw(c); if (err) return err; } else if (!(sb->s_flags & MS_RDONLY) && (*flags & MS_RDONLY)) { - if (c->ro_media) { - ubifs_msg("cannot re-mount due to prior errors"); + if (c->ro_error) { + ubifs_msg("cannot re-mount R/O due to prior errors"); return -EROFS; } ubifs_remount_ro(c); diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index c4dc9b18f73e..f47ebb442d1c 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -1032,6 +1032,7 @@ struct ubifs_debug_info; * @max_leb_cnt: maximum count of logical eraseblocks * @old_leb_cnt: count of logical eraseblocks before re-size * @ro_media: the underlying UBI volume is read-only + * @ro_error: UBIFS switched to R/O mode because an error happened * * @dirty_pg_cnt: number of dirty pages (not used) * @dirty_zn_cnt: number of dirty znodes @@ -1272,7 +1273,8 @@ struct ubifs_info { int leb_cnt; int max_leb_cnt; int old_leb_cnt; - int ro_media; + unsigned int ro_media:1; + unsigned int ro_error:1; atomic_long_t dirty_pg_cnt; atomic_long_t dirty_zn_cnt; From 2ef13294d29bcfb306e0d360f1b97f37b647b0c0 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 19 Sep 2010 18:34:26 +0300 Subject: [PATCH 15/20] UBIFS: introduce new flags for RO mounts Commit 2fde99cb55fb9d9b88180512a5e8a5d939d27fec "UBIFS: mark VFS SB RO too" introduced regression. This commit made UBIFS set the 'MS_RDONLY' flag in the VFS superblock when it switches to R/O mode due to an error. This was done to make VFS show the R/O UBIFS flag in /proc/mounts. However, several places in UBIFS relied on the 'MS_RDONLY' flag and assume this flag can only change when we re-mount. For example, 'ubifs_put_super()'. This patch introduces new UBIFS flag - 'c->ro_mount' which changes only when we re-mount, and preserves the way UBIFS was originally mounted (R/W or R/O). This allows us to de-initialize UBIFS cleanly in 'ubifs_put_super()'. This patch also changes all 'ubifs_assert(!c->ro_media)' assertions to 'ubifs_assert(!c->ro_media && !c->ro_mount)', because we never should write anything if the FS was mounter R/O. All the places where we test for 'MS_RDONLY' flag in the VFS SB were changed and now we test the 'c->ro_mount' flag instead, because it preserves the original UBIFS mount type, unlike the 'MS_RDONLY' flag. Signed-off-by: Artem Bityutskiy --- fs/ubifs/commit.c | 2 +- fs/ubifs/file.c | 5 ++--- fs/ubifs/gc.c | 2 +- fs/ubifs/io.c | 9 ++++---- fs/ubifs/journal.c | 2 +- fs/ubifs/log.c | 4 ++-- fs/ubifs/master.c | 2 +- fs/ubifs/misc.h | 6 ++--- fs/ubifs/recovery.c | 8 +++---- fs/ubifs/replay.c | 3 +-- fs/ubifs/sb.c | 9 +++----- fs/ubifs/shrinker.c | 2 +- fs/ubifs/super.c | 55 ++++++++++++++++++++++++--------------------- fs/ubifs/ubifs.h | 13 +++++++---- 14 files changed, 62 insertions(+), 60 deletions(-) diff --git a/fs/ubifs/commit.c b/fs/ubifs/commit.c index 712432789fb8..02429d81ca33 100644 --- a/fs/ubifs/commit.c +++ b/fs/ubifs/commit.c @@ -63,7 +63,7 @@ static int do_commit(struct ubifs_info *c) struct ubifs_lp_stats lst; dbg_cmt("start"); - ubifs_assert(!c->ro_media); + ubifs_assert(!c->ro_media && !c->ro_mount); if (c->ro_error) { err = -EROFS; diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c index c6bc51c9f07c..d77db7e36484 100644 --- a/fs/ubifs/file.c +++ b/fs/ubifs/file.c @@ -433,7 +433,7 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping, struct page *page; ubifs_assert(ubifs_inode(inode)->ui_size == inode->i_size); - ubifs_assert(!c->ro_media); + ubifs_assert(!c->ro_media && !c->ro_mount); if (unlikely(c->ro_error)) return -EROFS; @@ -1440,8 +1440,7 @@ static int ubifs_vm_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vm dbg_gen("ino %lu, pg %lu, i_size %lld", inode->i_ino, page->index, i_size_read(inode)); - ubifs_assert(!(inode->i_sb->s_flags & MS_RDONLY)); - ubifs_assert(!c->ro_media); + ubifs_assert(!c->ro_media && !c->ro_mount); if (unlikely(c->ro_error)) return VM_FAULT_SIGBUS; /* -EROFS */ diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index d927196d730b..151f10882820 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -616,7 +616,7 @@ int ubifs_garbage_collect(struct ubifs_info *c, int anyway) struct ubifs_wbuf *wbuf = &c->jheads[GCHD].wbuf; ubifs_assert_cmt_locked(c); - ubifs_assert(!c->ro_media); + ubifs_assert(!c->ro_media && !c->ro_mount); if (ubifs_gc_should_commit(c)) return -EAGAIN; diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c index 18a4b8d7c844..d82173182eeb 100644 --- a/fs/ubifs/io.c +++ b/fs/ubifs/io.c @@ -356,10 +356,9 @@ int ubifs_wbuf_sync_nolock(struct ubifs_wbuf *wbuf) dbg_io("LEB %d:%d, %d bytes, jhead %s", wbuf->lnum, wbuf->offs, wbuf->used, dbg_jhead(wbuf->jhead)); - ubifs_assert(!(c->vfs_sb->s_flags & MS_RDONLY)); ubifs_assert(!(wbuf->avail & 7)); ubifs_assert(wbuf->offs + c->min_io_size <= c->leb_size); - ubifs_assert(!c->ro_media); + ubifs_assert(!c->ro_media && !c->ro_mount); if (c->ro_error) return -EROFS; @@ -441,7 +440,7 @@ int ubifs_bg_wbufs_sync(struct ubifs_info *c) { int err, i; - ubifs_assert(!c->ro_media); + ubifs_assert(!c->ro_media && !c->ro_mount); if (!c->need_wbuf_sync) return 0; c->need_wbuf_sync = 0; @@ -521,7 +520,7 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len) ubifs_assert(!(wbuf->offs & 7) && wbuf->offs <= c->leb_size); ubifs_assert(wbuf->avail > 0 && wbuf->avail <= c->min_io_size); ubifs_assert(mutex_is_locked(&wbuf->io_mutex)); - ubifs_assert(!c->ro_media); + ubifs_assert(!c->ro_media && !c->ro_mount); if (c->leb_size - wbuf->offs - wbuf->used < aligned_len) { err = -ENOSPC; @@ -666,7 +665,7 @@ int ubifs_write_node(struct ubifs_info *c, void *buf, int len, int lnum, buf_len); ubifs_assert(lnum >= 0 && lnum < c->leb_cnt && offs >= 0); ubifs_assert(offs % c->min_io_size == 0 && offs < c->leb_size); - ubifs_assert(!c->ro_media); + ubifs_assert(!c->ro_media && !c->ro_mount); if (c->ro_error) return -EROFS; diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c index a6da8aa68f37..914f1bd89e57 100644 --- a/fs/ubifs/journal.c +++ b/fs/ubifs/journal.c @@ -122,7 +122,7 @@ static int reserve_space(struct ubifs_info *c, int jhead, int len) * better to try to allocate space at the ends of eraseblocks. This is * what the squeeze parameter does. */ - ubifs_assert(!c->ro_media); + ubifs_assert(!c->ro_media && !c->ro_mount); squeeze = (jhead == BASEHD); again: mutex_lock_nested(&wbuf->io_mutex, wbuf->jhead); diff --git a/fs/ubifs/log.c b/fs/ubifs/log.c index a41713e2fbb3..4d0cb1241460 100644 --- a/fs/ubifs/log.c +++ b/fs/ubifs/log.c @@ -159,7 +159,7 @@ void ubifs_add_bud(struct ubifs_info *c, struct ubifs_bud *bud) jhead = &c->jheads[bud->jhead]; list_add_tail(&bud->list, &jhead->buds_list); } else - ubifs_assert(c->replaying && (c->vfs_sb->s_flags & MS_RDONLY)); + ubifs_assert(c->replaying && c->ro_mount); /* * Note, although this is a new bud, we anyway account this space now, @@ -223,7 +223,7 @@ int ubifs_add_bud_to_log(struct ubifs_info *c, int jhead, int lnum, int offs) } mutex_lock(&c->log_mutex); - ubifs_assert(!c->ro_media); + ubifs_assert(!c->ro_media && !c->ro_mount); if (c->ro_error) { err = -EROFS; goto out_unlock; diff --git a/fs/ubifs/master.c b/fs/ubifs/master.c index 0c818e855f59..21f47afdacff 100644 --- a/fs/ubifs/master.c +++ b/fs/ubifs/master.c @@ -361,7 +361,7 @@ int ubifs_write_master(struct ubifs_info *c) { int err, lnum, offs, len; - ubifs_assert(!c->ro_media); + ubifs_assert(!c->ro_media && !c->ro_mount); if (c->ro_error) return -EROFS; diff --git a/fs/ubifs/misc.h b/fs/ubifs/misc.h index 5d476ba184e6..c3de04dc952a 100644 --- a/fs/ubifs/misc.h +++ b/fs/ubifs/misc.h @@ -132,7 +132,7 @@ static inline int ubifs_leb_unmap(const struct ubifs_info *c, int lnum) { int err; - ubifs_assert(!c->ro_media); + ubifs_assert(!c->ro_media && !c->ro_mount); if (c->ro_error) return -EROFS; err = ubi_leb_unmap(c->ubi, lnum); @@ -160,7 +160,7 @@ static inline int ubifs_leb_write(const struct ubifs_info *c, int lnum, { int err; - ubifs_assert(!c->ro_media); + ubifs_assert(!c->ro_media && !c->ro_mount); if (c->ro_error) return -EROFS; err = ubi_leb_write(c->ubi, lnum, buf, offs, len, dtype); @@ -188,7 +188,7 @@ static inline int ubifs_leb_change(const struct ubifs_info *c, int lnum, { int err; - ubifs_assert(!c->ro_media); + ubifs_assert(!c->ro_media && !c->ro_mount); if (c->ro_error) return -EROFS; err = ubi_leb_change(c->ubi, lnum, buf, len, dtype); diff --git a/fs/ubifs/recovery.c b/fs/ubifs/recovery.c index daae9e1f5382..c902a5de90ae 100644 --- a/fs/ubifs/recovery.c +++ b/fs/ubifs/recovery.c @@ -292,7 +292,7 @@ int ubifs_recover_master_node(struct ubifs_info *c) memcpy(c->mst_node, mst, UBIFS_MST_NODE_SZ); - if ((c->vfs_sb->s_flags & MS_RDONLY)) { + if (c->ro_mount) { /* Read-only mode. Keep a copy for switching to rw mode */ c->rcvrd_mst_node = kmalloc(sz, GFP_KERNEL); if (!c->rcvrd_mst_node) { @@ -469,7 +469,7 @@ static int fix_unclean_leb(struct ubifs_info *c, struct ubifs_scan_leb *sleb, endpt = snod->offs + snod->len; } - if ((c->vfs_sb->s_flags & MS_RDONLY) && !c->remounting_rw) { + if (c->ro_mount && !c->remounting_rw) { /* Add to recovery list */ struct ubifs_unclean_leb *ucleb; @@ -883,7 +883,7 @@ int ubifs_recover_inl_heads(const struct ubifs_info *c, void *sbuf) { int err; - ubifs_assert(!(c->vfs_sb->s_flags & MS_RDONLY) || c->remounting_rw); + ubifs_assert(!c->ro_mount || c->remounting_rw); dbg_rcvry("checking index head at %d:%d", c->ihead_lnum, c->ihead_offs); err = recover_head(c, c->ihead_lnum, c->ihead_offs, sbuf); @@ -1461,7 +1461,7 @@ int ubifs_recover_size(struct ubifs_info *c) } } if (e->exists && e->i_size < e->d_size) { - if (!e->inode && (c->vfs_sb->s_flags & MS_RDONLY)) { + if (!e->inode && c->ro_mount) { /* Fix the inode size and pin it in memory */ struct inode *inode; diff --git a/fs/ubifs/replay.c b/fs/ubifs/replay.c index 5c2d6d759a3e..730598cf6342 100644 --- a/fs/ubifs/replay.c +++ b/fs/ubifs/replay.c @@ -627,8 +627,7 @@ static int replay_bud(struct ubifs_info *c, int lnum, int offs, int jhead, ubifs_assert(sleb->endpt - offs >= used); ubifs_assert(sleb->endpt % c->min_io_size == 0); - if (sleb->endpt + c->min_io_size <= c->leb_size && - !(c->vfs_sb->s_flags & MS_RDONLY)) + if (sleb->endpt + c->min_io_size <= c->leb_size && !c->ro_mount) err = ubifs_wbuf_seek_nolock(&c->jheads[jhead].wbuf, lnum, sleb->endpt, UBI_SHORTTERM); diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c index 96cb62c8a9dd..bf31b4729e51 100644 --- a/fs/ubifs/sb.c +++ b/fs/ubifs/sb.c @@ -542,11 +542,8 @@ int ubifs_read_superblock(struct ubifs_info *c) * due to the unavailability of time-travelling equipment. */ if (c->fmt_version > UBIFS_FORMAT_VERSION) { - struct super_block *sb = c->vfs_sb; - int mounting_ro = sb->s_flags & MS_RDONLY; - - ubifs_assert(!c->ro_media || mounting_ro); - if (!mounting_ro || + ubifs_assert(!c->ro_media || c->ro_mount); + if (!c->ro_mount || c->ro_compat_version > UBIFS_RO_COMPAT_VERSION) { ubifs_err("on-flash format version is w%d/r%d, but " "software only supports up to version " @@ -624,7 +621,7 @@ int ubifs_read_superblock(struct ubifs_info *c) c->old_leb_cnt = c->leb_cnt; if (c->leb_cnt < c->vi.size && c->leb_cnt < c->max_leb_cnt) { c->leb_cnt = min_t(int, c->max_leb_cnt, c->vi.size); - if (c->vfs_sb->s_flags & MS_RDONLY) + if (c->ro_mount) dbg_mnt("Auto resizing (ro) from %d LEBs to %d LEBs", c->old_leb_cnt, c->leb_cnt); else { diff --git a/fs/ubifs/shrinker.c b/fs/ubifs/shrinker.c index 10eec8778438..46961c003236 100644 --- a/fs/ubifs/shrinker.c +++ b/fs/ubifs/shrinker.c @@ -250,7 +250,7 @@ static int kick_a_thread(void) dirty_zn_cnt = atomic_long_read(&c->dirty_zn_cnt); if (!dirty_zn_cnt || c->cmt_state == COMMIT_BROKEN || - c->ro_media || c->ro_error) { + c->ro_mount || c->ro_error) { mutex_unlock(&c->umount_mutex); continue; } diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 1cfeec56df91..4e5bf3fbf782 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -1137,11 +1137,11 @@ static int check_free_space(struct ubifs_info *c) */ static int mount_ubifs(struct ubifs_info *c) { - struct super_block *sb = c->vfs_sb; - int err, mounted_read_only = (sb->s_flags & MS_RDONLY); + int err; long long x; size_t sz; + c->ro_mount = !!(c->vfs_sb->s_flags & MS_RDONLY); err = init_constants_early(c); if (err) return err; @@ -1154,7 +1154,7 @@ static int mount_ubifs(struct ubifs_info *c) if (err) goto out_free; - if (c->empty && (mounted_read_only || c->ro_media)) { + if (c->empty && (c->ro_mount || c->ro_media)) { /* * This UBI volume is empty, and read-only, or the file system * is mounted read-only - we cannot format it. @@ -1165,7 +1165,7 @@ static int mount_ubifs(struct ubifs_info *c) goto out_free; } - if (c->ro_media && !mounted_read_only) { + if (c->ro_media && !c->ro_mount) { ubifs_err("cannot mount read-write - read-only media"); err = -EROFS; goto out_free; @@ -1185,7 +1185,7 @@ static int mount_ubifs(struct ubifs_info *c) if (!c->sbuf) goto out_free; - if (!mounted_read_only) { + if (!c->ro_mount) { c->ileb_buf = vmalloc(c->leb_size); if (!c->ileb_buf) goto out_free; @@ -1228,7 +1228,7 @@ static int mount_ubifs(struct ubifs_info *c) } sprintf(c->bgt_name, BGT_NAME_PATTERN, c->vi.ubi_num, c->vi.vol_id); - if (!mounted_read_only) { + if (!c->ro_mount) { err = alloc_wbufs(c); if (err) goto out_cbuf; @@ -1254,12 +1254,12 @@ static int mount_ubifs(struct ubifs_info *c) if ((c->mst_node->flags & cpu_to_le32(UBIFS_MST_DIRTY)) != 0) { ubifs_msg("recovery needed"); c->need_recovery = 1; - if (!mounted_read_only) { + if (!c->ro_mount) { err = ubifs_recover_inl_heads(c, c->sbuf); if (err) goto out_master; } - } else if (!mounted_read_only) { + } else if (!c->ro_mount) { /* * Set the "dirty" flag so that if we reboot uncleanly we * will notice this immediately on the next mount. @@ -1270,7 +1270,7 @@ static int mount_ubifs(struct ubifs_info *c) goto out_master; } - err = ubifs_lpt_init(c, 1, !mounted_read_only); + err = ubifs_lpt_init(c, 1, !c->ro_mount); if (err) goto out_lpt; @@ -1285,11 +1285,11 @@ static int mount_ubifs(struct ubifs_info *c) /* Calculate 'min_idx_lebs' after journal replay */ c->min_idx_lebs = ubifs_calc_min_idx_lebs(c); - err = ubifs_mount_orphans(c, c->need_recovery, mounted_read_only); + err = ubifs_mount_orphans(c, c->need_recovery, c->ro_mount); if (err) goto out_orphans; - if (!mounted_read_only) { + if (!c->ro_mount) { int lnum; err = check_free_space(c); @@ -1351,7 +1351,7 @@ static int mount_ubifs(struct ubifs_info *c) spin_unlock(&ubifs_infos_lock); if (c->need_recovery) { - if (mounted_read_only) + if (c->ro_mount) ubifs_msg("recovery deferred"); else { c->need_recovery = 0; @@ -1378,7 +1378,7 @@ static int mount_ubifs(struct ubifs_info *c) ubifs_msg("mounted UBI device %d, volume %d, name \"%s\"", c->vi.ubi_num, c->vi.vol_id, c->vi.name); - if (mounted_read_only) + if (c->ro_mount) ubifs_msg("mounted read-only"); x = (long long)c->main_lebs * c->leb_size; ubifs_msg("file system size: %lld bytes (%lld KiB, %lld MiB, %d " @@ -1640,7 +1640,7 @@ static int ubifs_remount_rw(struct ubifs_info *c) } dbg_gen("re-mounted read-write"); - c->vfs_sb->s_flags &= ~MS_RDONLY; + c->ro_mount = 0; c->remounting_rw = 0; c->always_chk_crc = 0; err = dbg_check_space_info(c); @@ -1676,7 +1676,7 @@ static void ubifs_remount_ro(struct ubifs_info *c) int i, err; ubifs_assert(!c->need_recovery); - ubifs_assert(!(c->vfs_sb->s_flags & MS_RDONLY)); + ubifs_assert(!c->ro_mount); mutex_lock(&c->umount_mutex); if (c->bgt) { @@ -1704,6 +1704,7 @@ static void ubifs_remount_ro(struct ubifs_info *c) vfree(c->ileb_buf); c->ileb_buf = NULL; ubifs_lpt_free(c, 1); + c->ro_mount = 1; err = dbg_check_space_info(c); if (err) ubifs_ro_mode(c, err); @@ -1735,7 +1736,7 @@ static void ubifs_put_super(struct super_block *sb) * the mutex is locked. */ mutex_lock(&c->umount_mutex); - if (!(c->vfs_sb->s_flags & MS_RDONLY)) { + if (!c->ro_mount) { /* * First of all kill the background thread to make sure it does * not interfere with un-mounting and freeing resources. @@ -1745,23 +1746,23 @@ static void ubifs_put_super(struct super_block *sb) c->bgt = NULL; } - /* Synchronize write-buffers */ - if (c->jheads) - for (i = 0; i < c->jhead_cnt; i++) - ubifs_wbuf_sync(&c->jheads[i].wbuf); - /* * On fatal errors c->ro_error is set to 1, in which case we do * not write the master node. */ if (!c->ro_error) { + int err; + + /* Synchronize write-buffers */ + if (c->jheads) + for (i = 0; i < c->jhead_cnt; i++) + ubifs_wbuf_sync(&c->jheads[i].wbuf); + /* * We are being cleanly unmounted which means the * orphans were killed - indicate this in the master * node. Also save the reserved GC LEB number. */ - int err; - c->mst_node->flags &= ~cpu_to_le32(UBIFS_MST_DIRTY); c->mst_node->flags |= cpu_to_le32(UBIFS_MST_NO_ORPHS); c->mst_node->gc_lnum = cpu_to_le32(c->gc_lnum); @@ -1797,7 +1798,7 @@ static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data) return err; } - if ((sb->s_flags & MS_RDONLY) && !(*flags & MS_RDONLY)) { + if (c->ro_mount && !(*flags & MS_RDONLY)) { if (c->ro_error) { ubifs_msg("cannot re-mount R/W due to prior errors"); return -EROFS; @@ -1809,7 +1810,7 @@ static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data) err = ubifs_remount_rw(c); if (err) return err; - } else if (!(sb->s_flags & MS_RDONLY) && (*flags & MS_RDONLY)) { + } else if (!c->ro_mount && (*flags & MS_RDONLY)) { if (c->ro_error) { ubifs_msg("cannot re-mount R/O due to prior errors"); return -EROFS; @@ -2068,9 +2069,11 @@ static int ubifs_get_sb(struct file_system_type *fs_type, int flags, } if (sb->s_root) { + struct ubifs_info *c1 = sb->s_fs_info; + /* A new mount point for already mounted UBIFS */ dbg_gen("this ubi volume is already mounted"); - if ((flags ^ sb->s_flags) & MS_RDONLY) { + if (!!(flags & MS_RDONLY) != c1->ro_mount) { err = -EBUSY; goto out_deact; } diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index f47ebb442d1c..381d6b207a52 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -1032,6 +1032,7 @@ struct ubifs_debug_info; * @max_leb_cnt: maximum count of logical eraseblocks * @old_leb_cnt: count of logical eraseblocks before re-size * @ro_media: the underlying UBI volume is read-only + * @ro_mount: the file-system was mounted as read-only * @ro_error: UBIFS switched to R/O mode because an error happened * * @dirty_pg_cnt: number of dirty pages (not used) @@ -1173,11 +1174,14 @@ struct ubifs_debug_info; * @replay_sqnum: sequence number of node currently being replayed * @need_recovery: file-system needs recovery * @replaying: set to %1 during journal replay - * @unclean_leb_list: LEBs to recover when mounting ro to rw - * @rcvrd_mst_node: recovered master node to write when mounting ro to rw + * @unclean_leb_list: LEBs to recover when re-mounting R/O mounted FS to R/W + * mode + * @rcvrd_mst_node: recovered master node to write when re-mounting R/O mounted + * FS to R/W mode * @size_tree: inode size information for recovery - * @remounting_rw: set while remounting from ro to rw (sb flags have MS_RDONLY) - * @always_chk_crc: always check CRCs (while mounting and remounting rw) + * @remounting_rw: set while re-mounting from R/O mode to R/W mode + * @always_chk_crc: always check CRCs (while mounting and remounting to R/W + * mode) * @mount_opts: UBIFS-specific mount options * * @dbg: debugging-related information @@ -1274,6 +1278,7 @@ struct ubifs_info { int max_leb_cnt; int old_leb_cnt; unsigned int ro_media:1; + unsigned int ro_mount:1; unsigned int ro_error:1; atomic_long_t dirty_pg_cnt; From 54dd55a406f6e9cb5ae208f258b907455162e045 Mon Sep 17 00:00:00 2001 From: Steffen Sledz Date: Mon, 27 Sep 2010 14:20:26 +0200 Subject: [PATCH 16/20] UBIFS: avoid kernel error if ubifs superblock read fails .get_sb is called on mounts with automatic fs detection too, so this function should print an error if it cannot read the superblock in debug mode only (new behaviour conforms the other fs types) Signed-off-by: Steffen Sledz Signed-off-by: Artem Bityutskiy --- fs/ubifs/super.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 4e5bf3fbf782..bb6ed5da2d10 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -2054,8 +2054,8 @@ static int ubifs_get_sb(struct file_system_type *fs_type, int flags, */ ubi = open_ubi(name, UBI_READONLY); if (IS_ERR(ubi)) { - ubifs_err("cannot open \"%s\", error %d", - name, (int)PTR_ERR(ubi)); + dbg_err("cannot open \"%s\", error %d", + name, (int)PTR_ERR(ubi)); return PTR_ERR(ubi); } ubi_get_volume_info(ubi, &vi); From 7d08ae3c9205b559f90c3d7a3abba3c6479673c7 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 17 Oct 2010 15:50:19 +0300 Subject: [PATCH 17/20] UBIFS: add a commentary about log recovery Add a commentary which elaborates that 'ubifs_recover_log_leb()' recovers only the last log LEB, not any. Also remove some unneeded newlines. Signed-off-by: Artem Bityutskiy --- fs/ubifs/recovery.c | 3 ++- fs/ubifs/replay.c | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/fs/ubifs/recovery.c b/fs/ubifs/recovery.c index c902a5de90ae..77e9b874b6c2 100644 --- a/fs/ubifs/recovery.c +++ b/fs/ubifs/recovery.c @@ -772,7 +772,8 @@ out_free: * @sbuf: LEB-sized buffer to use * * This function does a scan of a LEB, but caters for errors that might have - * been caused by the unclean unmount from which we are attempting to recover. + * been caused by unclean reboots from which we are attempting to recover + * (assume that only the last log LEB can be corrupted by an unclean reboot). * * This function returns %0 on success and a negative error code on failure. */ diff --git a/fs/ubifs/replay.c b/fs/ubifs/replay.c index 730598cf6342..7df04ba4878e 100644 --- a/fs/ubifs/replay.c +++ b/fs/ubifs/replay.c @@ -839,6 +839,11 @@ static int replay_log_leb(struct ubifs_info *c, int lnum, int offs, void *sbuf) if (IS_ERR(sleb)) { if (PTR_ERR(sleb) != -EUCLEAN || !c->need_recovery) return PTR_ERR(sleb); + /* + * Note, the below function will recover this log LEB only if + * it is the last, because unclean reboots can possibly corrupt + * only the tail of the log. + */ sleb = ubifs_recover_log_leb(c, lnum, offs, sbuf); if (IS_ERR(sleb)) return PTR_ERR(sleb); @@ -850,7 +855,6 @@ static int replay_log_leb(struct ubifs_info *c, int lnum, int offs, void *sbuf) } node = sleb->buf; - snod = list_entry(sleb->nodes.next, struct ubifs_scan_node, list); if (c->cs_sqnum == 0) { /* @@ -897,7 +901,6 @@ static int replay_log_leb(struct ubifs_info *c, int lnum, int offs, void *sbuf) } list_for_each_entry(snod, &sleb->nodes, list) { - cond_resched(); if (snod->sqnum >= SQNUM_WATERMARK) { @@ -1030,9 +1033,7 @@ int ubifs_replay_journal(struct ubifs_info *c) return -ENOMEM; dbg_mnt("start replaying the journal"); - c->replaying = 1; - lnum = c->ltail_lnum = c->lhead_lnum; offs = c->lhead_offs; From 39037559e651c417fb68b828926dc61cd5d6e5e2 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Mon, 18 Oct 2010 08:28:50 +0300 Subject: [PATCH 18/20] UBIFS: remove a bit of unneeded code This is a clean-up patch which: 1. Removes explicite 'hrtimer_cancel()' after 'ubifs_wbuf_sync()' in 'ubifs_remount_ro()', because the timers will be canceled by 'ubifs_wbuf_sync()', no need to cancel them for the second time. 2. Remove "if (c->jheads)" check from 'ubifs_put_super()', because at journal heads must always be allocated there, since we checked earlier that we were mounted R/W, and the olny situation when journal heads are not allocated is when mounter or re-mounted R/O. Signed-off-by: Artem Bityutskiy --- fs/ubifs/super.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index bb6ed5da2d10..6ff2f802bd7e 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -1686,10 +1686,8 @@ static void ubifs_remount_ro(struct ubifs_info *c) dbg_save_space_info(c); - for (i = 0; i < c->jhead_cnt; i++) { + for (i = 0; i < c->jhead_cnt; i++) ubifs_wbuf_sync(&c->jheads[i].wbuf); - hrtimer_cancel(&c->jheads[i].wbuf.timer); - } c->mst_node->flags &= ~cpu_to_le32(UBIFS_MST_DIRTY); c->mst_node->flags |= cpu_to_le32(UBIFS_MST_NO_ORPHS); @@ -1754,9 +1752,8 @@ static void ubifs_put_super(struct super_block *sb) int err; /* Synchronize write-buffers */ - if (c->jheads) - for (i = 0; i < c->jhead_cnt; i++) - ubifs_wbuf_sync(&c->jheads[i].wbuf); + for (i = 0; i < c->jhead_cnt; i++) + ubifs_wbuf_sync(&c->jheads[i].wbuf); /* * We are being cleanly unmounted which means the From 3601ba27353a968df843454e4b81155376682505 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Mon, 18 Oct 2010 08:32:35 +0300 Subject: [PATCH 19/20] UBIFS: do not forget to cancel timers This is a bug-fix: when we unmount, and we are currently in R/O mode because of an error - we do not sync write-buffers, which means we also do not cancel write-buffer timers we may possibly have armed. This patch fixes the issue. The issue can easily be reproduced by enabling UBIFS failure debug mode (echo 4 > /sys/module/ubifs/parameters/debug_tsts) and unmounting as soon as a failure happen. At some point the system oopses because we have an armed hrtimer but UBIFS is unmounted already. Signed-off-by: Artem Bityutskiy --- fs/ubifs/super.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 6ff2f802bd7e..9a47c9f0ad07 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -1772,6 +1772,10 @@ static void ubifs_put_super(struct super_block *sb) */ ubifs_err("failed to write master node, " "error %d", err); + } else { + for (i = 0; i < c->jhead_cnt; i++) + /* Make sure write-buffer timers are canceled */ + hrtimer_cancel(&c->jheads[i].wbuf.timer); } } From 6599fcbd01baf9d57e847db103d215ea4ec088f9 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Mon, 18 Oct 2010 10:00:40 +0300 Subject: [PATCH 20/20] UBIFS: do not allocate unneeded scan buffer In 'ubifs_replay_journal()' we allocate 'sbuf' for scanning the log. However, we already have 'c->sbuf' for these purposes, so do not allocate yet another one. This reduces UBIFS memory consumption while recovering. Signed-off-by: Artem Bityutskiy --- fs/ubifs/replay.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/fs/ubifs/replay.c b/fs/ubifs/replay.c index 7df04ba4878e..eed0fcff8d73 100644 --- a/fs/ubifs/replay.c +++ b/fs/ubifs/replay.c @@ -1013,7 +1013,6 @@ out: int ubifs_replay_journal(struct ubifs_info *c) { int err, i, lnum, offs, free; - void *sbuf = NULL; BUILD_BUG_ON(UBIFS_TRUN_KEY > 5); @@ -1028,10 +1027,6 @@ int ubifs_replay_journal(struct ubifs_info *c) return -EINVAL; } - sbuf = vmalloc(c->leb_size); - if (!sbuf) - return -ENOMEM; - dbg_mnt("start replaying the journal"); c->replaying = 1; lnum = c->ltail_lnum = c->lhead_lnum; @@ -1046,7 +1041,7 @@ int ubifs_replay_journal(struct ubifs_info *c) lnum = UBIFS_LOG_LNUM; offs = 0; } - err = replay_log_leb(c, lnum, offs, sbuf); + err = replay_log_leb(c, lnum, offs, c->sbuf); if (err == 1) /* We hit the end of the log */ break; @@ -1079,7 +1074,6 @@ int ubifs_replay_journal(struct ubifs_info *c) out: destroy_replay_tree(c); destroy_bud_list(c); - vfree(sbuf); c->replaying = 0; return err; }