Re: [PATCH 1/4] f2fs: fix the recovery flow to handle errors correctly

[Jaegeuk Kim]

2013-03-25 (ì), 15:30 +0900, Namjae Jeon:
> 2013/3/25, Jaegeuk Kim <jaegeuk.kim@xxxxxxxxxxx>:
> > We should handle errors during the recovery flow correctly.
> > For example, if we get -ENOMEM, we should report a mount failure instead of
> > conducting the remained mount procedure.
> >
> > Signed-off-by: Jaegeuk Kim <jaegeuk.kim@xxxxxxxxxxx>
> > ---
> >  fs/f2fs/f2fs.h     |  2 +-
> >  fs/f2fs/recovery.c | 46 ++++++++++++++++++++++++++++------------------
> >  fs/f2fs/super.c    |  9 +++++++--
> >  3 files changed, 36 insertions(+), 21 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 5bb87e0..109e12d 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -1027,7 +1027,7 @@ void destroy_gc_caches(void);
> >  /*
> >   * recovery.c
> >   */
> > -void recover_fsync_data(struct f2fs_sb_info *);
> > +int recover_fsync_data(struct f2fs_sb_info *);
> >  bool space_for_roll_forward(struct f2fs_sb_info *);
> >
> >  /*
> > diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> > index 2d86eb2..61bdaa7 100644
> > --- a/fs/f2fs/recovery.c
> > +++ b/fs/f2fs/recovery.c
> > @@ -118,10 +118,8 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi,
> > struct list_head *head)
> >
> >  		lock_page(page);
> >
> Hi Jaegeuk.
> I have a question.
> > -		if (cp_ver != cpver_of_node(page)) {
> > -			err = -EINVAL;
> > +		if (cp_ver != cpver_of_node(page))
> >  			goto unlock_out;
> > -		}
> err = 0 is initialized to zero in the start of function
> Why have you remove err = -EINVAL; code when mismatching cp_ver ?

This ending condition is used to find the latest node pages that we have
to recover, not to detect an error to exit the recovery routine.
For example, the error conditions include -ENOMEM or -EIO, something
like such the obvious errors.
Thanks,

> 
> Thanks.
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Jaegeuk Kim
Samsung

Attachment: signature.asc
Description: This is a digitally signed message part