[f2fs-dev] fsck.f2fs result parsing

2018-02-13 Thread Ashlie Martinez 
Hello,

I am a student at the University of Texas at Austin's CS department
and I am working on a new, record-and-replay, file system agnostic
crash consistency test harness called CrashMonkey [1][2]. As part of
this project, I need to run various file system checkers and return
one of several different enums that represents the results of the file
system checker run. The basic result enums mirror those in the man
page for fsck(8), and include values for "no errors found", "errors
corrected", "errors left uncorrected", and a catch-all "other error".

The man page for fsck.f2fs shows that it only returns either 0 or -1
for "success" or "failure". Is there any way I could parse some of the
output of fsck.f2fs (either stdout or stderr) to determine the outcome
of fsck.f2fs so that it can be categorized into the enums given above?
If so, where should I look for the relevant strings to parse for?

[1] https://github.com/utsaslab/crashmonkey
[2] https://www.usenix.org/conference/hotstorage17/program/presentation/martinez

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v3] f2fs: introduce "fsync={posix, strict}" mount options

2018-02-13 Thread Chao Yu 
On 2018/2/13 18:57, Junling Zheng wrote:
> Commit "0a007b97aad6"(f2fs: recover directory operations by fsync)
> fixed xfstest generic/342 case, but it also increased the written
> data and caused the performance degradation. In most cases, there's
> no need to do so heavy fsync actually.
> 
> So we introduce two new mount options "fsync={posix,strict}" to
> control the policy of fsync. "fsync=posix" is set by default, and
> means that f2fs uses a light fsync, which follows POSIX semantics.
> And "fsync=strict" means that it's a heavy fsync, which behaves in
> line with xfs, ext4 and btrfs, where generic/342 will pass, but the
> performance will regress.
> 
> Signed-off-by: Junling Zheng 
> ---
> Changes from v2:
>  - Change to "fsync={posix,strict}" format
>  - Set "fsync=posix" default
> Changes from v1:
>  - Add document modify
>  - Add reviewer
>  Documentation/filesystems/f2fs.txt |  7 +++
>  fs/f2fs/dir.c  |  3 ++-
>  fs/f2fs/f2fs.h |  1 +
>  fs/f2fs/file.c |  3 ++-
>  fs/f2fs/namei.c|  9 ++---
>  fs/f2fs/super.c| 10 ++
>  6 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/filesystems/f2fs.txt 
> b/Documentation/filesystems/f2fs.txt
> index 0caf7da0a532..c00d40655bbc 100644
> --- a/Documentation/filesystems/f2fs.txt
> +++ b/Documentation/filesystems/f2fs.txt
> @@ -180,6 +180,13 @@ whint_mode=%s  Control which write hints are 
> passed down to block
> down hints. In "user-based" mode, f2fs tries to pass
> down hints given by users. And in "fs-based" mode, 
> f2fs
> passes down hints with its policy.
> +fsync=%s   Control the policy of fsync. This supports "posix" and
> +   "strict" modes. In "posix" mode, which is default, 
> fsync
> +   will follow POSIX semantics and does a light 
> operation to
> +   improve the filesystem performance. In "strict" mode, 
> fsync
> +   will be heavy and behaves in line with xfs, ext4 and 
> btrfs,
> +   where xfstest generic/342 will pass, but the 
> performance
> +   will regress.
>  
>  
> 
>  DEBUGFS ENTRIES
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index f00b5ed8c011..37d1259f1b92 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -713,7 +713,8 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, 
> struct page *page,
>  
>   f2fs_update_time(F2FS_I_SB(dir), REQ_TIME);
>  
> - add_ino_entry(F2FS_I_SB(dir), dir->i_ino, TRANS_DIR_INO);
> + if (test_opt(F2FS_I_SB(dir), STRICT_FSYNC))
> + add_ino_entry(F2FS_I_SB(dir), dir->i_ino, TRANS_DIR_INO);
>  
>   if (f2fs_has_inline_dentry(dir))
>   return f2fs_delete_inline_entry(dentry, page, dir, inode);
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index dbe87c7a266e..8cf914d12f17 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -97,6 +97,7 @@ extern char *fault_name[FAULT_MAX];
>  #define F2FS_MOUNT_QUOTA 0x0040
>  #define F2FS_MOUNT_INLINE_XATTR_SIZE 0x0080
>  #define F2FS_MOUNT_RESERVE_ROOT  0x0100
> +#define F2FS_MOUNT_STRICT_FSYNC  0x0200
>  
>  #define clear_opt(sbi, option)   ((sbi)->mount_opt.opt &= 
> ~F2FS_MOUNT_##option)
>  #define set_opt(sbi, option) ((sbi)->mount_opt.opt |= F2FS_MOUNT_##option)
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 672a542e5464..509b3e045247 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -165,7 +165,8 @@ static inline enum cp_reason_type 
> need_do_checkpoint(struct inode *inode)
>   cp_reason = CP_FASTBOOT_MODE;
>   else if (sbi->active_logs == 2)
>   cp_reason = CP_SPEC_LOG_NUM;
> - else if (need_dentry_mark(sbi, inode->i_ino) &&
> + else if (test_opt(sbi, STRICT_FSYNC) &&
> + need_dentry_mark(sbi, inode->i_ino) &&
>   exist_written_data(sbi, F2FS_I(inode)->i_pino, TRANS_DIR_INO))
>   cp_reason = CP_RECOVER_DIR;
>  
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index c4c94c7e9f4f..5a385b8ab172 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -936,7 +936,8 @@ static int f2fs_rename(struct inode *old_dir, struct 
> dentry *old_dentry,
>   }
>   f2fs_i_links_write(old_dir, false);
>   }
> - add_ino_entry(sbi, new_dir->i_ino, TRANS_DIR_INO);
> + if (test_opt(sbi, STRICT_FSYNC))
> + add_ino_entry(sbi, new_dir->i_ino, TRANS_DIR_INO);
>  
>   f2fs_unlock_op(sbi);
>  
> @@ -1091,8 +1092,10 @@ static int f2fs_cross_rename(struct inode *old_dir, 
> struct dentry *old_dentry,
>   }
>   f2fs_mark_inode_dirty_sync(new_dir, false);
>  
> -

