[PATCH 1/3] f2fs: flush symlink path to avoid broken symlink after POR

[PATCH 1/3] f2fs: flush symlink path to avoid broken symlink after POR

[Jaegeuk Kim]

This patch tries to avoid broken symlink case after POR in best effort.
This results in performance regression.
But, if f2fs has inline_data and the target path is under 3KB-sized long,
the page would be stored in its inode_block, so that there would be no
performance regression.

Note that, if user wants to keep this file atomically, it needs to trigger
dir->fsync.
And, there is still a hole to produce broken symlink.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/namei.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 8055e30..5d990d8 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -324,6 +324,17 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 	d_instantiate(dentry, inode);
 	unlock_new_inode(inode);
 
+	/*
+	 * Let's flush symlink data in order to avoid broken symlink as much as
+	 * possible. Nevertheless, fsyncing is the best way, but there is no
+	 * way to get a file descriptor in order to flush that.
+	 *
+	 * Note that, it needs to do dir->fsync to make this recoverable.
+	 * If the symlink path is stored into inline_data, there is no
+	 * performance regression.
+	 */
+	filemap_write_and_wait_range(inode->i_mapping, 0, symlen);
+
 	if (IS_DIRSYNC(dir))
 		f2fs_sync_fs(sbi->sb, 1);
 	return err;
-- 
2.1.1

[PATCH 2/3] f2fs: avoid abnormal behavior on broken symlink

[Jaegeuk Kim]

When f2fs_symlink was triggered and checkpoint was done before syncing its
link path, f2fs can get broken symlink like "xxx -> \0\0\0".
This incurs abnormal path_walk by VFS.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/namei.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 5d990d8..6cfd954 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -14,6 +14,7 @@
 #include <linux/sched.h>
 #include <linux/ctype.h>
 #include <linux/dcache.h>
+#include <linux/namei.h>
 
 #include "f2fs.h"
 #include "node.h"
@@ -295,6 +296,23 @@ fail:
 	return err;
 }
 
+static void *f2fs_follow_link(struct dentry *dentry, struct nameidata *nd)
+{
+	struct page *page;
+
+	page = page_follow_link_light(dentry, nd);
+	if (IS_ERR(page))
+		return page;
+
+	/* this is broken symlink case */
+	if (*nd_get_link(nd) == 0) {
+		kunmap(page);
+		page_cache_release(page);
+		return ERR_PTR(-ENOENT);
+	}
+	return page;
+}
+
 static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 					const char *symname)
 {
@@ -790,7 +808,7 @@ const struct inode_operations f2fs_dir_inode_operations = {
 
 const struct inode_operations f2fs_symlink_inode_operations = {
 	.readlink       = generic_readlink,
-	.follow_link    = page_follow_link_light,
+	.follow_link    = f2fs_follow_link,
 	.put_link       = page_put_link,
 	.getattr	= f2fs_getattr,
 	.setattr	= f2fs_setattr,
-- 
2.1.1


------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF

[PATCH 3/3] f2fs: pass checkpoint reason on roll-forward recovery

[Jaegeuk Kim]

This patch adds CP_RECOVERY to remain recovery information for checkpoint.
And, it makes sure writing checkpoint in this case.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/checkpoint.c        | 6 +++++-
 fs/f2fs/f2fs.h              | 1 +
 fs/f2fs/recovery.c          | 2 +-
 include/trace/events/f2fs.h | 1 +
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 384bfc4..a5e17a2 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1051,7 +1051,7 @@ void write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 	mutex_lock(&sbi->cp_mutex);
 
 	if (!is_sbi_flag_set(sbi, SBI_IS_DIRTY) &&
-			cpc->reason != CP_DISCARD && cpc->reason != CP_UMOUNT)
+		(cpc->reason == CP_FASTBOOT || cpc->reason == CP_SYNC))
 		goto out;
 	if (unlikely(f2fs_cp_error(sbi)))
 		goto out;
@@ -1086,6 +1086,10 @@ void write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 
 	unblock_operations(sbi);
 	stat_inc_cp_count(sbi->stat_info);
+
+	if (cpc->reason == CP_RECOVERY)
+		f2fs_msg(sbi->sb, KERN_NOTICE,
+			"checkpoint: version = %llx", ckpt_ver);
 out:
 	mutex_unlock(&sbi->cp_mutex);
 	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, "finish checkpoint");
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 053361a..c06a25e 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -103,6 +103,7 @@ enum {
 	CP_UMOUNT,
 	CP_FASTBOOT,
 	CP_SYNC,
+	CP_RECOVERY,
 	CP_DISCARD,
 };
 
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 4b742c9..8d8ea99 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -564,7 +564,7 @@ out:
 		mutex_unlock(&sbi->cp_mutex);
 	} else if (need_writecp) {
 		struct cp_control cpc = {
-			.reason = CP_SYNC,
+			.reason = CP_RECOVERY,
 		};
 		mutex_unlock(&sbi->cp_mutex);
 		write_checkpoint(sbi, &cpc);
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index 75724bd..8804f22 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -78,6 +78,7 @@
 		{ CP_UMOUNT,	"Umount" },				\
 		{ CP_FASTBOOT,	"Fastboot" },				\
 		{ CP_SYNC,	"Sync" },				\
+		{ CP_RECOVERY,	"Recovery" },				\
 		{ CP_DISCARD,	"Discard" })
 
 struct victim_sel_policy;
-- 
2.1.1


------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF

RE: [f2fs-dev] [PATCH 1/3] f2fs: flush symlink path to avoid broken symlink after POR

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Thursday, April 16, 2015 3:31 PM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 1/3] f2fs: flush symlink path to avoid broken symlink after POR
> 
> This patch tries to avoid broken symlink case after POR in best effort.
> This results in performance regression.
> But, if f2fs has inline_data and the target path is under 3KB-sized long,
> the page would be stored in its inode_block, so that there would be no
> performance regression.
> 
> Note that, if user wants to keep this file atomically, it needs to trigger
> dir->fsync.
> And, there is still a hole to produce broken symlink.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/namei.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index 8055e30..5d990d8 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -324,6 +324,17 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
>  	d_instantiate(dentry, inode);
>  	unlock_new_inode(inode);
> 
> +	/*
> +	 * Let's flush symlink data in order to avoid broken symlink as much as
> +	 * possible. Nevertheless, fsyncing is the best way, but there is no
> +	 * way to get a file descriptor in order to flush that.
> +	 *
> +	 * Note that, it needs to do dir->fsync to make this recoverable.
> +	 * If the symlink path is stored into inline_data, there is no
> +	 * performance regression.
> +	 */
> +	filemap_write_and_wait_range(inode->i_mapping, 0, symlen);

One minor thing.

filemap_write_and_wait_range(inode->i_mapping, 0, symlen - 1);

Because we don't need to write out data exceed isize here.

Thanks,

> +
>  	if (IS_DIRSYNC(dir))
>  		f2fs_sync_fs(sbi->sb, 1);
>  	return err;
> --
> 2.1.1
> 
> 
> ------------------------------------------------------------------------------
> BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
> Develop your own process in accordance with the BPMN 2 standard
> Learn Process modeling best practices with Bonita BPM through live exercises
> http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
> source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 1/3] f2fs: flush symlink path to avoid broken symlink after POR

[Jaegeuk Kim]

Hi Chao,

On Thu, Apr 16, 2015 at 06:33:29PM +0800, Chao Yu wrote:
> Hi Jaegeuk,
> 
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> > Sent: Thursday, April 16, 2015 3:31 PM
> > To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> > linux-f2fs-devel@lists.sourceforge.net
> > Cc: Jaegeuk Kim
> > Subject: [f2fs-dev] [PATCH 1/3] f2fs: flush symlink path to avoid broken symlink after POR
> > 
> > This patch tries to avoid broken symlink case after POR in best effort.
> > This results in performance regression.
> > But, if f2fs has inline_data and the target path is under 3KB-sized long,
> > the page would be stored in its inode_block, so that there would be no
> > performance regression.
> > 
> > Note that, if user wants to keep this file atomically, it needs to trigger
> > dir->fsync.
> > And, there is still a hole to produce broken symlink.
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/namei.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> > index 8055e30..5d990d8 100644
> > --- a/fs/f2fs/namei.c
> > +++ b/fs/f2fs/namei.c
> > @@ -324,6 +324,17 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
> >  	d_instantiate(dentry, inode);
> >  	unlock_new_inode(inode);
> > 
> > +	/*
> > +	 * Let's flush symlink data in order to avoid broken symlink as much as
> > +	 * possible. Nevertheless, fsyncing is the best way, but there is no
> > +	 * way to get a file descriptor in order to flush that.
> > +	 *
> > +	 * Note that, it needs to do dir->fsync to make this recoverable.
> > +	 * If the symlink path is stored into inline_data, there is no
> > +	 * performance regression.
> > +	 */
> > +	filemap_write_and_wait_range(inode->i_mapping, 0, symlen);
> 
> One minor thing.
> 
> filemap_write_and_wait_range(inode->i_mapping, 0, symlen - 1);
> 
> Because we don't need to write out data exceed isize here.

Thank you for the review.
I fixed this and merged.

Thanks,

> 
> Thanks,
> 
> > +
> >  	if (IS_DIRSYNC(dir))
> >  		f2fs_sync_fs(sbi->sb, 1);
> >  	return err;
> > --
> > 2.1.1
> > 
> > 
> > ------------------------------------------------------------------------------
> > BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
> > Develop your own process in accordance with the BPMN 2 standard
> > Learn Process modeling best practices with Bonita BPM through live exercises
> > http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
> > source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel