Re: [f2fs-dev] [PATCH] fscrypto: use standard macros to compute length of fname ciphertext

2016-09-29 Thread Theodore Ts'o 
On Thu, Sep 22, 2016 at 01:31:49PM -0700, Eric Biggers wrote:
> Signed-off-by: Eric Biggers 

Thanks, applied.

- Ted

--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] ext4: do not unnecessarily null-terminate encrypted symlink data

2016-09-29 Thread Theodore Ts'o 
On Thu, Sep 22, 2016 at 01:31:47PM -0700, Eric Biggers wrote:
> Null-terminating the fscrypt_symlink_data on read is unnecessary because
> it is not string data --- it contains binary ciphertext.
> 
> Signed-off-by: Eric Biggers 

Thanks, applied.

- Ted

--
___
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/2] f2fs: use crc and cp version to determine roll-forward recovery

2016-09-29 Thread Chao Yu 
On 2016/9/30 8:53, Jaegeuk Kim wrote:
> On Thu, Sep 29, 2016 at 08:01:32PM +0800, Chao Yu wrote:
>> On 2016/9/20 10:55, Jaegeuk Kim wrote:
>>> Previously, we used cp_version only to detect recoverable dnodes.
>>> In order to avoid same garbage cp_version, we needed to truncate the next
>>> dnode during checkpoint, resulting in additional discard or data write.
>>> If we can distinguish this by using crc in addition to cp_version, we can
>>> remove this overhead.
>>>
>>> There is backward compatibility concern where it changes node_footer layout.
>>> But, it only affects the direct nodes written after the last checkpoint.
>>> We simply expect that user would change kernel versions back and forth after
>>> stable checkpoint.
>>
>> Seems with new released v4.8 f2fs, old image with recoverable data could be
>> mounted successfully, but meanwhile all fsynced data which needs to be 
>> recovered
>> will be lost w/o any hints?
>>
>> Could we release a new version mkfs paired with new kernel module, so we can 
>> tag
>> image as a new layout one, then new kernel module can recognize the image 
>> layout
>> and adjust version suited comparing method with old or new image?
> 
> Hmm, how about adding a checkpoint flag like CP_CRC_RECOVERY_FLAG?
> Then, we can proceed crc|cp_ver, if the last checkpoint has this flag.
> 
> Any thought?

Ah, that's better. :)

Thanks,

> 
>>
>> Thanks,
>>
>>
> 
> .
> 


--
___
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/2] f2fs: use crc and cp version to determine roll-forward recovery

2016-09-29 Thread Jaegeuk Kim 
On Thu, Sep 29, 2016 at 08:01:32PM +0800, Chao Yu wrote:
> On 2016/9/20 10:55, Jaegeuk Kim wrote:
> > Previously, we used cp_version only to detect recoverable dnodes.
> > In order to avoid same garbage cp_version, we needed to truncate the next
> > dnode during checkpoint, resulting in additional discard or data write.
> > If we can distinguish this by using crc in addition to cp_version, we can
> > remove this overhead.
> > 
> > There is backward compatibility concern where it changes node_footer layout.
> > But, it only affects the direct nodes written after the last checkpoint.
> > We simply expect that user would change kernel versions back and forth after
> > stable checkpoint.
> 
> Seems with new released v4.8 f2fs, old image with recoverable data could be
> mounted successfully, but meanwhile all fsynced data which needs to be 
> recovered
> will be lost w/o any hints?
> 
> Could we release a new version mkfs paired with new kernel module, so we can 
> tag
> image as a new layout one, then new kernel module can recognize the image 
> layout
> and adjust version suited comparing method with old or new image?

Hmm, how about adding a checkpoint flag like CP_CRC_RECOVERY_FLAG?
Then, we can proceed crc|cp_ver, if the last checkpoint has this flag.

Any thought?

> 
> Thanks,
> 
> 

--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH v4 RESEND] f2fs: introduce get_checkpoint_version for cleanup