Re: [f2fs-dev] [PATCH] f2fs: handle quota for orphan inodes

2018-02-13 Thread Chao Yu 
On 2018/2/10 10:28, Jaegeuk Kim wrote:
> This is to fix missing dquot_initialize for orphan inodes.

IMO, we don't need to call dquot_initialize as we have call the function
in evict(), right?

Thanks,

> 
> Signed-off-by: Jaegeuk Kim 
> ---
>  fs/f2fs/checkpoint.c | 28 
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 8b0945ba284d..e3bf753a47be 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -569,13 +569,8 @@ static int recover_orphan_inode(struct f2fs_sb_info 
> *sbi, nid_t ino)
>   struct node_info ni;
>   int err = acquire_orphan_inode(sbi);
>  
> - if (err) {
> - set_sbi_flag(sbi, SBI_NEED_FSCK);
> - f2fs_msg(sbi->sb, KERN_WARNING,
> - "%s: orphan failed (ino=%x), run fsck to fix.",
> - __func__, ino);
> - return err;
> - }
> + if (err)
> + goto err_out;
>  
>   __add_ino_entry(sbi, ino, 0, ORPHAN_INO);
>  
> @@ -589,6 +584,11 @@ static int recover_orphan_inode(struct f2fs_sb_info 
> *sbi, nid_t ino)
>   return PTR_ERR(inode);
>   }
>  
> + err = dquot_initialize(inode);
> + if (err)
> + goto err_out;
> +
> + dquot_initialize(inode);
>   clear_nlink(inode);
>  
>   /* truncate all the data during iput */
> @@ -598,14 +598,18 @@ static int recover_orphan_inode(struct f2fs_sb_info 
> *sbi, nid_t ino)
>  
>   /* ENOMEM was fully retried in f2fs_evict_inode. */
>   if (ni.blk_addr != NULL_ADDR) {
> - set_sbi_flag(sbi, SBI_NEED_FSCK);
> - f2fs_msg(sbi->sb, KERN_WARNING,
> - "%s: orphan failed (ino=%x) by kernel, retry mount.",
> - __func__, ino);
> - return -EIO;
> + err = -EIO;
> + goto err_out;
>   }
>   __remove_ino_entry(sbi, ino, ORPHAN_INO);
>   return 0;
> +
> +err_out:
> + set_sbi_flag(sbi, SBI_NEED_FSCK);
> + f2fs_msg(sbi->sb, KERN_WARNING,
> + "%s: orphan failed (ino=%x), run fsck to fix.",
> + __func__, ino);
> + return err;
>  }
>  
>  int recover_orphan_inodes(struct f2fs_sb_info *sbi)
> 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [RFC PATCH 5/5] fsck.f2fs: reconnect unreachable files to lost+found

