Re: [f2fs-dev] f2fs: f2fs unmount hangs if f2fs_init_acl() fails during mkdir syscall
Hi, 06.02.2014 10:02, Jaegeuk Kim пишет: Hi, Thank you for the test and valuable report. This bug was fixed recently by: commit 03dea3129d558bf5293a6e9f12777176619ac876 Author: Jaegeuk Kim jaegeuk@samsung.com Date: Wed Feb 5 11:16:39 2014 +0900 f2fs: fix to truncate dentry pages in the error case Now remove_inode_page() succeed, but another assertion failed (tested on revision e964751c): [ 1272.747011] kernel BUG at fs/f2fs/inode.c:274! [ 1272.747011] invalid opcode: [#1] SMP [ 1272.747011] Modules linked in: f2fs kedr_fsim_indicator_common(OF) kedr_fsim_indicator_capable(OF) kedr_fsim_indicator_kmalloc(OF) kedr_fsim_vmm(OF) kedr_fsim_mem_util(OF) kedr_fsim_capable(OF) kedr_fsim_uaccess(OF) kedr_fsim_cmm(OF) kedr_fault_simulation(OF) kedr(OF) fuse nf_conntrack_netbios_ns nf_conntrack_broadcast ipt_MASQUERADE ip6t_REJECT xt_conntrack ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw parport_pc i2c_piix4 e1000 i2c_core microcode parport ata_generic pata_acpi [last unloaded: kedr] [ 1272.747011] CPU: 0 PID: 14613 Comm: fs-driver-tests Tainted: GF W O 3.14.0-rc1fs #1 [ 1272.747011] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 [ 1272.747011] task: 88001e939190 ti: 88000d7ec000 task.ti: 88000d7ec000 [ 1272.747011] RIP: 0010:[a01c74a8] [a01c74a8] f2fs_evict_inode+0x178/0x180 [f2fs] [ 1272.747011] RSP: 0018:88000d7ede50 EFLAGS: 00010202 [ 1272.747011] RAX: 0001 RBX: 88000475cc30 RCX: 88001e9398a0 [ 1272.747011] RDX: 0002 RSI: RDI: 88000475ce10 [ 1272.747011] RBP: 88000d7ede68 R08: R09: [ 1272.747011] R10: R11: 0001 R12: 88000475cc30 [ 1272.747011] R13: 88000f147800 R14: a01e7080 R15: 88000f147b80 [ 1272.747011] FS: 7f1795424740() GS:88003fc0() knlGS: [ 1272.747011] CS: 0010 DS: ES: CR0: 8005003b [ 1272.747011] CR2: 7fc33bfa9000 CR3: 0f14e000 CR4: 06f0 [ 1272.747011] Stack: [ 1272.747011] 88000475cc30 88000475cdc8 a01e7080 88000d7ede90 [ 1272.747011] 811fde03 88000475cc30 88000475ccb8 88000f147000 [ 1272.747011] 88000d7edec0 811fe615 88000475cc30 88000f147800 [ 1272.747011] Call Trace: [ 1272.747011] [811fde03] evict+0xa3/0x1a0 [ 1272.747011] [811fe615] iput+0xf5/0x180 [ 1272.747011] [a01c7f63] f2fs_mkdir+0xf3/0x150 [f2fs] [ 1272.747011] [811f2a77] vfs_mkdir+0xb7/0x160 [ 1272.747011] [811f36bf] SyS_mkdir+0x5f/0xc0 [ 1272.747011] [81680769] system_call_fastpath+0x16/0x1b [ 1272.747011] Code: 01 e1 4c 89 e7 e8 39 59 03 e1 5b 41 5c 41 5d 5d c3 31 c0 49 83 bc 24 c8 00 00 00 01 0f 97 c0 eb 8f 4c 89 e7 e8 fa ec ff ff eb 89 0f 0b 66 0f 1f 44 00 00 0f 1f 44 00 00 55 48 c7 c0 dc ff ff ff [ 1272.747011] RIP [a01c74a8] f2fs_evict_inode+0x178/0x180 [f2fs] [ 1272.747011] RSP 88000d7ede50 Failed assertion claims that dirty dentries counter should be zero when inode is deleted. This counter is incremented by mkdir()- f2fs_add_link()- init_inode_metadata()- make_empty_dir()- set_page_dirty(); but no one decrement it. May be, this should be done along with truncating directory inode in error-path of init_inode_metadata() ? -- Best regards, Andrey Tsyvarev Linux Verification Center, ISPRAS web: http://linuxtesting.org -- Managing the Performance of Cloud-Based Applications Take advantage of what the Cloud has to offer - Avoid Common Pitfalls. Read the Whitepaper. http://pubads.g.doubleclick.net/gampad/clk?id=121051231iu=/4140/ostg.clktrk ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] f2fs: f2fs unmount hangs if f2fs_init_acl() fails during mkdir syscall
Hi, 2014-02-06 (목), 16:17 +0400, Andrey Tsyvarev: Hi, 06.02.2014 10:02, Jaegeuk Kim пишет: Hi, Thank you for the test and valuable report. This bug was fixed recently by: commit 03dea3129d558bf5293a6e9f12777176619ac876 Author: Jaegeuk Kim jaegeuk@samsung.com Date: Wed Feb 5 11:16:39 2014 +0900 f2fs: fix to truncate dentry pages in the error case Now remove_inode_page() succeed, but another assertion failed (tested on revision e964751c): [ 1272.747011] kernel BUG at fs/f2fs/inode.c:274! [ 1272.747011] invalid opcode: [#1] SMP [ 1272.747011] Modules linked in: f2fs kedr_fsim_indicator_common(OF) kedr_fsim_indicator_capable(OF) kedr_fsim_indicator_kmalloc(OF) kedr_fsim_vmm(OF) kedr_fsim_mem_util(OF) kedr_fsim_capable(OF) kedr_fsim_uaccess(OF) kedr_fsim_cmm(OF) kedr_fault_simulation(OF) kedr(OF) fuse nf_conntrack_netbios_ns nf_conntrack_broadcast ipt_MASQUERADE ip6t_REJECT xt_conntrack ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw parport_pc i2c_piix4 e1000 i2c_core microcode parport ata_generic pata_acpi [last unloaded: kedr] [ 1272.747011] CPU: 0 PID: 14613 Comm: fs-driver-tests Tainted: GF W O 3.14.0-rc1fs #1 [ 1272.747011] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 [ 1272.747011] task: 88001e939190 ti: 88000d7ec000 task.ti: 88000d7ec000 [ 1272.747011] RIP: 0010:[a01c74a8] [a01c74a8] f2fs_evict_inode+0x178/0x180 [f2fs] [ 1272.747011] RSP: 0018:88000d7ede50 EFLAGS: 00010202 [ 1272.747011] RAX: 0001 RBX: 88000475cc30 RCX: 88001e9398a0 [ 1272.747011] RDX: 0002 RSI: RDI: 88000475ce10 [ 1272.747011] RBP: 88000d7ede68 R08: R09: [ 1272.747011] R10: R11: 0001 R12: 88000475cc30 [ 1272.747011] R13: 88000f147800 R14: a01e7080 R15: 88000f147b80 [ 1272.747011] FS: 7f1795424740() GS:88003fc0() knlGS: [ 1272.747011] CS: 0010 DS: ES: CR0: 8005003b [ 1272.747011] CR2: 7fc33bfa9000 CR3: 0f14e000 CR4: 06f0 [ 1272.747011] Stack: [ 1272.747011] 88000475cc30 88000475cdc8 a01e7080 88000d7ede90 [ 1272.747011] 811fde03 88000475cc30 88000475ccb8 88000f147000 [ 1272.747011] 88000d7edec0 811fe615 88000475cc30 88000f147800 [ 1272.747011] Call Trace: [ 1272.747011] [811fde03] evict+0xa3/0x1a0 [ 1272.747011] [811fe615] iput+0xf5/0x180 [ 1272.747011] [a01c7f63] f2fs_mkdir+0xf3/0x150 [f2fs] [ 1272.747011] [811f2a77] vfs_mkdir+0xb7/0x160 [ 1272.747011] [811f36bf] SyS_mkdir+0x5f/0xc0 [ 1272.747011] [81680769] system_call_fastpath+0x16/0x1b [ 1272.747011] Code: 01 e1 4c 89 e7 e8 39 59 03 e1 5b 41 5c 41 5d 5d c3 31 c0 49 83 bc 24 c8 00 00 00 01 0f 97 c0 eb 8f 4c 89 e7 e8 fa ec ff ff eb 89 0f 0b 66 0f 1f 44 00 00 0f 1f 44 00 00 55 48 c7 c0 dc ff ff ff [ 1272.747011] RIP [a01c74a8] f2fs_evict_inode+0x178/0x180 [f2fs] [ 1272.747011] RSP 88000d7ede50 Failed assertion claims that dirty dentries counter should be zero when inode is deleted. This counter is incremented by mkdir()- f2fs_add_link()- init_inode_metadata()- make_empty_dir()- set_page_dirty(); but no one decrement it. May be, this should be done along with truncating directory inode in error-path of init_inode_metadata() ? It's weird, since original intention was that pages should be invalidated by: f2fs_evict_inode - truncate_inode_pages - f2fs_invalidate_page - decrement dirty_dents I'll see what happened a little bit more. Thanks, -- Best regards, Andrey Tsyvarev Linux Verification Center, ISPRAS web: http://linuxtesting.org -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- Jaegeuk Kim Samsung -- Managing the Performance of Cloud-Based Applications Take advantage of what the Cloud has to offer - Avoid Common Pitfalls. Read the Whitepaper. http://pubads.g.doubleclick.net/gampad/clk?id=121051231iu=/4140/ostg.clktrk ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] f2fs: f2fs unmount hangs if f2fs_init_acl() fails during mkdir syscall
Hi, It turns out that make_bad_inode prior to iput sets i_mode to a regular file, so that f2fs_evict_inode - truncate_inode_pages - f2fs_invalidate_data_page doesn't decrement dirty_dents. This patch should resolve the bug. Thank you :) When a new directory is allocated, if an error is occurred, we should truncate preallocated dentry pages too. This bug was reported by Andrey Tsyvarev after a while as follows. mkdir()- f2fs_add_link()- init_inode_metadata()- f2fs_init_acl()- f2fs_get_acl()- f2fs_getxattr()- read_all_xattrs() fails. Also there was a BUG_ON triggered after the fault in mkdir()- f2fs_add_link()- init_inode_metadata()- remove_inode_page() - f2fs_bug_on(inode-i_blocks != 0 inode-i_blocks != 1); But, previous patch wasn't perfect to resolve that bug, so the following bug report was also submitted. kernel BUG at fs/f2fs/inode.c:274! Call Trace: [811fde03] evict+0xa3/0x1a0 [811fe615] iput+0xf5/0x180 [a01c7f63] f2fs_mkdir+0xf3/0x150 [f2fs] [811f2a77] vfs_mkdir+0xb7/0x160 [811f36bf] SyS_mkdir+0x5f/0xc0 [81680769] system_call_fastpath+0x16/0x1b Finally, this patch resolves all the issues like below. If an error is occurred after make_empty_dir(), 1. truncate_inode_pages() The make_bad_inode() prior to iput() will change i_mode to S_IFREG, which means that f2fs will not decrement fi-dirty_dents during f2fs_evict_inode. But, by calling it here, we can do that. 2. truncate_blocks() Preallocated dentry pages are trucated here to sync i_blocks. Reported-by: Andrey Tsyvarev tsyva...@ispras.ru Signed-off-by: Jaegeuk Kim jaegeuk@samsung.com --- fs/f2fs/dir.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c index bfcb4ae..92ce1db 100644 --- a/fs/f2fs/dir.c +++ b/fs/f2fs/dir.c @@ -372,6 +372,9 @@ static struct page *init_inode_metadata(struct inode *inode, put_error: f2fs_put_page(page, 1); + /* once the failed inode becomes a bad inode, i_mode is S_IFREG */ + truncate_inode_pages(inode-i_data, 0); + truncate_blocks(inode, 0); error: remove_inode_page(inode); return ERR_PTR(err); -- 1.8.4.474.g128a96c -- Jaegeuk Kim Samsung -- Managing the Performance of Cloud-Based Applications Take advantage of what the Cloud has to offer - Avoid Common Pitfalls. Read the Whitepaper. http://pubads.g.doubleclick.net/gampad/clk?id=121051231iu=/4140/ostg.clktrk ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: clean up redundant function call
This patch integrates inode_[inc|dec]_dirty_dents with inc_page_count to remove redundant calls. Signed-off-by: Jaegeuk Kim jaegeuk@samsung.com --- fs/f2fs/checkpoint.c | 1 - fs/f2fs/data.c | 11 ++- fs/f2fs/dir.c| 4 ++-- fs/f2fs/f2fs.h | 5 + fs/f2fs/gc.c | 7 +-- 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index 8f5dff1..427dd55 100644 --- a/fs/f2fs/checkpoint.c +++ b/fs/f2fs/checkpoint.c @@ -508,7 +508,6 @@ void set_dirty_dir_page(struct inode *inode, struct page *page) if (__add_dirty_inode(inode, new)) kmem_cache_free(inode_entry_slab, new); - inc_page_count(sbi, F2FS_DIRTY_DENTS); inode_inc_dirty_dents(inode); SetPagePrivate(page); spin_unlock(sbi-dir_inode_lock); diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index d175ae3..b401be7 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -799,10 +799,7 @@ static int f2fs_write_data_page(struct page *page, */ offset = i_size (PAGE_CACHE_SIZE - 1); if ((page-index = end_index + 1) || !offset) { - if (S_ISDIR(inode-i_mode)) { - dec_page_count(sbi, F2FS_DIRTY_DENTS); - inode_dec_dirty_dents(inode); - } + inode_dec_dirty_dents(inode); goto out; } @@ -815,7 +812,6 @@ write: /* Dentry blocks are controlled by checkpoint */ if (S_ISDIR(inode-i_mode)) { - dec_page_count(sbi, F2FS_DIRTY_DENTS); inode_dec_dirty_dents(inode); err = do_write_data_page(page, fio); } else { @@ -1033,11 +1029,8 @@ static void f2fs_invalidate_data_page(struct page *page, unsigned int offset, unsigned int length) { struct inode *inode = page-mapping-host; - struct f2fs_sb_info *sbi = F2FS_SB(inode-i_sb); - if (S_ISDIR(inode-i_mode) PageDirty(page)) { - dec_page_count(sbi, F2FS_DIRTY_DENTS); + if (PageDirty(page)) inode_dec_dirty_dents(inode); - } ClearPagePrivate(page); } diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c index 92ce1db..e095a4f 100644 --- a/fs/f2fs/dir.c +++ b/fs/f2fs/dir.c @@ -531,7 +531,6 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page, unsigned int bit_pos; struct address_space *mapping = page-mapping; struct inode *dir = mapping-host; - struct f2fs_sb_info *sbi = F2FS_SB(dir-i_sb); int slots = GET_DENTRY_SLOTS(le16_to_cpu(dentry-name_len)); void *kaddr = page_address(page); int i; @@ -554,6 +553,8 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page, dir-i_ctime = dir-i_mtime = CURRENT_TIME; if (inode) { + struct f2fs_sb_info *sbi = F2FS_SB(dir-i_sb); + if (S_ISDIR(inode-i_mode)) { drop_nlink(dir); update_inode_page(dir); @@ -576,7 +577,6 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page, truncate_hole(dir, page-index, page-index + 1); clear_page_dirty_for_io(page); ClearPageUptodate(page); - dec_page_count(sbi, F2FS_DIRTY_DENTS); inode_dec_dirty_dents(dir); } f2fs_put_page(page, 1); diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index aeff132..4841d12 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -662,6 +662,7 @@ static inline void inc_page_count(struct f2fs_sb_info *sbi, int count_type) static inline void inode_inc_dirty_dents(struct inode *inode) { + inc_page_count(F2FS_SB(inode-i_sb), F2FS_DIRTY_DENTS); atomic_inc(F2FS_I(inode)-dirty_dents); } @@ -672,6 +673,10 @@ static inline void dec_page_count(struct f2fs_sb_info *sbi, int count_type) static inline void inode_dec_dirty_dents(struct inode *inode) { + if (!S_ISDIR(inode-i_mode)) + return; + + dec_page_count(F2FS_SB(inode-i_sb), F2FS_DIRTY_DENTS); atomic_dec(F2FS_I(inode)-dirty_dents); } diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index b0f5762..b161db4 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -531,15 +531,10 @@ static void move_data_page(struct inode *inode, struct page *page, int gc_type) set_page_dirty(page); set_cold_data(page); } else { - struct f2fs_sb_info *sbi = F2FS_SB(inode-i_sb); - f2fs_wait_on_page_writeback(page, DATA); - if (clear_page_dirty_for_io(page) - S_ISDIR(inode-i_mode)) { - dec_page_count(sbi, F2FS_DIRTY_DENTS); + if (clear_page_dirty_for_io(page)) inode_dec_dirty_dents(inode); - } set_cold_data(page);
Re: [f2fs-dev] [PATCH] f2fs: introduce ra_meta_pages to readahead CP/NAT/SIT pages
Hi Chao, Sorry for the delay. When I tested this patch, a deadlock was occurred, so I couldn't afford to look inside in more detail. When I see again, there is a bug wrt blkno. Please see below. 2014-01-28 (화), 10:28 +0800, Chao Yu: This patch help us to cleanup the readahead code by merging ra_{sit,nat}_pages function into ra_meta_pages. Additionally the new function is used to readahead cp block in recover_orphan_inodes. Signed-off-by: Chao Yu chao2...@samsung.com --- fs/f2fs/checkpoint.c | 78 ++ fs/f2fs/f2fs.h | 10 +++ fs/f2fs/node.c | 38 +--- fs/f2fs/segment.c| 43 +--- 4 files changed, 90 insertions(+), 79 deletions(-) diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index 293d048..a1986c3 100644 --- a/fs/f2fs/checkpoint.c +++ b/fs/f2fs/checkpoint.c @@ -75,6 +75,82 @@ out: return page; } +inline int get_max_meta_blks(struct f2fs_sb_info *sbi, int type) +{ + switch (type) { + case META_NAT: + return NM_I(sbi)-max_nid / NAT_ENTRY_PER_BLOCK; + case META_SIT: + return SIT_BLK_CNT(sbi); + case META_CP: + return 0; + default: + BUG(); + } +} + +/* + * Readahead CP/NAT/SIT pages + */ +int ra_meta_pages(struct f2fs_sb_info *sbi, int start, int nrpages, int type) +{ + struct address_space *mapping = sbi-meta_inode-i_mapping; We don't need to do this. Instead, we can use META_MAPPING(sbi) below. + struct page *page; + block_t blk_addr, prev_blk_addr = 0; + int blkno = start; + int max_blks = get_max_meta_blks(sbi, type); block_t prev_blk_addr = 0; struct page *page; int blkno = start; int max_blks = get_max_meta_blks(sbi, type); + struct f2fs_io_info fio = { + .type = META, + .rw = READ_SYNC | REQ_META | REQ_PRIO + }; + + for (blkno = start; blkno start + nrpages; blkno++) { for (; nrpages-- 0; blkno++) { block_t blk_addr; -- In the case of META_NAT, blkno becomes 0 if it hits max_blks. So, we should not use *blkno start + nrpages*. + + switch (type) { + case META_NAT: + /* get nat block addr */ + if (unlikely(blkno = max_blks)) + blkno = 0; + blk_addr = current_nat_addr(sbi, + blkno * NAT_ENTRY_PER_BLOCK); + break; + case META_SIT: + /* get sit block addr */ + if (unlikely(blkno = max_blks)) + goto out; + blk_addr = current_sit_addr(sbi, + blkno * SIT_ENTRY_PER_BLOCK); + if (blkno != start prev_blk_addr + 1 != blk_addr) + goto out; + prev_blk_addr = blk_addr; + break; + case META_CP: + /* get cp block addr */ + blk_addr = blkno; + break; + default: + BUG(); + } + + page = grab_cache_page(mapping, blk_addr); page = grab_cache_page(META_MAPPING(sbi), blk_addr); + if (!page) + continue; + if (PageUptodate(page)) { + mark_page_accessed(page); + f2fs_put_page(page, 1); + continue; + } + + f2fs_submit_page_mbio(sbi, page, blk_addr, fio); + mark_page_accessed(page); + f2fs_put_page(page, 0); + } +out: + f2fs_submit_merged_bio(sbi, META, READ); + return blkno - start; +} + static int f2fs_write_meta_page(struct page *page, struct writeback_control *wbc) { @@ -285,6 +361,8 @@ void recover_orphan_inodes(struct f2fs_sb_info *sbi) start_blk = __start_cp_addr(sbi) + 1; orphan_blkaddr = __start_sum_addr(sbi) - 1; + ra_meta_pages(sbi, start_blk, orphan_blkaddr, META_CP); + for (i = 0; i orphan_blkaddr; i++) { struct page *page = get_meta_page(sbi, start_blk + i); struct f2fs_orphan_block *orphan_blk; diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index af51a0b..69ffc1b 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -88,6 +88,15 @@ enum { SIT_BITMAP }; +/* + * For CP/NAT/SIT readahead + */ +enum { + META_CP, + META_NAT, + META_SIT +}; + /* for the list of orphan inodes */ struct orphan_inode_entry { struct list_head list; /* list head */ @@ -1158,6 +1167,7 @@ void destroy_segment_manager_caches(void); */ struct page *grab_meta_page(struct