2016-09-29 Thread Tiezhu Yang 
There exists almost same codes when get the value of pre_version
and cur_version in function validate_checkpoint, this patch adds
get_checkpoint_version to clean up redundant codes.

Signed-off-by: Tiezhu Yang 
---
 fs/f2fs/checkpoint.c | 66 ++--
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index de8693c..764ed0b 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -663,45 +663,55 @@ static void write_orphan_inodes(struct f2fs_sb_info *sbi, 
block_t start_blk)
}
 }
 
-static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
-   block_t cp_addr, unsigned long long *version)
+static int get_checkpoint_version(struct f2fs_sb_info *sbi, block_t cp_addr,
+   struct f2fs_checkpoint **cp_block, struct page **cp_page,
+   unsigned long long *version)
 {
-   struct page *cp_page_1, *cp_page_2 = NULL;
unsigned long blk_size = sbi->blocksize;
-   struct f2fs_checkpoint *cp_block;
-   unsigned long long cur_version = 0, pre_version = 0;
-   size_t crc_offset;
+   size_t crc_offset = 0;
__u32 crc = 0;
 
-   /* Read the 1st cp block in this CP pack */
-   cp_page_1 = get_meta_page(sbi, cp_addr);
+   *cp_page = get_meta_page(sbi, cp_addr);
+   *cp_block = (struct f2fs_checkpoint *)page_address(*cp_page);
 
-   /* get the version number */
-   cp_block = (struct f2fs_checkpoint *)page_address(cp_page_1);
-   crc_offset = le32_to_cpu(cp_block->checksum_offset);
-   if (crc_offset >= blk_size)
-   goto invalid_cp1;
+   crc_offset = le32_to_cpu((*cp_block)->checksum_offset);
+   if (crc_offset >= blk_size) {
+   f2fs_msg(sbi->sb, KERN_WARNING,
+   "invalid crc_offset: %zu", crc_offset);
+   return -EINVAL;
+   }
 
-   crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
crc_offset)));
-   if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
-   goto invalid_cp1;
+   crc = le32_to_cpu(*((__le32 *)((unsigned char *)*cp_block
+   + crc_offset)));
+   if (!f2fs_crc_valid(sbi, crc, *cp_block, crc_offset)) {
+   f2fs_msg(sbi->sb, KERN_WARNING, "invalid crc value");
+   return -EINVAL;
+   }
 
-   pre_version = cur_cp_version(cp_block);
+   *version = cur_cp_version(*cp_block);
+   return 0;
+}
 
-   /* Read the 2nd cp block in this CP pack */
-   cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
-   cp_page_2 = get_meta_page(sbi, cp_addr);
+static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
+   block_t cp_addr, unsigned long long *version)
+{
+   struct page *cp_page_1 = NULL, *cp_page_2 = NULL;
+   struct f2fs_checkpoint *cp_block = NULL;
+   unsigned long long cur_version = 0, pre_version = 0;
+   int err;
 
-   cp_block = (struct f2fs_checkpoint *)page_address(cp_page_2);
-   crc_offset = le32_to_cpu(cp_block->checksum_offset);
-   if (crc_offset >= blk_size)
-   goto invalid_cp2;
+   err = get_checkpoint_version(sbi, cp_addr, _block,
+   _page_1, version);
+   if (err)
+   goto invalid_cp1;
+   pre_version = *version;
 
-   crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
crc_offset)));
-   if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
+   cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
+   err = get_checkpoint_version(sbi, cp_addr, _block,
+   _page_2, version);
+   if (err)
goto invalid_cp2;
-
-   cur_version = cur_cp_version(cp_block);
+   cur_version = *version;
 
if (cur_version == pre_version) {
*version = cur_version;
-- 
1.8.3.1
--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH v4] f2fs: introduce get_checkpoint_version for cleanup

2016-09-29 Thread Tiezhu Yang 
There exists almost same codes when get the value of pre_version
and cur_version in function validate_checkpoint, this patch adds
get_checkpoint_version to clean up redundant codes.