2018-02-13 Thread Chao Yu 
On 2018/2/6 12:31, Sheng Yong wrote:
> This patch introduces lost+found feature to fsck. If a file is found
> unreachable by fsck. Fsck tries to reconnect the file to lost+found
> directory:
>   1. Scan all unreachable file inodes, ignore non-inodes ones and
>  directories.
>   2. Check them and fix incorrupted data to make sure filesystem
>  metadata (mainly counters and main/nat bitmap) are all consistent.
>   3. Reconnect these files to lost+found. If lost+found does not exist,
>  create it first. During reconnecting, expand lost+found's dentry
>  block automatically. Reconnected files are renamed after its ino
>  number.
>   4. If reconnect fails drop the node and restore filesystem metadata.
> 
> Signed-off-by: Sheng Yong 
> ---
>  fsck/dir.c   |  19 ++-
>  fsck/fsck.c  | 376 
> ++-
>  fsck/fsck.h  |   3 +
>  fsck/mount.c |   2 +
>  4 files changed, 397 insertions(+), 3 deletions(-)
> 
> diff --git a/fsck/dir.c b/fsck/dir.c
> index b2ea18f..567a4e9 100644
> --- a/fsck/dir.c
> +++ b/fsck/dir.c
> @@ -176,6 +176,23 @@ static int f2fs_find_entry(struct f2fs_sb_info *sbi,
>   return 0;
>  }
>  
> +/* return ino if file exists, otherwise return 0 */
> +nid_t f2fs_lookup(struct f2fs_sb_info *sbi, struct f2fs_node *dir,
> + u8 *name, int len)
> +{
> + int err;
> + struct dentry de = {
> + .name = name,
> + .len = len,
> + };
> +
> + err = f2fs_find_entry(sbi, dir, );
> + if (err == 1)
> + return de.ino;
> + else
> + return 0;
> +}
> +
>  static void f2fs_update_dentry(nid_t ino, int file_type,
>   struct f2fs_dentry_ptr *d,
>   const unsigned char *name, int len, f2fs_hash_t name_hash,
> @@ -199,7 +216,7 @@ static void f2fs_update_dentry(nid_t ino, int file_type,
>  /*
>   * f2fs_add_link - Add a new file(dir) to parent dir.
>   */
> -static int f2fs_add_link(struct f2fs_sb_info *sbi, struct f2fs_node *parent,
> +int f2fs_add_link(struct f2fs_sb_info *sbi, struct f2fs_node *parent,
>   const unsigned char *name, int name_len, nid_t ino,
>   int file_type, block_t p_blkaddr, int inc_link)
>  {
> diff --git a/fsck/fsck.c b/fsck/fsck.c
> index fcaab14..81e1145 100644
> --- a/fsck/fsck.c
> +++ b/fsck/fsck.c
> @@ -10,6 +10,7 @@
>   */
>  #include "fsck.h"
>  #include "quotaio.h"
> +#include 
>  
>  char *tree_mark;
>  uint32_t tree_mark_size = 256;
> @@ -43,6 +44,14 @@ static inline int f2fs_test_main_bitmap(struct 
> f2fs_sb_info *sbi, u32 blk)
>   fsck->main_area_bitmap);
>  }
>  
> +static inline int f2fs_clear_main_bitmap(struct f2fs_sb_info *sbi, u32 blk)
> +{
> + struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
> +
> + return f2fs_clear_bit(BLKOFF_FROM_MAIN(sbi, blk),
> + fsck->main_area_bitmap);
> +}
> +
>  static inline int f2fs_test_sit_bitmap(struct f2fs_sb_info *sbi, u32 blk)
>  {
>   struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
> @@ -448,9 +457,11 @@ static int sanity_check_nid(struct f2fs_sb_info *sbi, 
> u32 nid,
>  
>   /* workaround to fix later */
>   if (ftype != F2FS_FT_ORPHAN ||
> - f2fs_test_bit(nid, fsck->nat_area_bitmap) != 0)
> + f2fs_test_bit(nid, fsck->nat_area_bitmap) != 0) {
>   f2fs_clear_bit(nid, fsck->nat_area_bitmap);
> - else
> + /* avoid reusing nid when reconnecting files */
> + f2fs_set_bit(nid, NM_I(sbi)->nid_bitmap);
> + } else
>   ASSERT_MSG("orphan or xattr nid is duplicated [0x%x]\n",
>   nid);
>  
> @@ -2041,6 +2052,362 @@ int check_sit_types(struct f2fs_sb_info *sbi)
>   return err;
>  }
>  
> +static struct f2fs_node *fsck_get_lpf(struct f2fs_sb_info *sbi)
> +{
> + struct f2fs_node *node;
> + struct node_info ni;
> + nid_t lpf_ino;
> + int err;
> +
> + /* read root inode first */
> + node = calloc(F2FS_BLKSIZE, 1);
> + ASSERT(node);
> + get_node_info(sbi, F2FS_ROOT_INO(sbi), );
> + err = dev_read_block(node, ni.blk_addr);
> + ASSERT(err >= 0);
> +
> + /* lookup lost+found in root directory */
> + lpf_ino = f2fs_lookup(sbi, node, (u8 *) LPF, strlen(LPF));
> + if (lpf_ino) { /* found */
> + get_node_info(sbi, lpf_ino, );
> + err = dev_read_block(node, ni.blk_addr);
> + ASSERT(err >= 0);
> + DBG(1, "Found lost+found 0x%x at blkaddr [0x%x]\n",
> + lpf_ino, ni.blk_addr);
> + if (!S_ISDIR(le16_to_cpu(node->i.i_mode))) {
> + ASSERT_MSG("lost+found is not directory [0%o]\n",
> +le16_to_cpu(node->i.i_mode));
> + /* FIXME: give up? */
> + goto out;
> + }
> + } else {

[f2fs-dev] [PATCH v3] f2fs: introduce "fsync={posix, strict}" mount options

2018-02-13 Thread Junling Zheng 
Commit "0a007b97aad6"(f2fs: recover directory operations by fsync)
fixed xfstest generic/342 case, but it also increased the written
data and caused the performance degradation. In most cases, there's
no need to do so heavy fsync actually.

So we introduce two new mount options "fsync={posix,strict}" to
control the policy of fsync. "fsync=posix" is set by default, and
means that f2fs uses a light fsync, which follows POSIX semantics.
And "fsync=strict" means that it's a heavy fsync, which behaves in
line with xfs, ext4 and btrfs, where generic/342 will pass, but the
performance will regress.

Signed-off-by: Junling Zheng 
---
Changes from v2:
 - Change to "fsync={posix,strict}" format
 - Set "fsync=posix" default
Changes from v1:
 - Add document modify
 - Add reviewer
 Documentation/filesystems/f2fs.txt |  7 +++
 fs/f2fs/dir.c  |  3 ++-
 fs/f2fs/f2fs.h |  1 +
 fs/f2fs/file.c |  3 ++-
 fs/f2fs/namei.c|  9 ++---
 fs/f2fs/super.c| 10 ++
 6 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/Documentation/filesystems/f2fs.txt 
b/Documentation/filesystems/f2fs.txt
index 0caf7da0a532..c00d40655bbc 100644
--- a/Documentation/filesystems/f2fs.txt
+++ b/Documentation/filesystems/f2fs.txt
@@ -180,6 +180,13 @@ whint_mode=%s  Control which write hints are 
passed down to block
down hints. In "user-based" mode, f2fs tries to pass
down hints given by users. And in "fs-based" mode, f2fs
passes down hints with its policy.
+fsync=%s   Control the policy of fsync. This supports "posix" and
+   "strict" modes. In "posix" mode, which is default, fsync
+   will follow POSIX semantics and does a light operation 
to
+   improve the filesystem performance. In "strict" mode, 
fsync
+   will be heavy and behaves in line with xfs, ext4 and 
btrfs,
+   where xfstest generic/342 will pass, but the performance
+   will regress.
 
 

 DEBUGFS ENTRIES
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index f00b5ed8c011..37d1259f1b92 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -713,7 +713,8 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, 
struct page *page,
 
f2fs_update_time(F2FS_I_SB(dir), REQ_TIME);
 
-   add_ino_entry(F2FS_I_SB(dir), dir->i_ino, TRANS_DIR_INO);
+   if (test_opt(F2FS_I_SB(dir), STRICT_FSYNC))
+   add_ino_entry(F2FS_I_SB(dir), dir->i_ino, TRANS_DIR_INO);
 
if (f2fs_has_inline_dentry(dir))
return f2fs_delete_inline_entry(dentry, page, dir, inode);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index dbe87c7a266e..8cf914d12f17 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -97,6 +97,7 @@ extern char *fault_name[FAULT_MAX];
 #define F2FS_MOUNT_QUOTA   0x0040
 #define F2FS_MOUNT_INLINE_XATTR_SIZE   0x0080
 #define F2FS_MOUNT_RESERVE_ROOT0x0100
+#define F2FS_MOUNT_STRICT_FSYNC0x0200
 
 #define clear_opt(sbi, option) ((sbi)->mount_opt.opt &= ~F2FS_MOUNT_##option)
 #define set_opt(sbi, option)   ((sbi)->mount_opt.opt |= F2FS_MOUNT_##option)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 672a542e5464..509b3e045247 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -165,7 +165,8 @@ static inline enum cp_reason_type need_do_checkpoint(struct 
inode *inode)
cp_reason = CP_FASTBOOT_MODE;
else if (sbi->active_logs == 2)
cp_reason = CP_SPEC_LOG_NUM;
-   else if (need_dentry_mark(sbi, inode->i_ino) &&
+   else if (test_opt(sbi, STRICT_FSYNC) &&
+   need_dentry_mark(sbi, inode->i_ino) &&
exist_written_data(sbi, F2FS_I(inode)->i_pino, TRANS_DIR_INO))
cp_reason = CP_RECOVER_DIR;
 
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index c4c94c7e9f4f..5a385b8ab172 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -936,7 +936,8 @@ static int f2fs_rename(struct inode *old_dir, struct dentry 
*old_dentry,
}
f2fs_i_links_write(old_dir, false);
}
-   add_ino_entry(sbi, new_dir->i_ino, TRANS_DIR_INO);
+   if (test_opt(sbi, STRICT_FSYNC))
+   add_ino_entry(sbi, new_dir->i_ino, TRANS_DIR_INO);
 
f2fs_unlock_op(sbi);
 
@@ -1091,8 +1092,10 @@ static int f2fs_cross_rename(struct inode *old_dir, 
struct dentry *old_dentry,
}
f2fs_mark_inode_dirty_sync(new_dir, false);
 
-   add_ino_entry(sbi, old_dir->i_ino, TRANS_DIR_INO);
-   add_ino_entry(sbi, new_dir->i_ino, TRANS_DIR_INO);
+   if (test_opt(sbi, STRICT_FSYNC)) {
+   add_ino_entry(sbi, old_dir->i_ino, TRANS_DIR_INO);
+

Re: [f2fs-dev] [PATCH RFC] f2fs: introduce a method to make nat journal more fresh

2018-02-13 Thread Chao Yu 
On 2018/2/12 14:55, Yunlei He wrote:
> This patch introduce a method to make nat journal more fresh:
> i.  sort set list using entry # plus cp version difference (last modify
> set cp # and current cp #)
> ii. if meet with cache hit, update last modify cp # to current cp #

As we discuss off-line, I think we can record weighted average of cpver for all
dirty nat entries into nat set.

Thanks,

> 
> Signed-off-by: Yunlei He 
> ---
>  fs/f2fs/f2fs.h |  2 ++
>  fs/f2fs/node.c | 57 +
>  fs/f2fs/node.h |  2 ++
>  3 files changed, 45 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index b7ba496..bd1e775 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -297,6 +297,8 @@ struct fsync_inode_entry {
>   block_t last_dentry;/* block address locating the last dentry */
>  };
>  
> +#define DEF_NAT_FACTOR 10
> +
>  #define nats_in_cursum(jnl)  (le16_to_cpu((jnl)->n_nats))
>  #define sits_in_cursum(jnl)  (le16_to_cpu((jnl)->n_sits))
>  
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index ced986d..f3f89e3 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -195,8 +195,8 @@ static void __del_from_nat_cache(struct f2fs_nm_info 
> *nm_i, struct nat_entry *e)
>   __free_nat_entry(e);
>  }
>  
> -static void __set_nat_cache_dirty(struct f2fs_nm_info *nm_i,
> - struct nat_entry *ne)
> +static void __set_nat_cache_dirty(struct f2fs_sb_info *sbi, bool 
> from_journal,
> + struct f2fs_nm_info *nm_i, struct nat_entry *ne)
>  {
>   nid_t set = NAT_BLOCK_OFFSET(ne->ni.nid);
>   struct nat_entry_set *head;
> @@ -209,9 +209,15 @@ static void __set_nat_cache_dirty(struct f2fs_nm_info 
> *nm_i,
>   INIT_LIST_HEAD(>set_list);
>   head->set = set;
>   head->entry_cnt = 0;
> + head->to_journal = false;
> + head->cp_ver = cur_cp_version(F2FS_CKPT(sbi));
>   f2fs_radix_tree_insert(_i->nat_set_root, set, head);
>   }
>  
> + /* journal hit case, try to locate set in journal */
> + if (head->to_journal && !from_journal)
> + head->cp_ver = cur_cp_version(F2FS_CKPT(sbi));
> +
>   if (get_nat_flag(ne, IS_DIRTY))
>   goto refresh_list;
>  
> @@ -359,7 +365,7 @@ static void set_node_addr(struct f2fs_sb_info *sbi, 
> struct node_info *ni,
>   nat_set_blkaddr(e, new_blkaddr);
>   if (new_blkaddr == NEW_ADDR || new_blkaddr == NULL_ADDR)
>   set_nat_flag(e, IS_CHECKPOINTED, false);
> - __set_nat_cache_dirty(nm_i, e);
> + __set_nat_cache_dirty(sbi, false, nm_i, e);
>  
>   /* update fsync_mark if its inode nat entry is still alive */
>   if (ni->nid != ni->ino)
> @@ -2397,14 +2403,30 @@ static void remove_nats_in_journal(struct 
> f2fs_sb_info *sbi)
>   spin_unlock(_i->nid_list_lock);
>   }
>  
> - __set_nat_cache_dirty(nm_i, ne);
> + __set_nat_cache_dirty(sbi, true, nm_i, ne);
>   }
>   update_nats_in_cursum(journal, -i);
>   up_write(>journal_rwsem);
>  }
>  
> -static void __adjust_nat_entry_set(struct nat_entry_set *nes,
> - struct list_head *head, int max)
> +static bool cost_compare(struct f2fs_sb_info *sbi,
> + struct nat_entry_set *nes, struct nat_entry_set * cur)
> +{
> + struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
> + __u64 cost1 = nes->entry_cnt * DEF_NAT_FACTOR +
> + (nes->cp_ver <= cur_cp_version(ckpt)) ?
> + cur_cp_version(ckpt) - nes->cp_ver :
> + ULONG_MAX - nes->cp_ver + cur_cp_version(ckpt);
> + __u64 cost2 = cur->entry_cnt * DEF_NAT_FACTOR +
> + (cur->cp_ver <= cur_cp_version(ckpt)) ?
> + cur_cp_version(ckpt) - cur->cp_ver :
> + ULONG_MAX - cur->cp_ver + cur_cp_version(ckpt);
> +
> + return cost1 <= cost2;
> +}
> +
> +static void __adjust_nat_entry_set(struct f2fs_sb_info *sbi,
> + struct nat_entry_set *nes, struct list_head *head, int 
> max)
>  {
>   struct nat_entry_set *cur;
>  
> @@ -2412,7 +2434,7 @@ static void __adjust_nat_entry_set(struct nat_entry_set 
> *nes,
>   goto add_out;
>  
>   list_for_each_entry(cur, head, set_list) {
> - if (cur->entry_cnt >= nes->entry_cnt) {
> + if (cost_compare(sbi, nes, cur)) {
>   list_add(>set_list, cur->set_list.prev);
>   return;
>   }
> @@ -2460,7 +2482,6 @@ static void __flush_nat_entry_set(struct f2fs_sb_info 
> *sbi,
>   struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
>   struct f2fs_journal *journal = curseg->journal;
>   nid_t start_nid = set->set * NAT_ENTRY_PER_BLOCK;
> - bool to_journal = true;
>

Re: [f2fs-dev] [PATCH] f2fs: add bug on back during flush nat entries

2018-02-13 Thread Chao Yu 
On 2018/2/12 11:42, Yunlei He wrote:
> This patch add two bug on check back during flush nat entries,
> nat_tree_lock will prevent race from node block allocation
> in write_begin.
> 
> Signed-off-by: Yunlei He 
> ---
>  fs/f2fs/node.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 833b46b..ced986d 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -2519,10 +2519,9 @@ static void __flush_nat_entry_set(struct f2fs_sb_info 
> *sbi,
>   }
>  
>   /* Allow dirty nats by node block allocation in write_begin */
> - if (!set->entry_cnt) {
> - radix_tree_delete(_I(sbi)->nat_set_root, set->set);
> - kmem_cache_free(nat_entry_set_slab, set);

During checkpoint, We allow buffered write which needs nid allocation, if we
rollback the codes here, we may encounter the BUG_ON?

Thanks,

> - }
> + f2fs_bug_on(sbi, set->entry_cnt);
> + radix_tree_delete(_I(sbi)->nat_set_root, set->set);
> + kmem_cache_free(nat_entry_set_slab, set);
>  }
>  
>  /*
> @@ -2566,8 +2565,8 @@ void flush_nat_entries(struct f2fs_sb_info *sbi, struct 
> cp_control *cpc)
>   list_for_each_entry_safe(set, tmp, , set_list)
>   __flush_nat_entry_set(sbi, set, cpc);
>  
> + f2fs_bug_on(sbi, nm_i->dirty_nat_cnt);
>   up_write(_i->nat_tree_lock);
> - /* Allow dirty nats by node block allocation in write_begin */
>  }
>  
>  static int __get_nat_bitmaps(struct f2fs_sb_info *sbi)
> 


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync

2018-02-13 Thread Chao Yu 
Hi Jaegeuk,

On 2018/2/12 8:07, Jaegeuk Kim wrote:
> On 02/11, Junling Zheng wrote:
>> Hi, Jaegeuk
>>
>> On 2018/2/10 8:44, Jaegeuk Kim wrote:
>>> On 02/02, Junling Zheng wrote:
 Commit "0a007b97aad6"(f2fs: recover directory operations by fsync)
 fixed xfstest generic/342 case, but it also increased the written
 data and caused the performance degradation. In most cases, there's
 no need to do so heavily fsync actually.

 So we introduce a new mount option "strict_fsync" to control the
 policy of fsync. It's set by default, and means that fsync follows
 POSIX semantics. And "nostrict_fsync" means that the behaviour is
 in line with xfs, ext4 and btrfs, where generic/342 will pass.
>>>
>>> How about adding "fsync=%s" to give another chance for fsync policies?

Agreed.

>>>
>>
>> OK, I'll give patch v3 to change to "fsync=%s" format.
>> BTW, which policy do u think should be the default behavior for f2fs? Posix
>> or ext4?
> 
> The default should be like ext4 as fsync=strict. We may add fsync=posix for
> this.

I'd like to suggest using fsync=posix option by default, because in most popular
in-used scenario of f2fs: android, all users of filesystem are posix-compliant,
so there is no such requirement that fs needs do more than posix as generic/342
restricted.

The performance of fsync=strict mode regressed as expected, so if we enable
fsync=strict mode by default, I suspect it may make f2fs losing some kinds of
benchmark competition.

How do you think?

Thanks,

> 
> Thanks,
> 
>>
>> Thanks
>> Junling
>>
>>> Thanks,
>>>

 Signed-off-by: Junling Zheng 
 Reviewed-by: Chao Yu 
 ---
  Documentation/filesystems/f2fs.txt |  4 
  fs/f2fs/dir.c  |  3 ++-
  fs/f2fs/f2fs.h |  1 +
  fs/f2fs/file.c |  3 ++-
  fs/f2fs/namei.c|  9 ++---
  fs/f2fs/super.c| 13 +
  6 files changed, 28 insertions(+), 5 deletions(-)

> 
> .
> 


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel