Re: [f2fs-dev] [PATCH] fscrypto: use standard macros to compute length of fname ciphertext
On Thu, Sep 22, 2016 at 01:31:49PM -0700, Eric Biggers wrote: > Signed-off-by: Eric BiggersThanks, 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
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 BiggersThanks, 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
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
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
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
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
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
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 BiggersReviewed-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
On 2016/9/29 18:37, Sheng Yong wrote: > Signed-off-by: Sheng YongAcked-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
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
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
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
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
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
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
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