Re: [PATCH] UBIFS: fix master node recovery

[Anatolij Gustschin <agust@denx.de>]

On Thu, 07 Jul 2011 09:26:26 +0300
Artem Bityutskiy <dedekind1@gmail.com> wrote:

> On Wed, 2011-07-06 at 11:30 +0200, Anatolij Gustschin wrote:
> > When the 1st LEB was unmapped and written but 2nd LEB not,
> > the master node recovery doesn't succeed after power cut.
> > We see following error when mounting UBIFS partition on NOR
> > flash:
> > 
> > UBIFS error (pid 1137): ubifs_recover_master_node: failed to recover master node
> > 
> > Additional 2nd master node offset check is needed to fix the
> > problem. If the 2nd master node is at the end in the 2nd LEB,
> > first master node is used for recovery.
> > 
> > Signed-off-by: Anatolij Gustschin <agust@denx.de>
> > ---
> >  fs/ubifs/recovery.c |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> > 
> > diff --git a/fs/ubifs/recovery.c b/fs/ubifs/recovery.c
> > index 783d8e0..0e951f0 100644
> > --- a/fs/ubifs/recovery.c
> > +++ b/fs/ubifs/recovery.c
> > @@ -274,7 +274,9 @@ int ubifs_recover_master_node(struct ubifs_info *c)
> >  				if (cor1)
> >  					goto out_err;
> >  				mst = mst1;
> > -			} else if (offs1 == 0 && offs2 + sz >= c->leb_size) {
> > +			} else if (offs1 == 0 &&
> > +				   (offs2 + sz >= c->leb_size ||
> > +				    (c->leb_size - offs2 - sz) < sz)) {
> 
> Hmm, ok, so we have situation like this I guess:
> 
> First LEB:   |M_______|
> Second LEB:  |MMMMMMM_|
>               1234567      
> 
> Where M - valid master node, _ - just free space.
> 
> So offs1 is 0, and offs2 is the position where the 7th M _starts_.

Yes, in our case we have 511 valid master nodes + 384 bytes free
space in LEB 2 and one valid master node in LEB 1 + free space.

> What we want to check is that there is no room for another master node
> in the second LEB. We have to take offs2, add sz, and make sure that LEB
> size minus that is less than sz, i.e., exactly what you have done.
> 
> And  offs2 + sz >= c->leb_size seems to be completely incorrect and
> should be removed, AFAICS. Can you confirm that?

Yes, offs2 + sz >= c->leb_size check is not correct, I think.

Anatolij