Signed-off-by: Tiezhu Yang 
---
 fs/f2fs/checkpoint.c | 66 ++--
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index de8693c..764ed0b 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -663,45 +663,55 @@ static void write_orphan_inodes(struct f2fs_sb_info *sbi, 
block_t start_blk)
}
 }
 
-static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
-   block_t cp_addr, unsigned long long *version)
+static int get_checkpoint_version(struct f2fs_sb_info *sbi, block_t cp_addr,
+   struct f2fs_checkpoint **cp_block, struct page **cp_page,
+   unsigned long long *version)
 {
-   struct page *cp_page_1, *cp_page_2 = NULL;
unsigned long blk_size = sbi->blocksize;
-   struct f2fs_checkpoint *cp_block;
-   unsigned long long cur_version = 0, pre_version = 0;
-   size_t crc_offset;
+   size_t crc_offset = 0;
__u32 crc = 0;
 
-   /* Read the 1st cp block in this CP pack */
-   cp_page_1 = get_meta_page(sbi, cp_addr);
+   *cp_page = get_meta_page(sbi, cp_addr);
+   *cp_block = (struct f2fs_checkpoint *)page_address(*cp_page);
 
-   /* get the version number */
-   cp_block = (struct f2fs_checkpoint *)page_address(cp_page_1);
-   crc_offset = le32_to_cpu(cp_block->checksum_offset);
-   if (crc_offset >= blk_size)
-   goto invalid_cp1;
+   crc_offset = le32_to_cpu((*cp_block)->checksum_offset);
+   if (crc_offset >= blk_size) {
+   f2fs_msg(sbi->sb, KERN_WARNING,
+   "invalid crc_offset: %zu.", crc_offset);
+   return -EINVAL;
+   }
 
-   crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
crc_offset)));
-   if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
-   goto invalid_cp1;
+   crc = le32_to_cpu(*((__le32 *)((unsigned char *)*cp_block
+   + crc_offset)));
+   if (!f2fs_crc_valid(sbi, crc, *cp_block, crc_offset)) {
+   f2fs_msg(sbi->sb, KERN_WARNING, "invalid crc value.");
+   return -EINVAL;
+   }
 
-   pre_version = cur_cp_version(cp_block);
+   *version = cur_cp_version(*cp_block);
+   return 0;
+}
 
-   /* Read the 2nd cp block in this CP pack */
-   cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
-   cp_page_2 = get_meta_page(sbi, cp_addr);
+static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
+   block_t cp_addr, unsigned long long *version)
+{
+   struct page *cp_page_1 = NULL, *cp_page_2 = NULL;
+   struct f2fs_checkpoint *cp_block = NULL;
+   unsigned long long cur_version = 0, pre_version = 0;
+   int err;
 
-   cp_block = (struct f2fs_checkpoint *)page_address(cp_page_2);
-   crc_offset = le32_to_cpu(cp_block->checksum_offset);
-   if (crc_offset >= blk_size)
-   goto invalid_cp2;
+   err = get_checkpoint_version(sbi, cp_addr, _block,
+   _page_1, version);
+   if (err)
+   goto invalid_cp1;
+   pre_version = *version;
 
-   crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
crc_offset)));
-   if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
+   cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
+   err = get_checkpoint_version(sbi, cp_addr, _block,
+   _page_2, version);
+   if (err)
goto invalid_cp2;
-
-   cur_version = cur_cp_version(cp_block);
+   cur_version = *version;
 
if (cur_version == pre_version) {
*version = cur_version;
-- 
1.8.3.1
--
___
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/2] f2fs: fix to commit bio cache after flushing node pages

2016-09-29 Thread Jaegeuk Kim 
On Thu, Sep 29, 2016 at 06:45:03PM +0800, Chao Yu wrote:
> On 2016/9/29 4:19, Jaegeuk Kim wrote:
> > On Tue, Sep 27, 2016 at 10:09:03AM +0800, Chao Yu wrote:
> >> On 2016/9/27 9:39, Jaegeuk Kim wrote:
> >>> On Tue, Sep 27, 2016 at 08:57:41AM +0800, Chao Yu wrote:
>  Hi Jaegeuk,
> 
>  On 2016/9/27 2:33, Jaegeuk Kim wrote:
> > Hi Chao,
> >
> > On Tue, Sep 27, 2016 at 12:09:52AM +0800, Chao Yu wrote:
> >> From: Chao Yu 
> >>
> >> In sync_node_pages, we won't check and commit last merged pages in 
> >> private
> >> bio cache of f2fs, as these pages were taged as writeback, someone who 
> >> is
> >> waiting for writebacking of the page will be blocked until the cache 
> >> was
> >> committed by someone else.
> >>
> >> We need to commit node type bio cache to avoid potential deadlock or 
> >> long
> >> delay of waiting writeback.
> >>
> >> Signed-off-by: Chao Yu 
> >> ---
> >>  fs/f2fs/node.c | 11 +--
> >>  1 file changed, 9 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> >> index 9faddcd..f73f774 100644
> >> --- a/fs/f2fs/node.c
> >> +++ b/fs/f2fs/node.c
> >> @@ -1416,6 +1416,7 @@ int sync_node_pages(struct f2fs_sb_info *sbi, 
> >> struct writeback_control *wbc)
> >>struct pagevec pvec;
> >>int step = 0;
> >>int nwritten = 0;
> >> +  int ret = 0;
> >>  
> >>pagevec_init(, 0);
> >>  
> >> @@ -1436,7 +1437,8 @@ next_step:
> >>  
> >>if (unlikely(f2fs_cp_error(sbi))) {
> >>pagevec_release();
> >> -  return -EIO;
> >> +  ret = -EIO;
> >> +  goto out;
> >>}
> >>  
> >>/*
> >> @@ -1487,6 +1489,8 @@ continue_unlock:
> >>  
> >>if (NODE_MAPPING(sbi)->a_ops->writepage(page, 
> >> wbc))
> >>unlock_page(page);
> >> +  else
> >> +  nwritten++;
> >>  
> >>if (--wbc->nr_to_write == 0)
> >>break;
> >> @@ -1504,7 +1508,10 @@ continue_unlock:
> >>step++;
> >>goto next_step;
> >>}
> >> -  return nwritten;
> >> +out:
> >> +  if (nwritten)
> >> +  f2fs_submit_merged_bio(sbi, NODE, WRITE);
> >
> > IIRC, we don't need to flush this, since f2fs_submit_merged_bio_cond() 
> > would
> > handle this in f2fs_wait_on_page_writeback().
> 
>  Yes, it covers all the cases in f2fs private codes, but there are still 
>  some
>  codes in mm or fs directory, and they didn't use 
>  f2fs_wait_on_page_writeback
>  when waiting page writeback. Such as do_writepages && filemap_fdatawait 
>  in
>  __writeback_single_inode...
> >>>
> >>> The do_writepages() is okay, which will call f2fs_write_node_pages().
> >>> The __writeback_single_inode() won't do filemap_fdatawait() with 
> >>> WB_SYNC_ALL.
> >>> We don't need to take care of truncation as well.
> >>>
> >>> Any missing one?
> >>
> >> Another is: while testing with first version of checkpoint error 
> >> injection, I
> >> encounter below dump stack:
> >>
> >> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> >> mount   D 8801c1bf7960 0 97685  97397 0x0008
> >>  8801c1bf7960 8801c1bf7930 88017590 8801c1bf7980
> >>  8801c1bf8000  7fff 88021f7be340
> >>  817c8880 8801c1bf7978 817c80a5 880214f58fc0
> >> Call Trace:
> >>  [] ? bit_wait+0x50/0x50
> >>  [] schedule+0x35/0x80
> >>  [] schedule_timeout+0x292/0x3d0
> >>  [] ? xen_clocksource_get_cycles+0x15/0x20
> >>  [] ? ktime_get+0x3c/0xb0
> >>  [] ? bit_wait+0x50/0x50
> >>  [] io_schedule_timeout+0xa6/0x110
> >>  [] bit_wait_io+0x1b/0x60
> >>  [] __wait_on_bit+0x64/0x90
> >>  [] wait_on_page_bit+0xc4/0xd0
> >>  [] ? autoremove_wake_function+0x40/0x40
> >>  [] truncate_inode_pages_range+0x409/0x840
> >>  [] ? pcpu_free_area+0x13d/0x1a0
> >>  [] ? wake_up_bit+0x25/0x30
> >>  [] truncate_inode_pages_final+0x4c/0x60
> >>  [] f2fs_evict_inode+0x48/0x390 [f2fs]
> >>  [] evict+0xc7/0x1a0
> >>  [] iput+0x197/0x200
> >>  [] f2fs_fill_super+0xab2/0x1130 [f2fs]
> >>  [] mount_bdev+0x184/0x1c0
> >>  [] ? f2fs_commit_super+0x100/0x100 [f2fs]
> >>  [] f2fs_mount+0x15/0x20 [f2fs]
> >>  [] mount_fs+0x39/0x160
> >>  [] vfs_kern_mount+0x67/0x110
> >>  [] do_mount+0x1bb/0xc80
> >>  [] SyS_mount+0x83/0xd0
> >>  [] do_syscall_64+0x6e/0x170
> >>  [] entry_SYSCALL64_slow_path+0x25/0x25
> >>
> >> Any thoughts?
> > 
> > I think

Re: [f2fs-dev] [PATCH] fscrypto: lock inode while setting encryption policy

2016-09-29 Thread Richard Weinberger 
On 28.09.2016 20:34, Eric Biggers wrote:
> i_rwsem needs to be acquired while setting an encryption policy so that
> concurrent calls to FS_IOC_SET_ENCRYPTION_POLICY are correctly
> serialized (especially the ->get_context() + ->set_context() pair), and
> so that new files cannot be created in the directory during or after the
> ->empty_dir() check.
> 
> Signed-off-by: Eric Biggers 

Reviewed-by: Richard Weinberger 

Thanks,
//richard

--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [trivial PATCH] f2fs: remove dead variable

2016-09-29 Thread Chao Yu 
On 2016/9/29 18:37, Sheng Yong wrote:
> Signed-off-by: Sheng Yong 

Acked-by: Chao Yu 


--
___
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/2] f2fs: use crc and cp version to determine roll-forward recovery

2016-09-29 Thread Chao Yu 
On 2016/9/20 10:55, Jaegeuk Kim wrote:
> Previously, we used cp_version only to detect recoverable dnodes.
> In order to avoid same garbage cp_version, we needed to truncate the next
> dnode during checkpoint, resulting in additional discard or data write.
> If we can distinguish this by using crc in addition to cp_version, we can
> remove this overhead.
> 
> There is backward compatibility concern where it changes node_footer layout.
> But, it only affects the direct nodes written after the last checkpoint.
> We simply expect that user would change kernel versions back and forth after
> stable checkpoint.

Seems with new released v4.8 f2fs, old image with recoverable data could be
mounted successfully, but meanwhile all fsynced data which needs to be recovered
will be lost w/o any hints?

Could we release a new version mkfs paired with new kernel module, so we can tag
image as a new layout one, then new kernel module can recognize the image layout
and adjust version suited comparing method with old or new image?

Thanks,




--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 3/4] fsck.f2fs: fix a typo in check_sector_size

2016-09-29 Thread Junling Zheng 
Signed-off-by: Junling Zheng 
---
 fsck/mount.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fsck/mount.c b/fsck/mount.c
index a247dec..5f51009 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -1908,7 +1908,7 @@ static int check_sector_size(struct f2fs_super_block *sb)
DBG(1, "\tWriting super block, at offset 0x%08x\n", 0);
for (index = 0; index < 2; index++) {
if (dev_write(zero_buff, index * F2FS_BLKSIZE, F2FS_BLKSIZE)) {
-   MSG(1, "\tError: While while writing supe_blk "
+   MSG(1, "\tError: Failed while writing supe_blk "
"on disk!!! index : %d\n", index);
free(zero_buff);
return -1;
-- 
2.7.4


--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 2/4] fsck.f2fs: fix incorrect ERR_MSG in f2fs_do_mount

2016-09-29 Thread Junling Zheng 
Signed-off-by: Junling Zheng 
---
 fsck/mount.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fsck/mount.c b/fsck/mount.c
index e390b26..a247dec 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -1978,7 +1978,7 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
}
 
if (build_node_manager(sbi)) {
-   ERR_MSG("build_segment_manager failed\n");
+   ERR_MSG("build_node_manager failed\n");
return -1;
}
 
-- 
2.7.4


--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 1/2] f2fs: fix to commit bio cache after flushing node pages

2016-09-29 Thread Chao Yu 
In sync_node_pages, we won't check and commit last merged pages in private
bio cache of f2fs, as these pages were taged as writeback, someone who is
waiting for writebacking of the page will be blocked until the cache was
committed by someone else.

We need to commit node type bio cache to avoid potential deadlock or long
delay of waiting writeback.

Signed-off-by: Chao Yu 
---
 fs/f2fs/node.c | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 9faddcd..8831035 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1312,6 +1312,7 @@ int fsync_node_pages(struct f2fs_sb_info *sbi, struct 
inode *inode,
struct page *last_page = NULL;
bool marked = false;
nid_t ino = inode->i_ino;
+   int nwritten = 0;
 
if (atomic) {
last_page = last_fsync_dnode(sbi, ino);
@@ -1385,7 +1386,10 @@ continue_unlock:
unlock_page(page);
f2fs_put_page(last_page, 0);
break;
+   } else {
+   nwritten++;
}
+
if (page == last_page) {
f2fs_put_page(page, 0);
marked = true;
@@ -1407,6 +1411,9 @@ continue_unlock:
unlock_page(last_page);
goto retry;
}
+
+   if (nwritten)
+   f2fs_submit_merged_bio_cond(sbi, NULL, NULL, ino, NODE, WRITE);
return ret ? -EIO: 0;
 }
 
@@ -1416,6 +1423,7 @@ int sync_node_pages(struct f2fs_sb_info *sbi, struct 
writeback_control *wbc)
struct pagevec pvec;
int step = 0;
int nwritten = 0;
+   int ret = 0;
 
pagevec_init(, 0);
 
@@ -1436,7 +1444,8 @@ next_step:
 
if (unlikely(f2fs_cp_error(sbi))) {
pagevec_release();
-   return -EIO;
+   ret = -EIO;
+   goto out;
}
 
/*
@@ -1487,6 +1496,8 @@ continue_unlock:
 
if (NODE_MAPPING(sbi)->a_ops->writepage(page, wbc))
unlock_page(page);
+   else
+   nwritten++;
 
if (--wbc->nr_to_write == 0)
break;
@@ -1504,7 +1515,10 @@ continue_unlock:
step++;
goto next_step;
}
-   return nwritten;
+out:
+   if (nwritten)
+   f2fs_submit_merged_bio(sbi, NODE, WRITE);
+   return ret;
 }
 
 int wait_on_node_pages_writeback(struct f2fs_sb_info *sbi, nid_t ino)
-- 
2.8.2.311.gee88674


--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 2/2] f2fs: don't submit irrelevant page

2016-09-29 Thread Chao Yu 
While we call ->writepages, there are two cases:
a. we didn't writeout any dirty pages, since they are writebacked by other
thread concurrently.
b. we writeout dirty pages, and have already submitted bio to block layer.

In these cases, we don't need to do additional bio flushing unnecessarily,
it may split bio in cache into smaller one.

Signed-off-by: Chao Yu 
---
 fs/f2fs/data.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 8b9a1dc..0d0177c 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1355,6 +1355,7 @@ static int f2fs_write_cache_pages(struct address_space 
*mapping,
int cycled;
int range_whole = 0;
int tag;
+   int nwritten = 0;
 
pagevec_init(, 0);
 
@@ -1429,6 +1430,8 @@ continue_unlock:
done_index = page->index + 1;
done = 1;
break;
+   } else {
+   nwritten++;
}
 
if (--wbc->nr_to_write <= 0 &&
@@ -1450,6 +1453,10 @@ continue_unlock:
if (wbc->range_cyclic || (range_whole && wbc->nr_to_write > 0))
mapping->writeback_index = done_index;
 
+   if (nwritten)
+   f2fs_submit_merged_bio_cond(F2FS_M_SB(mapping), mapping->host,
+   NULL, 0, DATA, WRITE);
+
return ret;
 }
 
@@ -1491,7 +1498,6 @@ static int f2fs_write_data_pages(struct address_space 
*mapping,
 * if some pages were truncated, we cannot guarantee its mapping->host
 * to detect pending bios.
 */
