* [PATCH 1/3] UBIFS: assert no fixup when writing a node
2011-05-30 18:56 [PATCH 0/3] fix-up free space earlier in mount_ubifs() Ben Gardiner
@ 2011-05-30 18:56 ` Ben Gardiner
2011-06-01 10:03 ` Artem Bityutskiy
2011-05-30 18:56 ` [PATCH 2/3] UBIFS: intialize the LPT earlier Ben Gardiner
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Ben Gardiner @ 2011-05-30 18:56 UTC (permalink / raw)
To: Artem Bityutskiy, Adrian Hunter
Cc: linux-mtd, linux-kernel, Matthew L. Creech
The current free space fixup can result in some writing to the UBI volume
when the space_fixup flag is set.
To catch instances where UBIFS is writing to the NAND while the space_fixup
flag is set, add an assert to ubifs_write_node().
Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
---
fs/ubifs/io.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index 166951e..db298de 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -763,6 +763,8 @@ int ubifs_write_node(struct ubifs_info *c, void *buf, int len, int lnum,
if (c->ro_error)
return -EROFS;
+ ubifs_assert(!c->space_fixup);
+
ubifs_prepare_node(c, buf, len, 1);
err = ubi_leb_write(c->ubi, lnum, buf, offs, buf_len, dtype);
if (err) {
--
1.7.4.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 1/3] UBIFS: assert no fixup when writing a node
2011-05-30 18:56 ` [PATCH 1/3] UBIFS: assert no fixup when writing a node Ben Gardiner
@ 2011-06-01 10:03 ` Artem Bityutskiy
0 siblings, 0 replies; 17+ messages in thread
From: Artem Bityutskiy @ 2011-06-01 10:03 UTC (permalink / raw)
To: Ben Gardiner; +Cc: linux-mtd, linux-kernel, Matthew L. Creech
On Mon, 2011-05-30 at 14:56 -0400, Ben Gardiner wrote:
> The current free space fixup can result in some writing to the UBI volume
> when the space_fixup flag is set.
>
> To catch instances where UBIFS is writing to the NAND while the space_fixup
> flag is set, add an assert to ubifs_write_node().
>
> Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
>
> ---
> fs/ubifs/io.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
> index 166951e..db298de 100644
> --- a/fs/ubifs/io.c
> +++ b/fs/ubifs/io.c
> @@ -763,6 +763,8 @@ int ubifs_write_node(struct ubifs_info *c, void *buf, int len, int lnum,
> if (c->ro_error)
> return -EROFS;
>
> + ubifs_assert(!c->space_fixup);
> +
I've moved this assert upper to where all other assertions are placed.
I've also added this assertion to 'ubifs_wbuf_write_nolock()' because it
is the other often used write path.
And pushed to the UBIFS tree, thanks.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/3] UBIFS: intialize the LPT earlier
2011-05-30 18:56 [PATCH 0/3] fix-up free space earlier in mount_ubifs() Ben Gardiner
2011-05-30 18:56 ` [PATCH 1/3] UBIFS: assert no fixup when writing a node Ben Gardiner
@ 2011-05-30 18:56 ` Ben Gardiner
2011-06-01 10:06 ` Artem Bityutskiy
2011-05-30 18:56 ` [PATCH 3/3] UBIFS: fix-up free space earlier Ben Gardiner
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Ben Gardiner @ 2011-05-30 18:56 UTC (permalink / raw)
To: Artem Bityutskiy, Adrian Hunter
Cc: linux-mtd, linux-kernel, Matthew L. Creech
The current mount_ubifs() implementation does not initialize the LPT until the
check for recovery is completed and the dirty flag is written.
Move the LPT initializtion to before the check for recovery and dirty flag in
preparation for the next patch which will move the free-space-fixup check to
directly after this allocation.
Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
---
fs/ubifs/super.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 6db0bdaa..89c8d33 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1281,13 +1281,17 @@ static int mount_ubifs(struct ubifs_info *c)
init_constants_master(c);
+ err = ubifs_lpt_init(c, 1, !c->ro_mount);
+ if (err)
+ goto out_lpt;
+
if ((c->mst_node->flags & cpu_to_le32(UBIFS_MST_DIRTY)) != 0) {
ubifs_msg("recovery needed");
c->need_recovery = 1;
if (!c->ro_mount) {
err = ubifs_recover_inl_heads(c, c->sbuf);
if (err)
- goto out_master;
+ goto out_lpt;
}
} else if (!c->ro_mount) {
/*
@@ -1297,13 +1301,9 @@ static int mount_ubifs(struct ubifs_info *c)
c->mst_node->flags |= cpu_to_le32(UBIFS_MST_DIRTY);
err = ubifs_write_master(c);
if (err)
- goto out_master;
+ goto out_lpt;
}
- err = ubifs_lpt_init(c, 1, !c->ro_mount);
- if (err)
- goto out_lpt;
-
err = dbg_check_idx_size(c, c->bi.old_idx_sz);
if (err)
goto out_lpt;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] UBIFS: intialize the LPT earlier
2011-05-30 18:56 ` [PATCH 2/3] UBIFS: intialize the LPT earlier Ben Gardiner
@ 2011-06-01 10:06 ` Artem Bityutskiy
0 siblings, 0 replies; 17+ messages in thread
From: Artem Bityutskiy @ 2011-06-01 10:06 UTC (permalink / raw)
To: Ben Gardiner; +Cc: linux-mtd, linux-kernel, Matthew L. Creech
Hi,
thanks for the patch, however
On Mon, 2011-05-30 at 14:56 -0400, Ben Gardiner wrote:
> + err = ubifs_lpt_init(c, 1, !c->ro_mount);
> + if (err)
> + goto out_lpt;
You cannot call ubifs_lpt_init()
> +
> if ((c->mst_node->flags & cpu_to_le32(UBIFS_MST_DIRTY)) != 0) {
> ubifs_msg("recovery needed");
> c->need_recovery = 1;
> if (!c->ro_mount) {
> err = ubifs_recover_inl_heads(c, c->sbuf);
Before ubifs_recover_inl_heads() is called in case of dirty FS.
I've massaged your patch and pushed the following analogous patch to the
ubifs tree, please check/test:
>From 678ef9ad3daa453723f04f84f9db6a22994e7343 Mon Sep 17 00:00:00 2001
From: Ben Gardiner <bengardiner@nanometrics.ca>
Date: Mon, 30 May 2011 14:56:15 -0400
Subject: [PATCH] UBIFS: intialize LPT earlier
The current 'mount_ubifs()' implementation does not initialize the LPT until the
the master node is marked dirty. Move the LPT initialization to before marking
the master node dirty. This is a preparation for the next patch which will move
the free-space-fixup check to before marking the master node dirty, because we
have to fix-up the free space before doing any writes.
Artem: massaged the patch and commit message.
Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
fs/ubifs/super.c | 29 ++++++++++++++++-------------
1 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 1e40db7..6d357fd 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1282,17 +1282,24 @@ static int mount_ubifs(struct ubifs_info *c)
if (err)
goto out_master;
- init_constants_master(c);
-
if ((c->mst_node->flags & cpu_to_le32(UBIFS_MST_DIRTY)) != 0) {
ubifs_msg("recovery needed");
c->need_recovery = 1;
- if (!c->ro_mount) {
- err = ubifs_recover_inl_heads(c, c->sbuf);
- if (err)
- goto out_master;
- }
- } else if (!c->ro_mount) {
+ }
+
+ init_constants_master(c);
+
+ if (c->need_recovery && !c->ro_mount) {
+ err = ubifs_recover_inl_heads(c, c->sbuf);
+ if (err)
+ goto out_master;
+ }
+
+ err = ubifs_lpt_init(c, 1, !c->ro_mount);
+ if (err)
+ goto out_master;
+
+ if (!c->ro_mount) {
/*
* Set the "dirty" flag so that if we reboot uncleanly we
* will notice this immediately on the next mount.
@@ -1300,13 +1307,9 @@ static int mount_ubifs(struct ubifs_info *c)
c->mst_node->flags |= cpu_to_le32(UBIFS_MST_DIRTY);
err = ubifs_write_master(c);
if (err)
- goto out_master;
+ goto out_lpt;
}
- err = ubifs_lpt_init(c, 1, !c->ro_mount);
- if (err)
- goto out_lpt;
-
err = dbg_check_idx_size(c, c->bi.old_idx_sz);
if (err)
goto out_lpt;
--
1.7.2.3
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/3] UBIFS: fix-up free space earlier
2011-05-30 18:56 [PATCH 0/3] fix-up free space earlier in mount_ubifs() Ben Gardiner
2011-05-30 18:56 ` [PATCH 1/3] UBIFS: assert no fixup when writing a node Ben Gardiner
2011-05-30 18:56 ` [PATCH 2/3] UBIFS: intialize the LPT earlier Ben Gardiner
@ 2011-05-30 18:56 ` Ben Gardiner
2011-06-01 10:16 ` Artem Bityutskiy
2011-05-31 14:43 ` [PATCH 0/3] fix-up free space earlier in mount_ubifs() Matthew L. Creech
2011-06-01 10:19 ` Artem Bityutskiy
4 siblings, 1 reply; 17+ messages in thread
From: Ben Gardiner @ 2011-05-30 18:56 UTC (permalink / raw)
To: Artem Bityutskiy, Adrian Hunter
Cc: linux-mtd, linux-kernel, Matthew L. Creech
The free space fixup is currently initiated during mount after the call to
ubifs_write_master() which results in a write to PEBs; this has been observed
with the patch 'assert no fixup when writing a node' applied:
Move the free space fixup on mount to before the calls to
ubifs_recover_inl_heads() and ubifs_write_master(). This results in no
assertions with the previously mentioned patch applied.
Signed-off-by: Ben Gardiner <bengardiner@nanometrics>
CC: Matthew L. Creech <mlcreech@gmail.com>
---
fs/ubifs/super.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 89c8d33..531b9b7 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1285,6 +1285,12 @@ static int mount_ubifs(struct ubifs_info *c)
if (err)
goto out_lpt;
+ if (!c->ro_mount && c->space_fixup) {
+ err = ubifs_fixup_free_space(c);
+ if (err)
+ goto out_infos;
+ }
+
if ((c->mst_node->flags & cpu_to_le32(UBIFS_MST_DIRTY)) != 0) {
ubifs_msg("recovery needed");
c->need_recovery = 1;
@@ -1396,12 +1402,6 @@ static int mount_ubifs(struct ubifs_info *c)
} else
ubifs_assert(c->lst.taken_empty_lebs > 0);
- if (!c->ro_mount && c->space_fixup) {
- err = ubifs_fixup_free_space(c);
- if (err)
- goto out_infos;
- }
-
err = dbg_check_filesystem(c);
if (err)
goto out_infos;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] UBIFS: fix-up free space earlier
2011-05-30 18:56 ` [PATCH 3/3] UBIFS: fix-up free space earlier Ben Gardiner
@ 2011-06-01 10:16 ` Artem Bityutskiy
2011-06-01 14:53 ` [PATCH] UBIFS: fixup merged 'UBIFS: fix-up free space earlier' Ben Gardiner
0 siblings, 1 reply; 17+ messages in thread
From: Artem Bityutskiy @ 2011-06-01 10:16 UTC (permalink / raw)
To: Ben Gardiner; +Cc: linux-mtd, linux-kernel, Matthew L. Creech
On Mon, 2011-05-30 at 14:56 -0400, Ben Gardiner wrote:
> The free space fixup is currently initiated during mount after the call to
> ubifs_write_master() which results in a write to PEBs; this has been observed
> with the patch 'assert no fixup when writing a node' applied:
>
> Move the free space fixup on mount to before the calls to
> ubifs_recover_inl_heads() and ubifs_write_master(). This results in no
> assertions with the previously mentioned patch applied.
>
> Signed-off-by: Ben Gardiner <bengardiner@nanometrics>
> CC: Matthew L. Creech <mlcreech@gmail.com>
>
> ---
> fs/ubifs/super.c | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 89c8d33..531b9b7 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1285,6 +1285,12 @@ static int mount_ubifs(struct ubifs_info *c)
> if (err)
> goto out_lpt;
>
> + if (!c->ro_mount && c->space_fixup) {
> + err = ubifs_fixup_free_space(c);
> + if (err)
> + goto out_infos;
> + }
Tweaked the patch a bit to make sure 'init_constants_master()' is called
before we start fixing-up - this does not matter now but it is less
error prone. And pushed to the UBIFS git tree, thank you.
But please, do review my tweaks and test them. When you confirm it
works, I'll send a pull request to Linus.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH] UBIFS: fixup merged 'UBIFS: fix-up free space earlier'
2011-06-01 10:16 ` Artem Bityutskiy
@ 2011-06-01 14:53 ` Ben Gardiner
2011-06-01 15:03 ` Artem Bityutskiy
2011-06-01 15:14 ` Artem Bityutskiy
0 siblings, 2 replies; 17+ messages in thread
From: Ben Gardiner @ 2011-06-01 14:53 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: linux-mtd, linux-kernel, Matthew L. Creech
When the patch 'UBIFS: intialize LPT earlier' was massaged before being applied
the context of the next patch 'UBIFS: fix-up free space earlier' was muddled
sufficiently for the order of allocate-lpt then free-space-fixup was reversed --
I think.
Put the free-space-fixup after allocate-lpt.
Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
---
With this fixup applied on-top-of the ubifs-2.6 master I am able to mount a
ubinized image containing a UBIFS volume which has the free-space-fixup flag
set. No assertions are displayed.
---
fs/ubifs/super.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 4722f31..70e8c92 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1284,12 +1284,6 @@ static int mount_ubifs(struct ubifs_info *c)
init_constants_master(c);
- if (!c->ro_mount && c->space_fixup) {
- err = ubifs_fixup_free_space(c);
- if (err)
- goto out_infos;
- }
-
if ((c->mst_node->flags & cpu_to_le32(UBIFS_MST_DIRTY)) != 0) {
ubifs_msg("recovery needed");
c->need_recovery = 1;
@@ -1305,6 +1299,12 @@ static int mount_ubifs(struct ubifs_info *c)
if (err)
goto out_master;
+ if (!c->ro_mount && c->space_fixup) {
+ err = ubifs_fixup_free_space(c);
+ if (err)
+ goto out_master;
+ }
+
if (!c->ro_mount) {
/*
* Set the "dirty" flag so that if we reboot uncleanly we
--
1.7.4.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH] UBIFS: fixup merged 'UBIFS: fix-up free space earlier'
2011-06-01 14:53 ` [PATCH] UBIFS: fixup merged 'UBIFS: fix-up free space earlier' Ben Gardiner
@ 2011-06-01 15:03 ` Artem Bityutskiy
2011-06-01 15:14 ` Artem Bityutskiy
1 sibling, 0 replies; 17+ messages in thread
From: Artem Bityutskiy @ 2011-06-01 15:03 UTC (permalink / raw)
To: Ben Gardiner; +Cc: linux-mtd, linux-kernel, Matthew L. Creech
On Wed, 2011-06-01 at 10:53 -0400, Ben Gardiner wrote:
> When the patch 'UBIFS: intialize LPT earlier' was massaged before being applied
> the context of the next patch 'UBIFS: fix-up free space earlier' was muddled
> sufficiently for the order of allocate-lpt then free-space-fixup was reversed --
> I think.
>
> Put the free-space-fixup after allocate-lpt.
>
> Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
I'll take a look at this and fold this change in a bit later, thanks.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] UBIFS: fixup merged 'UBIFS: fix-up free space earlier'
2011-06-01 14:53 ` [PATCH] UBIFS: fixup merged 'UBIFS: fix-up free space earlier' Ben Gardiner
2011-06-01 15:03 ` Artem Bityutskiy
@ 2011-06-01 15:14 ` Artem Bityutskiy
2011-06-01 15:28 ` Ben Gardiner
1 sibling, 1 reply; 17+ messages in thread
From: Artem Bityutskiy @ 2011-06-01 15:14 UTC (permalink / raw)
To: Ben Gardiner; +Cc: linux-mtd, linux-kernel, Matthew L. Creech
On Wed, 2011-06-01 at 10:53 -0400, Ben Gardiner wrote:
> When the patch 'UBIFS: intialize LPT earlier' was massaged before being applied
> the context of the next patch 'UBIFS: fix-up free space earlier' was muddled
> sufficiently for the order of allocate-lpt then free-space-fixup was reversed --
> I think.
>
> Put the free-space-fixup after allocate-lpt.
>
> Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
>
> ---
> With this fixup applied on-top-of the ubifs-2.6 master I am able to mount a
> ubinized image containing a UBIFS volume which has the free-space-fixup flag
> set. No assertions are displayed.
Thanks. Merged this patch to the 3rd one and pushed out.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] UBIFS: fixup merged 'UBIFS: fix-up free space earlier'
2011-06-01 15:14 ` Artem Bityutskiy
@ 2011-06-01 15:28 ` Ben Gardiner
0 siblings, 0 replies; 17+ messages in thread
From: Ben Gardiner @ 2011-06-01 15:28 UTC (permalink / raw)
To: dedekind1; +Cc: linux-mtd, linux-kernel, Matthew L. Creech
Hi Artem,
On Wed, Jun 1, 2011 at 11:14 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Wed, 2011-06-01 at 10:53 -0400, Ben Gardiner wrote:
>> When the patch 'UBIFS: intialize LPT earlier' was massaged before being applied
>> the context of the next patch 'UBIFS: fix-up free space earlier' was muddled
>> sufficiently for the order of allocate-lpt then free-space-fixup was reversed --
>> I think.
>>
>> Put the free-space-fixup after allocate-lpt.
>>
>> Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
>>
>> ---
>> With this fixup applied on-top-of the ubifs-2.6 master I am able to mount a
>> ubinized image containing a UBIFS volume which has the free-space-fixup flag
>> set. No assertions are displayed.
>
> Thanks. Merged this patch to the 3rd one and pushed out.
You're most welcome.
Both branches master and linux-next are working as expected.
Best Regards,
Ben Gardiner
---
Nanometrics Inc.
http://www.nanometrics.ca
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] fix-up free space earlier in mount_ubifs()
2011-05-30 18:56 [PATCH 0/3] fix-up free space earlier in mount_ubifs() Ben Gardiner
` (2 preceding siblings ...)
2011-05-30 18:56 ` [PATCH 3/3] UBIFS: fix-up free space earlier Ben Gardiner
@ 2011-05-31 14:43 ` Matthew L. Creech
2011-05-31 14:52 ` Ben Gardiner
2011-05-31 15:37 ` Ben Gardiner
2011-06-01 10:19 ` Artem Bityutskiy
4 siblings, 2 replies; 17+ messages in thread
From: Matthew L. Creech @ 2011-05-31 14:43 UTC (permalink / raw)
To: Ben Gardiner; +Cc: Artem Bityutskiy, Adrian Hunter, linux-mtd, linux-kernel
On Mon, May 30, 2011 at 2:56 PM, Ben Gardiner
<bengardiner@nanometrics.ca> wrote:
> In testing Mattew Creech's free-space-fixup flag series I found that was unable
> to boot a da850evm which had flashed to it's NAND a ubinized image containing a
> UBIFS that has the free-space-fixup flag set.
>
> The cause of the problem was found to be the call to ubifs_write_master() from
> mount_ubifs() as is evidenced the backtrace produced by the assertion
> introduced in the first patch of this series; where the assertion introduced is
> that c->space_fixup is false when ubifs_write_node() is called.
>
Interesting - so the problem is that if ubifs_read_master() resizes
the master node, a subsequent attempt to read the first (c->mst_offs +
c->mst_node_alsz) bytes from the master LEB fails?
I wonder why this is the case when free-space fixup is enabled, and
not otherwise? The -EBADMSG seems to imply that this is the original
problem that the fix-up is intended to solve - i.e. the master node
has empty pages with non-empty OOB values, and writing to them results
in a bogus ECC.
Do you happen to know if the same thing occurs when you uncleanly
reboot (so that recovery is needed on the next boot)? It looks like
ubifs_recover_master_node() reads the same data that
fixup_leb_free_space() does, so I'd expect it to fail similarly if
that's what's happening, regardless of whether fix-up is performed or
not.
Just trying to fully understand the error. :) That aside, this patch
set makes sense to me. Thanks!
--
Matthew L. Creech
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 0/3] fix-up free space earlier in mount_ubifs()
2011-05-31 14:43 ` [PATCH 0/3] fix-up free space earlier in mount_ubifs() Matthew L. Creech
@ 2011-05-31 14:52 ` Ben Gardiner
2011-05-31 15:22 ` Matthew L. Creech
2011-05-31 15:37 ` Ben Gardiner
1 sibling, 1 reply; 17+ messages in thread
From: Ben Gardiner @ 2011-05-31 14:52 UTC (permalink / raw)
To: Matthew L. Creech
Cc: Artem Bityutskiy, Adrian Hunter, linux-mtd, linux-kernel
Hi Matthew,
On Tue, May 31, 2011 at 10:43 AM, Matthew L. Creech <mlcreech@gmail.com> wrote:
> On Mon, May 30, 2011 at 2:56 PM, Ben Gardiner
> <bengardiner@nanometrics.ca> wrote:
>> In testing Mattew Creech's free-space-fixup flag series I found that was unable
>> to boot a da850evm which had flashed to it's NAND a ubinized image containing a
>> UBIFS that has the free-space-fixup flag set.
>>
>> The cause of the problem was found to be the call to ubifs_write_master() from
>> mount_ubifs() as is evidenced the backtrace produced by the assertion
>> introduced in the first patch of this series; where the assertion introduced is
>> that c->space_fixup is false when ubifs_write_node() is called.
>>
>
> Interesting - so the problem is that if ubifs_read_master() resizes
> the master node, a subsequent attempt to read the first (c->mst_offs +
> c->mst_node_alsz) bytes from the master LEB fails?
>
> I wonder why this is the case when free-space fixup is enabled, and
> not otherwise? The -EBADMSG seems to imply that this is the original
> problem that the fix-up is intended to solve - i.e. the master node
> has empty pages with non-empty OOB values, and writing to them results
> in a bogus ECC.
Right. I should have mentioned that it is also true that without
free-space-fixup the initial flash of a UBInized image containing a
UBIFS volume results in a bootable system which can mount the rootfs
_the first time only_ subsequent attemps at mounting result in a
failure to mount due to -74 errors.
> Just trying to fully understand the error. :) That aside, this patch
> set makes sense to me. Thanks!
Thank you for your endorsement. Can we take that as a Reviewed-by ?
Best Regards,
Ben Gardiner
---
Nanometrics Inc.
http://www.nanometrics.ca
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] fix-up free space earlier in mount_ubifs()
2011-05-31 14:43 ` [PATCH 0/3] fix-up free space earlier in mount_ubifs() Matthew L. Creech
2011-05-31 14:52 ` Ben Gardiner
@ 2011-05-31 15:37 ` Ben Gardiner
1 sibling, 0 replies; 17+ messages in thread
From: Ben Gardiner @ 2011-05-31 15:37 UTC (permalink / raw)
To: Matthew L. Creech
Cc: Artem Bityutskiy, Adrian Hunter, linux-mtd, linux-kernel
Hi Matthew,
To answer your other question:
On Tue, May 31, 2011 at 10:43 AM, Matthew L. Creech <mlcreech@gmail.com> wrote:
> [...]
>
> Do you happen to know if the same thing occurs when you uncleanly
> reboot (so that recovery is needed on the next boot)? It looks like
> ubifs_recover_master_node() reads the same data that
> fixup_leb_free_space() does, so I'd expect it to fail similarly if
> that's what's happening, regardless of whether fix-up is performed or
> not.
To test unclean w/o free-space-fixup flag set I did a powercut during
'yes > /test-file';
The first time I hsaw a dump from ubifs_check_node:
UBIFS: recovery needed
UBIFS error (pid 1): ubifs_check_node: bad CRC: calculated 0x4763322d,
read 0x4c8c2537
UBIFS error (pid 1): ubifs_check_node: bad node at LEB 501:81608
Backtrace:
[<c002dd8c>] (dump_backtrace+0x0/0x10c) from [<c02ecbac>] (dump_stack+0x18/0x1c)
r7:00013ec8 r6:c781f000 r5:c89f0ec8 r4:ffffff8b
[<c02ecb94>] (dump_stack+0x0/0x1c) from [<c015ba88>]
(ubifs_check_node+0x1b8/0x2ec)
[<c015b8d0>] (ubifs_check_node+0x0/0x2ec) from [<c0163ee4>]
(ubifs_scan_a_node+0x180/0x398)
[<c0163d64>] (ubifs_scan_a_node+0x0/0x398) from [<c017b498>]
(ubifs_recover_leb+0xd4/0xa94)
[<c017b3c4>] (ubifs_recover_leb+0x0/0xa94) from [<c0164e90>]
(ubifs_replay_journal+0x6ac/0x1c30)
[<c01647e4>] (ubifs_replay_journal+0x0/0x1c30) from [<c01571a8>]
(ubifs_fill_super+0xdf4/0x1ee4)
[<c01563b4>] (ubifs_fill_super+0x0/0x1ee4) from [<c0158508>]
(ubifs_mount+0x270/0x3a4)
[<c0158298>] (ubifs_mount+0x0/0x3a4) from [<c00afe5c>] (mount_fs+0x1c/0xe8)
[<c00afe40>] (mount_fs+0x0/0xe8) from [<c00cc8c0>] (vfs_kern_mount+0x58/0x94)
r6:00008000 r5:c780c780 r4:c7a78b80
[<c00cc868>] (vfs_kern_mount+0x0/0x94) from [<c00cc958>]
(do_kern_mount+0x3c/0xd4)
r9:c782dee8 r8:c03ebaa8 r7:c7a78b60 r6:00000000 r5:c7a78b80
r4:00008000
[<c00cc91c>] (do_kern_mount+0x0/0xd4) from [<c00ccb3c>] (do_mount+0x14c/0x734)
r9:c782dee8 r8:00000020 r7:c7a78b60 r6:c7a78b80 r5:00008000
r4:00000000
[<c00cc9f0>] (do_mount+0x0/0x734) from [<c00cd1c4>] (sys_mount+0xa0/0xd0)
[<c00cd124>] (sys_mount+0x0/0xd0) from [<c0008dd4>]
(mount_block_root+0x98/0x2c8)
r7:c0023e88 r6:c781b000 r5:00008000 r4:c781b000
[<c0008d3c>] (mount_block_root+0x0/0x2c8) from [<c00090f0>]
(prepare_namespace+0x80/0x1c4)
[<c0009070>] (prepare_namespace+0x0/0x1c4) from [<c0008480>]
(kernel_init+0x110/0x158)
r7:00000013 r6:c0023300 r5:c0023668 r4:c0023668
[<c0008370>] (kernel_init+0x0/0x158) from [<c0043d58>] (do_exit+0x0/0x750)
r7:00000013 r6:c0043d58 r5:c0008370 r4:00000000
UBIFS: recovery completed
every time after that there is nothing printed during recovery:
UBIFS: recovery needed
UBIFS: recovery completed
UBIFS: mounted UBI device 0, volume 1, name "rootfs"
To test unclean with free-space-flag set, I did powercut during 'nand
write' in u-boot.
When attempting to mount the rootfs I got dumps from ubifs_read_node:
UBIFS error (pid 1): ubifs_read_node: bad node type (255 but expected 9)
UBIFS error (pid 1): ubifs_read_node: bad node at LEB 496:101088, LEB
mapping status 0
Backtrace:
[<c002dd8c>] (dump_backtrace+0x0/0x10c) from [<c02ecbac>] (dump_stack+0x18/0x1c)
r7:c782c000 r6:c781f000 r5:c7859780 r4:ffffffea
[<c02ecb94>] (dump_stack+0x0/0x1c) from [<c015bdf4>]
(ubifs_read_node+0x238/0x2ec)
[<c015bbbc>] (ubifs_read_node+0x0/0x2ec) from [<c0168080>]
(dbg_old_index_check_init+0x70/0xe0)
[<c0168010>] (dbg_old_index_check_init+0x0/0xe0) from [<c0163864>]
(ubifs_read_master+0xdbc/0xe20)
[<c0162aa8>] (ubifs_read_master+0x0/0xe20) from [<c0156ffc>]
(ubifs_fill_super+0xc48/0x1ee4)
[<c01563b4>] (ubifs_fill_super+0x0/0x1ee4) from [<c0158508>]
(ubifs_mount+0x270/0x3a4)
[<c0158298>] (ubifs_mount+0x0/0x3a4) from [<c00afe5c>] (mount_fs+0x1c/0xe8)
[<c00afe40>] (mount_fs+0x0/0xe8) from [<c00cc8c0>] (vfs_kern_mount+0x58/0x94)
r6:00008000 r5:c780c780 r4:c7a848e0
[<c00cc868>] (vfs_kern_mount+0x0/0x94) from [<c00cc958>]
(do_kern_mount+0x3c/0xd4)
r9:c782dee8 r8:c03ebaa8 r7:c7a848c0 r6:00000000 r5:c7a848e0
r4:00008000
[<c00cc91c>] (do_kern_mount+0x0/0xd4) from [<c00ccb3c>] (do_mount+0x14c/0x734)
r9:c782dee8 r8:00000020 r7:c7a848c0 r6:c7a848e0 r5:00008000
r4:00000000
[<c00cc9f0>] (do_mount+0x0/0x734) from [<c00cd1c4>] (sys_mount+0xa0/0xd0)
[<c00cd124>] (sys_mount+0x0/0xd0) from [<c0008dd4>]
(mount_block_root+0x98/0x2c8)
r7:c0023e88 r6:c781b000 r5:00008000 r4:c781b000
[<c0008d3c>] (mount_block_root+0x0/0x2c8) from [<c00090f0>]
(prepare_namespace+0x80/0x1c4)
[<c0009070>] (prepare_namespace+0x0/0x1c4) from [<c0008480>]
(kernel_init+0x110/0x158)
r7:00000013 r6:c0023300 r5:c0023668 r4:c0023668
[<c0008370>] (kernel_init+0x0/0x158) from [<c0043d58>] (do_exit+0x0/0x750)
r7:00000013 r6:c0043d58 r5:c0008370 r4:00000000
I didn't ever see the -74 errors or the assertion I placed in
ubifs_write_node().
Best Regards,
Ben Gardiner
---
Nanometrics Inc.
http://www.nanometrics.ca
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] fix-up free space earlier in mount_ubifs()
2011-05-30 18:56 [PATCH 0/3] fix-up free space earlier in mount_ubifs() Ben Gardiner
` (3 preceding siblings ...)
2011-05-31 14:43 ` [PATCH 0/3] fix-up free space earlier in mount_ubifs() Matthew L. Creech
@ 2011-06-01 10:19 ` Artem Bityutskiy
4 siblings, 0 replies; 17+ messages in thread
From: Artem Bityutskiy @ 2011-06-01 10:19 UTC (permalink / raw)
To: Ben Gardiner; +Cc: linux-mtd, linux-kernel, Matthew L. Creech
On Mon, 2011-05-30 at 14:56 -0400, Ben Gardiner wrote:
> In testing Mattew Creech's free-space-fixup flag series I found that was unable
> to boot a da850evm which had flashed to it's NAND a ubinized image containing a
> UBIFS that has the free-space-fixup flag set.
>
> The cause of the problem was found to be the call to ubifs_write_master() from
> mount_ubifs() as is evidenced the backtrace produced by the assertion
> introduced in the first patch of this series; where the assertion introduced is
> that c->space_fixup is false when ubifs_write_node() is called.
So the fixes are now in the ubifs tree. You can see them in the
linux-next banch's tip or in the master branch under a pile of other
changes. The point is that linux-next currently contains what I want to
merge to linux-3.0 and master branch contains the same plus a pile of
things for linux-3.1.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 17+ messages in thread