Skip to content

Commit c8bd617

Browse files
chaseyuTheScarastic
authored andcommitted
f2fs: fix inode cache leak
When testing f2fs with inline_dentry option, generic/342 reports: VFS: Busy inodes after unmount of dm-0. Self-destruct in 5 seconds. Have a nice day... After rmmod f2fs module, kenrel shows following dmesg: ============================================================================= BUG f2fs_inode_cache (Tainted: G O ): Objects remaining in f2fs_inode_cache on __kmem_cache_shutdown() ----------------------------------------------------------------------------- Disabling lock debugging due to kernel taint INFO: Slab 0xf51ca0e0 objects=22 used=1 fp=0xd1e6fc60 flags=0x40004080 CPU: 3 PID: 7455 Comm: rmmod Tainted: G B O 4.6.0-rc4+ TheScarastic#16 Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 00000086 00000086 d062fe18 c13a83a0 f51ca0e0 d062fe38 d062fea4 c11c7276 c1981040 f51ca0e0 00000016 00000001 d1e6fc60 40004080 656a624f 20737463 616d6572 6e696e69 6e692067 66326620 6e695f73 5f65646f 68636163 6e6f2065 Call Trace: [<c13a83a0>] dump_stack+0x5f/0x8f [<c11c7276>] slab_err+0x76/0x80 [<c11cbfc0>] ? __kmem_cache_shutdown+0x100/0x2f0 [<c11cbfc0>] ? __kmem_cache_shutdown+0x100/0x2f0 [<c11cbfe5>] __kmem_cache_shutdown+0x125/0x2f0 [<c1198a38>] kmem_cache_destroy+0x158/0x1f0 [<c176b43d>] ? mutex_unlock+0xd/0x10 [<f8f15aa3>] exit_f2fs_fs+0x4b/0x5a8 [f2fs] [<c10f596c>] SyS_delete_module+0x16c/0x1d0 [<c1001b10>] ? do_fast_syscall_32+0x30/0x1c0 [<c13c59bf>] ? __this_cpu_preempt_check+0xf/0x20 [<c10afa7d>] ? trace_hardirqs_on_caller+0xdd/0x210 [<c10ad50b>] ? trace_hardirqs_off+0xb/0x10 [<c1001b81>] do_fast_syscall_32+0xa1/0x1c0 [<c176d888>] sysenter_past_esp+0x45/0x74 INFO: Object 0xd1e6d9e0 @offset=6624 kmem_cache_destroy f2fs_inode_cache: Slab cache still has objects CPU: 3 PID: 7455 Comm: rmmod Tainted: G B O 4.6.0-rc4+ TheScarastic#16 Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 00000286 00000286 d062fef4 c13a83a0 f174b000 d062ff14 d062ff28 c1198ac7 c197fe18 f3c5b980 d062ff20 000d04f2 d062ff0c d062ff0c d062ff14 d062ff14 f8f20dc0 fffffff5 d062e000 d062ff30 f8f15aa3 d062ff7c c10f596c 73663266 Call Trace: [<c13a83a0>] dump_stack+0x5f/0x8f [<c1198ac7>] kmem_cache_destroy+0x1e7/0x1f0 [<f8f15aa3>] exit_f2fs_fs+0x4b/0x5a8 [f2fs] [<c10f596c>] SyS_delete_module+0x16c/0x1d0 [<c1001b10>] ? do_fast_syscall_32+0x30/0x1c0 [<c13c59bf>] ? __this_cpu_preempt_check+0xf/0x20 [<c10afa7d>] ? trace_hardirqs_on_caller+0xdd/0x210 [<c10ad50b>] ? trace_hardirqs_off+0xb/0x10 [<c1001b81>] do_fast_syscall_32+0xa1/0x1c0 [<c176d888>] sysenter_past_esp+0x45/0x74 The reason is: in recovery flow, we use delayed iput mechanism for directory which has recovered dentry block. It means the reference of inode will be held until last dirty dentry page being writebacked. But when we mount f2fs with inline_dentry option, during recovery, dirent may only be recovered into dir inode page rather than dentry page, so there are no chance for us to release inode reference in ->writepage when writebacking last dentry page. We can call paired iget/iput explicityly for inline_dentry case, but for non-inline_dentry case, iput will call writeback_single_inode to write all data pages synchronously, but during recovery, ->writepages of f2fs skips writing all pages, result in losing dirent. This patch fixes this issue by obsoleting old mechanism, and introduce a new dir_list to hold all directory inodes which has recovered datas until finishing recovery. Signed-off-by: Chao Yu <yuchao0@huawei.com> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
1 parent d051311 commit c8bd617

3 files changed

Lines changed: 31 additions & 43 deletions

File tree

fs/f2fs/checkpoint.c

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -795,19 +795,9 @@ void update_dirty_page(struct inode *inode, struct page *page)
795795
f2fs_trace_pid(page);
796796
}
797797

798-
void add_dirty_dir_inode(struct inode *inode)
799-
{
800-
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
801-
802-
spin_lock(&sbi->inode_lock[DIR_INODE]);
803-
__add_dirty_inode(inode, DIR_INODE);
804-
spin_unlock(&sbi->inode_lock[DIR_INODE]);
805-
}
806-
807798
void remove_dirty_inode(struct inode *inode)
808799
{
809800
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
810-
struct f2fs_inode_info *fi = F2FS_I(inode);
811801
enum inode_type type = S_ISDIR(inode->i_mode) ? DIR_INODE : FILE_INODE;
812802

813803
if (!S_ISDIR(inode->i_mode) && !S_ISREG(inode->i_mode) &&
@@ -817,12 +807,6 @@ void remove_dirty_inode(struct inode *inode)
817807
spin_lock(&sbi->inode_lock[type]);
818808
__remove_dirty_inode(inode, type);
819809
spin_unlock(&sbi->inode_lock[type]);
820-
821-
/* Only from the recovery routine */
822-
if (is_inode_flag_set(fi, FI_DELAY_IPUT)) {
823-
clear_inode_flag(fi, FI_DELAY_IPUT);
824-
iput(inode);
825-
}
826810
}
827811

828812
int sync_dirty_inodes(struct f2fs_sb_info *sbi, enum inode_type type)

fs/f2fs/f2fs.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,7 +1519,6 @@ enum {
15191519
FI_NO_ALLOC, /* should not allocate any blocks */
15201520
FI_FREE_NID, /* free allocated nide */
15211521
FI_UPDATE_DIR, /* should update inode block for consistency */
1522-
FI_DELAY_IPUT, /* used for the recovery */
15231522
FI_NO_EXTENT, /* not to use the extent cache */
15241523
FI_INLINE_XATTR, /* used for inline xattr */
15251524
FI_INLINE_DATA, /* used for inline data*/
@@ -1961,7 +1960,6 @@ void remove_orphan_inode(struct f2fs_sb_info *, nid_t);
19611960
int recover_orphan_inodes(struct f2fs_sb_info *);
19621961
int get_valid_checkpoint(struct f2fs_sb_info *);
19631962
void update_dirty_page(struct inode *, struct page *);
1964-
void add_dirty_dir_inode(struct inode *);
19651963
void remove_dirty_inode(struct inode *);
19661964
int sync_dirty_inodes(struct f2fs_sb_info *, enum inode_type);
19671965
int write_checkpoint(struct f2fs_sb_info *, struct cp_control *);

fs/f2fs/recovery.c

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -89,34 +89,46 @@ static void del_fsync_inode(struct fsync_inode_entry *entry)
8989
kmem_cache_free(fsync_entry_slab, entry);
9090
}
9191

92-
static int recover_dentry(struct inode *inode, struct page *ipage)
92+
static int recover_dentry(struct inode *inode, struct page *ipage,
93+
struct list_head *dir_list)
9394
{
9495
struct f2fs_inode *raw_inode = F2FS_INODE(ipage);
9596
nid_t pino = le32_to_cpu(raw_inode->i_pino);
9697
struct f2fs_dir_entry *de;
9798
struct qstr name;
9899
struct page *page;
99100
struct inode *dir, *einode;
101+
struct fsync_inode_entry *entry;
100102
int err = 0;
101103

102-
dir = f2fs_iget(inode->i_sb, pino);
103-
if (IS_ERR(dir)) {
104-
err = PTR_ERR(dir);
105-
goto out;
104+
entry = get_fsync_inode(dir_list, pino);
105+
if (!entry) {
106+
dir = f2fs_iget(inode->i_sb, pino);
107+
if (IS_ERR(dir)) {
108+
err = PTR_ERR(dir);
109+
goto out;
110+
}
111+
112+
entry = add_fsync_inode(dir_list, dir);
113+
if (!entry) {
114+
err = -ENOMEM;
115+
iput(dir);
116+
goto out;
117+
}
106118
}
107119

108-
if (file_enc_name(inode)) {
109-
iput(dir);
120+
dir = entry->inode;
121+
122+
if (file_enc_name(inode))
110123
return 0;
111-
}
112124

113125
name.len = le32_to_cpu(raw_inode->i_namelen);
114126
name.name = raw_inode->i_name;
115127

116128
if (unlikely(name.len > F2FS_NAME_LEN)) {
117129
WARN_ON(1);
118130
err = -ENAMETOOLONG;
119-
goto out_err;
131+
goto out;
120132
}
121133
retry:
122134
de = f2fs_find_entry(dir, &name, &page);
@@ -142,23 +154,12 @@ static int recover_dentry(struct inode *inode, struct page *ipage)
142154
goto retry;
143155
}
144156
err = __f2fs_add_link(dir, &name, inode, inode->i_ino, inode->i_mode);
145-
if (err)
146-
goto out_err;
147-
148-
if (is_inode_flag_set(F2FS_I(dir), FI_DELAY_IPUT)) {
149-
iput(dir);
150-
} else {
151-
add_dirty_dir_inode(dir);
152-
set_inode_flag(F2FS_I(dir), FI_DELAY_IPUT);
153-
}
154157

155158
goto out;
156159

157160
out_unmap_put:
158161
f2fs_dentry_kunmap(dir, page);
159162
f2fs_put_page(page, 0);
160-
out_err:
161-
iput(dir);
162163
out:
163164
f2fs_msg(inode->i_sb, KERN_NOTICE,
164165
"%s: ino = %x, name = %s, dir = %lx, err = %d",
@@ -501,7 +502,8 @@ static int do_recover_data(struct f2fs_sb_info *sbi, struct inode *inode,
501502
return err;
502503
}
503504

504-
static int recover_data(struct f2fs_sb_info *sbi, struct list_head *head)
505+
static int recover_data(struct f2fs_sb_info *sbi, struct list_head *inode_list,
506+
struct list_head *dir_list)
505507
{
506508
unsigned long long cp_ver = cur_cp_version(F2FS_CKPT(sbi));
507509
struct curseg_info *curseg;
@@ -528,7 +530,7 @@ static int recover_data(struct f2fs_sb_info *sbi, struct list_head *head)
528530
break;
529531
}
530532

531-
entry = get_fsync_inode(head, ino_of_node(page));
533+
entry = get_fsync_inode(inode_list, ino_of_node(page));
532534
if (!entry)
533535
goto next;
534536
/*
@@ -539,7 +541,7 @@ static int recover_data(struct f2fs_sb_info *sbi, struct list_head *head)
539541
if (IS_INODE(page))
540542
recover_inode(entry->inode, page);
541543
if (entry->last_dentry == blkaddr) {
542-
err = recover_dentry(entry->inode, page);
544+
err = recover_dentry(entry->inode, page, dir_list);
543545
if (err) {
544546
f2fs_put_page(page, 1);
545547
break;
@@ -567,6 +569,7 @@ int recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
567569
{
568570
struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_WARM_NODE);
569571
struct list_head inode_list;
572+
struct list_head dir_list;
570573
block_t blkaddr;
571574
int err;
572575
int ret = 0;
@@ -578,6 +581,7 @@ int recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
578581
return -ENOMEM;
579582

580583
INIT_LIST_HEAD(&inode_list);
584+
INIT_LIST_HEAD(&dir_list);
581585

582586
/* prevent checkpoint */
583587
mutex_lock(&sbi->cp_mutex);
@@ -597,12 +601,11 @@ int recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
597601
need_writecp = true;
598602

599603
/* step #2: recover data */
600-
err = recover_data(sbi, &inode_list);
604+
err = recover_data(sbi, &inode_list, &dir_list);
601605
if (!err)
602606
f2fs_bug_on(sbi, !list_empty(&inode_list));
603607
out:
604608
destroy_fsync_dnodes(&inode_list);
605-
kmem_cache_destroy(fsync_entry_slab);
606609

607610
/* truncate meta pages to be used by the recovery */
608611
truncate_inode_pages_range(META_MAPPING(sbi),
@@ -640,5 +643,8 @@ int recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
640643
} else {
641644
mutex_unlock(&sbi->cp_mutex);
642645
}
646+
647+
destroy_fsync_dnodes(&dir_list);
648+
kmem_cache_destroy(fsync_entry_slab);
643649
return ret ? ret: err;
644650
}

0 commit comments

Comments
 (0)