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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-id: <1364197791.29882.5.camel@kjgkr>
Date:	Mon, 25 Mar 2013 16:49:51 +0900
From:	Jaegeuk Kim <jaegeuk.kim@...sung.com>
To:	Namjae Jeon <linkinjeon@...il.com>
Cc:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [PATCH 1/4] f2fs: fix the recovery flow to handle errors correctly

2013-03-25 (월), 15:30 +0900, Namjae Jeon:
> 2013/3/25, Jaegeuk Kim <jaegeuk.kim@...sung.com>:
> > 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@...sung.com>
> > ---
> >  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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Jaegeuk Kim
Samsung

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)