[PATCH resend] UBIFS: return -EINVAL if first log leb is empty
Artem Bityutskiy
dedekind1 at gmail.com
Sat Jan 31 01:51:46 PST 2015
Hi Hujianyang,
On Sat, 2015-01-31 at 11:43 +0800, hujianyang wrote:
> CS node is recognized as a sign in UBIFS log replay mechanism.
> Log relaying during mount should find the CS node in first log
> leb at beginning and then replay the following uncommitted
> buds.
Lets use the "log head" term instead of "the first log LEB", just to be
consistent.
> Here is a bug in log replay path: If first log leb, which is
> indicated by @log_lnum in mst_node, is empty, current UBIFS
> replay nothing and directly mount the partition without any
> warning. This action will put filesystem in an abnormal state,
> e.g. space management in LPT area is incorrect to the real
> space usage in main area.
Looks like a good catch, thank you! I have few requests, though.
>
> if (sleb->nodes_cnt == 0) {
> - err = 1;
> + if (unlikely(c->cs_sqnum == 0)) {
Please, do not use unlikely(), it may only be used on hot-paths, which
the replay code is certainly not.
IIUC, this if translates to "is this the head of the log?". If I am
right, then the better place for doing this check is the caller function
- 'ubifs_replay_journal()'. In that function you just do
if (lnum == c->lhead_lnum)
which is way easier to understand than "if (c->cs_sqnum == 0)", I think.
> + /* This is the first log LEB, should not be empty */
I'd expand this comment a bit to:
The head of the log must always start with the "commit start" node on a
properly formatted UBIFS. But we found no nodes at all, which means that
something went wrong and we cannot proceed mounting the file-system.
I think the next developer who looks at the will appreciate these kinds
of commentaries.
> + ubifs_err("first log leb LEB %d:%d is empty, no CS node exist",
> + lnum, offs);
Lets use consistent terminology. We call it the "log head".
I'd phrase it like this:
no UBIFS nodes found at the log head LEB %d:%d, possibly the FS is
corrupted.
More information about the linux-mtd
mailing list