-   f2fs_submit_merged_bio(sbi, DATA, WRITE);
 
remove_dirty_inode(inode);
return ret;
-- 
2.8.2.311.gee88674


--
___
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/2] f2fs: fix to commit bio cache after flushing node pages

2016-09-29 Thread Chao Yu 
On 2016/9/29 4:19, Jaegeuk Kim wrote:
> On Tue, Sep 27, 2016 at 10:09:03AM +0800, Chao Yu wrote:
>> On 2016/9/27 9:39, Jaegeuk Kim wrote:
>>> On Tue, Sep 27, 2016 at 08:57:41AM +0800, Chao Yu wrote:
 Hi Jaegeuk,

 On 2016/9/27 2:33, Jaegeuk Kim wrote:
> Hi Chao,
>
> On Tue, Sep 27, 2016 at 12:09:52AM +0800, Chao Yu wrote:
>> From: Chao Yu 
>>
>> In sync_node_pages, we won't check and commit last merged pages in 
>> private
>> bio cache of f2fs, as these pages were taged as writeback, someone who is
>> waiting for writebacking of the page will be blocked until the cache was
>> committed by someone else.
>>
>> We need to commit node type bio cache to avoid potential deadlock or long
>> delay of waiting writeback.
>>
>> Signed-off-by: Chao Yu 
>> ---
>>  fs/f2fs/node.c | 11 +--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>> index 9faddcd..f73f774 100644
>> --- a/fs/f2fs/node.c
>> +++ b/fs/f2fs/node.c
>> @@ -1416,6 +1416,7 @@ int sync_node_pages(struct f2fs_sb_info *sbi, 
>> struct writeback_control *wbc)
>>  struct pagevec pvec;
>>  int step = 0;
>>  int nwritten = 0;
>> +int ret = 0;
>>  
>>  pagevec_init(, 0);
>>  
>> @@ -1436,7 +1437,8 @@ next_step:
>>  
>>  if (unlikely(f2fs_cp_error(sbi))) {
>>  pagevec_release();
>> -return -EIO;
>> +ret = -EIO;
>> +goto out;
>>  }
>>  
>>  /*
>> @@ -1487,6 +1489,8 @@ continue_unlock:
>>  
>>  if (NODE_MAPPING(sbi)->a_ops->writepage(page, 
>> wbc))
>>  unlock_page(page);
>> +else
>> +nwritten++;
>>  
>>  if (--wbc->nr_to_write == 0)
>>  break;
>> @@ -1504,7 +1508,10 @@ continue_unlock:
>>  step++;
>>  goto next_step;
>>  }
>> -return nwritten;
>> +out:
>> +if (nwritten)
>> +f2fs_submit_merged_bio(sbi, NODE, WRITE);
>
> IIRC, we don't need to flush this, since f2fs_submit_merged_bio_cond() 
> would
> handle this in f2fs_wait_on_page_writeback().

 Yes, it covers all the cases in f2fs private codes, but there are still 
 some
 codes in mm or fs directory, and they didn't use 
 f2fs_wait_on_page_writeback
 when waiting page writeback. Such as do_writepages && filemap_fdatawait in
 __writeback_single_inode...
>>>
>>> The do_writepages() is okay, which will call f2fs_write_node_pages().
>>> The __writeback_single_inode() won't do filemap_fdatawait() with 
>>> WB_SYNC_ALL.
>>> We don't need to take care of truncation as well.
>>>
>>> Any missing one?
>>
>> Another is: while testing with first version of checkpoint error injection, I
>> encounter below dump stack:
>>
>> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> mount   D 8801c1bf7960 0 97685  97397 0x0008
>>  8801c1bf7960 8801c1bf7930 88017590 8801c1bf7980
>>  8801c1bf8000  7fff 88021f7be340
>>  817c8880 8801c1bf7978 817c80a5 880214f58fc0
>> Call Trace:
>>  [] ? bit_wait+0x50/0x50
>>  [] schedule+0x35/0x80
>>  [] schedule_timeout+0x292/0x3d0
>>  [] ? xen_clocksource_get_cycles+0x15/0x20
>>  [] ? ktime_get+0x3c/0xb0
>>  [] ? bit_wait+0x50/0x50
>>  [] io_schedule_timeout+0xa6/0x110
>>  [] bit_wait_io+0x1b/0x60
>>  [] __wait_on_bit+0x64/0x90
>>  [] wait_on_page_bit+0xc4/0xd0
>>  [] ? autoremove_wake_function+0x40/0x40
>>  [] truncate_inode_pages_range+0x409/0x840
>>  [] ? pcpu_free_area+0x13d/0x1a0
>>  [] ? wake_up_bit+0x25/0x30
>>  [] truncate_inode_pages_final+0x4c/0x60
>>  [] f2fs_evict_inode+0x48/0x390 [f2fs]
>>  [] evict+0xc7/0x1a0
>>  [] iput+0x197/0x200
>>  [] f2fs_fill_super+0xab2/0x1130 [f2fs]
>>  [] mount_bdev+0x184/0x1c0
>>  [] ? f2fs_commit_super+0x100/0x100 [f2fs]
>>  [] f2fs_mount+0x15/0x20 [f2fs]
>>  [] mount_fs+0x39/0x160
>>  [] vfs_kern_mount+0x67/0x110
>>  [] do_mount+0x1bb/0xc80
>>  [] SyS_mount+0x83/0xd0
>>  [] do_syscall_64+0x6e/0x170
>>  [] entry_SYSCALL64_slow_path+0x25/0x25
>>
>> Any thoughts?
> 
> I think this should not happen normally, since f2fs_stop_checkpoint() calls
> f2fs_flush_merged_bios().

In write_end_io, f2fs_stop_checkpoint will not call f2fs_flush_merged_bios.

One other problem here is it can cause latency during waiting writeback:

In fsync()
1. fsync_node_pages a/b/c pages is submitted,

[f2fs-dev] [trivial PATCH] f2fs: remove dead variable

2016-09-29 Thread Sheng Yong 
Signed-off-by: Sheng Yong 
---
 fs/f2fs/gc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index c9b8a67..93985c6 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -275,7 +275,7 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,
 {
struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
struct victim_sel_policy p;
-   unsigned int secno, max_cost, last_victim;
+   unsigned int secno, last_victim;
unsigned int last_segment = MAIN_SEGS(sbi);
unsigned int nsearched = 0;
 
@@ -285,7 +285,7 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,
select_policy(sbi, gc_type, type, );
 
p.min_segno = NULL_SEGNO;
-   p.min_cost = max_cost = get_max_cost(sbi, );
+   p.min_cost = get_max_cost(sbi, );
 
if (p.max_search == 0)
goto out;
-- 
2.9.2


--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel