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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Hi Jaegeuk,
> -----Original Message-----
> From: Jaegeuk Kim [mailto:[email protected]]
> Sent: Thursday, April 16, 2015 3:31 PM
> To: [email protected]; [email protected];
> [email protected]
> 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 <[email protected]>
> ---
> 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Hi Chao,
On Thu, Apr 16, 2015 at 06:33:29PM +0800, Chao Yu wrote:
> Hi Jaegeuk,
>
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:[email protected]]
> > Sent: Thursday, April 16, 2015 3:31 PM
> > To: [email protected]; [email protected];
> > [email protected]
> > 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 <[email protected]>
> > ---
> > 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
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel