Skip to contentSkip to conversation

f2fs: use kvmalloc, if kmalloc is failed

PATCH In review Jaegeuk Kim 2 participants 8 messages Dec 14, 2018 – Dec 19, 2018 F2FS file system

lore
Share & tools
lore

Jaegeuk Kim

author maintainer
#0 lore raw

PATCH 1/3 f2fs: use kvmalloc, if kmalloc is failed

One report says memalloc failure during mount.

(unwind_backtrace) from [<c010cd4c>] (show_stack+0x10/0x14) (show_stack) from [<c049c6b8>] (dump_stack+0x8c/0xa0) (dump_stack) from [<c024fcf0>] (warn_alloc+0xc4/0x160) (warn_alloc) from [<c0250218>] (__alloc_pages_nodemask+0x3f4/0x10d0) (__alloc_pages_nodemask) from [<c0270450>] (kmalloc_order_trace+0x2c/0x120) (kmalloc_order_trace) from [<c03fa748>] (build_node_manager+0x35c/0x688) (build_node_manager) from [<c03de494>] (f2fs_fill_super+0xf0c/0x16cc) (f2fs_fill_super) from [<c02a5864>] (mount_bdev+0x15c/0x188) (mount_bdev) from [<c03da624>] (f2fs_mount+0x18/0x20) (f2fs_mount) from [<c02a68b8>] (mount_fs+0x158/0x19c) (mount_fs) from [<c02c3c9c>] (vfs_kern_mount+0x78/0x134) (vfs_kern_mount) from [<c02c76ac>] (do_mount+0x474/0xca4) (do_mount) from [<c02c8264>] (SyS_mount+0x94/0xbc)
(SyS_mount) from [<c0108180>] (ret_fast_syscall+0x0/0x48)

Signed-off-by Jaegeuk Kim <[email protected]>
---
 fs/f2fs/acl.c        |  6 ++--
 fs/f2fs/checkpoint.c |  2 +-
 fs/f2fs/data.c       |  2 +-
 fs/f2fs/debug.c      |  2 +-
 fs/f2fs/f2fs.h       | 10 +++++--
 fs/f2fs/gc.c         |  4 +--
 fs/f2fs/inline.c     |  4 +--
 fs/f2fs/namei.c      |  2 +-
 fs/f2fs/node.c       | 10 +++----
 fs/f2fs/segment.c    | 36 +++++++++++------------
 fs/f2fs/super.c      | 68 ++++++++++++++++++++++----------------------
 11 files changed, 76 insertions(+), 70 deletions(-)
fs/f2fs/acl.c +3 -3
index 22f0d17cde43..63e599524085 100644
@@ -160,7 +160,7 @@ static void *f2fs_acl_to_disk(struct f2fs_sb_info *sbi,
 	return (void *)f2fs_acl;
 
 fail:
-	kfree(f2fs_acl);
+	kvfree(f2fs_acl);
 	return ERR_PTR(-EINVAL);
 }
 

@@ -190,7 +190,7 @@ static struct posix_acl *__f2fs_get_acl(struct inode *inode, int type,
 		acl = NULL;
 	else
 		acl = ERR_PTR(retval);
-	kfree(value);
+	kvfree(value);
 
 	return acl;
 }

@@ -240,7 +240,7 @@ static int __f2fs_set_acl(struct inode *inode, int type,
 
 	error = f2fs_setxattr(inode, name_index, "", value, size, ipage, 0);
 
-	kfree(value);
+	kvfree(value);
 	if (!error)
 		set_cached_acl(inode, type, acl);
fs/f2fs/checkpoint.c +1 -1
index 3a25cf22d732..5cba6a8ee55c 100644
@@ -911,7 +911,7 @@ int f2fs_get_valid_checkpoint(struct f2fs_sb_info *sbi)
 	f2fs_put_page(cp1, 1);
 	f2fs_put_page(cp2, 1);
 fail_no_cp:
-	kfree(sbi->ckpt);
+	kvfree(sbi->ckpt);
 	return -EINVAL;
 }
fs/f2fs/data.c +1 -1
index fd3a1e5ab6d9..59d86f692c84 100644
@@ -2573,7 +2573,7 @@ static void f2fs_dio_end_io(struct bio *bio)
 	bio->bi_private = dio->orig_private;
 	bio->bi_end_io = dio->orig_end_io;
 
-	kfree(dio);
+	kvfree(dio);
 
 	bio_endio(bio);
 }
fs/f2fs/debug.c +1 -1
index 1f1230f690ec..11d4448e8e09 100644
@@ -503,7 +503,7 @@ void f2fs_destroy_stats(struct f2fs_sb_info *sbi)
 	list_del(&si->stat_list);
 	mutex_unlock(&f2fs_stat_mutex);
 
-	kfree(si);
+	kvfree(si);
 }
 
 int __init f2fs_create_root_stats(void)
fs/f2fs/f2fs.h +8 -2
index 7cec897146a3..81bd9a2bf22b 100644
@@ -1631,7 +1631,7 @@ static inline void disable_nat_bits(struct f2fs_sb_info *sbi, bool lock)
 	if (lock)
 		spin_lock_irqsave(&sbi->cp_lock, flags);
 	__clear_ckpt_flags(F2FS_CKPT(sbi), CP_NAT_BITS_FLAG);
-	kfree(NM_I(sbi)->nat_bits);
+	kvfree(NM_I(sbi)->nat_bits);
 	NM_I(sbi)->nat_bits = NULL;
 	if (lock)
 		spin_unlock_irqrestore(&sbi->cp_lock, flags);

@@ -2705,12 +2705,18 @@ static inline bool f2fs_may_extent_tree(struct inode *inode)
 static inline void *f2fs_kmalloc(struct f2fs_sb_info *sbi,
 					size_t size, gfp_t flags)
 {
+	void *ret;
+
 	if (time_to_inject(sbi, FAULT_KMALLOC)) {
 		f2fs_show_injection_info(FAULT_KMALLOC);
 		return NULL;
 	}
 
-	return kmalloc(size, flags);
+	ret = kmalloc(size, flags);
+	if (ret)
+		return ret;
+
+	return kvmalloc(size, flags);
 }
 
 static inline void *f2fs_kzalloc(struct f2fs_sb_info *sbi,
fs/f2fs/gc.c +2 -2
index 71462f2e47d4..20ca8141a720 100644
@@ -142,7 +142,7 @@ int f2fs_start_gc_thread(struct f2fs_sb_info *sbi)
 			"f2fs_gc-%u:%u", MAJOR(dev), MINOR(dev));
 	if (IS_ERR(gc_th->f2fs_gc_task)) {
 		err = PTR_ERR(gc_th->f2fs_gc_task);
-		kfree(gc_th);
+		kvfree(gc_th);
 		sbi->gc_thread = NULL;
 	}
 out:

@@ -155,7 +155,7 @@ void f2fs_stop_gc_thread(struct f2fs_sb_info *sbi)
 	if (!gc_th)
 		return;
 	kthread_stop(gc_th->f2fs_gc_task);
-	kfree(gc_th);
+	kvfree(gc_th);
 	sbi->gc_thread = NULL;
 }
fs/f2fs/inline.c +2 -2
index 7b0cff7e6051..0113bebe2ea8 100644
@@ -501,7 +501,7 @@ static int f2fs_move_rehashed_dirents(struct inode *dir, struct page *ipage,
 
 	stat_dec_inline_dir(dir);
 	clear_inode_flag(dir, FI_INLINE_DENTRY);
-	kfree(backup_dentry);
+	kvfree(backup_dentry);
 	return 0;
 recover:
 	lock_page(ipage);

@@ -512,7 +512,7 @@ static int f2fs_move_rehashed_dirents(struct inode *dir, struct page *ipage,
 	set_page_dirty(ipage);
 	f2fs_put_page(ipage, 1);
 
-	kfree(backup_dentry);
+	kvfree(backup_dentry);
 	return err;
 }
fs/f2fs/namei.c +1 -1
index 2d5626939893..62d9829f3a6a 100644
@@ -635,7 +635,7 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 	f2fs_handle_failed_inode(inode);
 out_free_encrypted_link:
 	if (disk_link.name != (unsigned char *)symname)
-		kfree(disk_link.name);
+		kvfree(disk_link.name);
 	return err;
 }
fs/f2fs/node.c +5 -5
index a2273340991f..a08cb2e46547 100644
@@ -3115,17 +3115,17 @@ void f2fs_destroy_node_manager(struct f2fs_sb_info *sbi)
 
 		for (i = 0; i < nm_i->nat_blocks; i++)
 			kvfree(nm_i->free_nid_bitmap[i]);
-		kfree(nm_i->free_nid_bitmap);
+		kvfree(nm_i->free_nid_bitmap);
 	}
 	kvfree(nm_i->free_nid_count);
 
-	kfree(nm_i->nat_bitmap);
-	kfree(nm_i->nat_bits);
+	kvfree(nm_i->nat_bitmap);
+	kvfree(nm_i->nat_bits);
 #ifdef CONFIG_F2FS_CHECK_FS
-	kfree(nm_i->nat_bitmap_mir);
+	kvfree(nm_i->nat_bitmap_mir);
 #endif
 	sbi->nm_info = NULL;
-	kfree(nm_i);
+	kvfree(nm_i);
 }
 
 int __init f2fs_create_node_manager_caches(void)
fs/f2fs/segment.c +18 -18
index 204d31e58967..98649d304a2f 100644
@@ -705,7 +705,7 @@ int f2fs_create_flush_cmd_control(struct f2fs_sb_info *sbi)
 				"f2fs_flush-%u:%u", MAJOR(dev), MINOR(dev));
 	if (IS_ERR(fcc->f2fs_issue_flush)) {
 		err = PTR_ERR(fcc->f2fs_issue_flush);
-		kfree(fcc);
+		kvfree(fcc);
 		SM_I(sbi)->fcc_info = NULL;
 		return err;
 	}

@@ -724,7 +724,7 @@ void f2fs_destroy_flush_cmd_control(struct f2fs_sb_info *sbi, bool free)
 		kthread_stop(flush_thread);
 	}
 	if (free) {
-		kfree(fcc);
+		kvfree(fcc);
 		SM_I(sbi)->fcc_info = NULL;
 	}
 }

@@ -2012,7 +2012,7 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
 				"f2fs_discard-%u:%u", MAJOR(dev), MINOR(dev));
 	if (IS_ERR(dcc->f2fs_issue_discard)) {
 		err = PTR_ERR(dcc->f2fs_issue_discard);
-		kfree(dcc);
+		kvfree(dcc);
 		SM_I(sbi)->dcc_info = NULL;
 		return err;
 	}

@@ -2029,7 +2029,7 @@ static void destroy_discard_cmd_control(struct f2fs_sb_info *sbi)
 
 	f2fs_stop_discard_thread(sbi);
 
-	kfree(dcc);
+	kvfree(dcc);
 	SM_I(sbi)->dcc_info = NULL;
 }
 

@@ -4316,7 +4316,7 @@ static void destroy_dirty_segmap(struct f2fs_sb_info *sbi)
 
 	destroy_victim_secmap(sbi);
 	SM_I(sbi)->dirty_info = NULL;
-	kfree(dirty_i);
+	kvfree(dirty_i);
 }
 
 static void destroy_curseg(struct f2fs_sb_info *sbi)

@@ -4328,10 +4328,10 @@ static void destroy_curseg(struct f2fs_sb_info *sbi)
 		return;
 	SM_I(sbi)->curseg_array = NULL;
 	for (i = 0; i < NR_CURSEG_TYPE; i++) {
-		kfree(array[i].sum_blk);
-		kfree(array[i].journal);
+		kvfree(array[i].sum_blk);
+		kvfree(array[i].journal);
 	}
-	kfree(array);
+	kvfree(array);
 }
 
 static void destroy_free_segmap(struct f2fs_sb_info *sbi)

@@ -4342,7 +4342,7 @@ static void destroy_free_segmap(struct f2fs_sb_info *sbi)
 	SM_I(sbi)->free_info = NULL;
 	kvfree(free_i->free_segmap);
 	kvfree(free_i->free_secmap);
-	kfree(free_i);
+	kvfree(free_i);
 }
 
 static void destroy_sit_info(struct f2fs_sb_info *sbi)

@@ -4355,26 +4355,26 @@ static void destroy_sit_info(struct f2fs_sb_info *sbi)
 
 	if (sit_i->sentries) {
 		for (start = 0; start < MAIN_SEGS(sbi); start++) {
-			kfree(sit_i->sentries[start].cur_valid_map);
+			kvfree(sit_i->sentries[start].cur_valid_map);
 #ifdef CONFIG_F2FS_CHECK_FS
-			kfree(sit_i->sentries[start].cur_valid_map_mir);
+			kvfree(sit_i->sentries[start].cur_valid_map_mir);
 #endif
-			kfree(sit_i->sentries[start].ckpt_valid_map);
-			kfree(sit_i->sentries[start].discard_map);
+			kvfree(sit_i->sentries[start].ckpt_valid_map);
+			kvfree(sit_i->sentries[start].discard_map);
 		}
 	}
-	kfree(sit_i->tmp_map);
+	kvfree(sit_i->tmp_map);
 
 	kvfree(sit_i->sentries);
 	kvfree(sit_i->sec_entries);
 	kvfree(sit_i->dirty_sentries_bitmap);
 
 	SM_I(sbi)->sit_info = NULL;
-	kfree(sit_i->sit_bitmap);
+	kvfree(sit_i->sit_bitmap);
 #ifdef CONFIG_F2FS_CHECK_FS
-	kfree(sit_i->sit_bitmap_mir);
+	kvfree(sit_i->sit_bitmap_mir);
 #endif
-	kfree(sit_i);
+	kvfree(sit_i);
 }
 
 void f2fs_destroy_segment_manager(struct f2fs_sb_info *sbi)

@@ -4390,7 +4390,7 @@ void f2fs_destroy_segment_manager(struct f2fs_sb_info *sbi)
 	destroy_free_segmap(sbi);
 	destroy_sit_info(sbi);
 	sbi->sm_info = NULL;
-	kfree(sm_info);
+	kvfree(sm_info);
 }
 
 int __init f2fs_create_segment_manager_caches(void)
fs/f2fs/super.c +34 -34

218 lines not on this page · raw message

Signature · 1 line
2.19.0.605.g01d371f741-goog
author maintainer
#1 lore raw

PATCH 2/3 f2fs: correct wrong spelling, issing_*

Let's use "queued" instead of "issuing".

Signed-off-by Jaegeuk Kim <[email protected]>
---
 fs/f2fs/debug.c   |  4 ++--
 fs/f2fs/f2fs.h    | 10 +++++-----
 fs/f2fs/segment.c | 26 +++++++++++++-------------
 3 files changed, 20 insertions(+), 20 deletions(-)
fs/f2fs/debug.c +2 -2
index 11d4448e8e09..ebcc121920ba 100644
@@ -64,7 +64,7 @@ static void update_general_status(struct f2fs_sb_info *sbi)
 		si->nr_flushed =
 			atomic_read(&SM_I(sbi)->fcc_info->issued_flush);
 		si->nr_flushing =
-			atomic_read(&SM_I(sbi)->fcc_info->issing_flush);
+			atomic_read(&SM_I(sbi)->fcc_info->queued_flush);
 		si->flush_list_empty =
 			llist_empty(&SM_I(sbi)->fcc_info->issue_list);
 	}

@@ -72,7 +72,7 @@ static void update_general_status(struct f2fs_sb_info *sbi)
 		si->nr_discarded =
 			atomic_read(&SM_I(sbi)->dcc_info->issued_discard);
 		si->nr_discarding =
-			atomic_read(&SM_I(sbi)->dcc_info->issing_discard);
+			atomic_read(&SM_I(sbi)->dcc_info->queued_discard);
 		si->nr_discard_cmd =
 			atomic_read(&SM_I(sbi)->dcc_info->discard_cmd_cnt);
 		si->undiscard_blks = SM_I(sbi)->dcc_info->undiscard_blks;
fs/f2fs/f2fs.h +5 -5
index 81bd9a2bf22b..b634043fe14c 100644
@@ -285,7 +285,7 @@ struct discard_cmd {
 	struct block_device *bdev;	/* bdev */
 	unsigned short ref;		/* reference count */
 	unsigned char state;		/* state */
-	unsigned char issuing;		/* issuing discard */
+	unsigned char queued;		/* queued discard */
 	int error;			/* bio error */
 	spinlock_t lock;		/* for state/bio_ref updating */
 	unsigned short bio_ref;		/* bio reference count */

@@ -327,7 +327,7 @@ struct discard_cmd_control {
 	unsigned int undiscard_blks;		/* # of undiscard blocks */
 	unsigned int next_pos;			/* next discard position */
 	atomic_t issued_discard;		/* # of issued discard */
-	atomic_t issing_discard;		/* # of issing discard */
+	atomic_t queued_discard;		/* # of queued discard */
 	atomic_t discard_cmd_cnt;		/* # of cached cmd count */
 	struct rb_root_cached root;		/* root of discard rb-tree */
 	bool rbtree_check;			/* config for consistence check */

@@ -892,7 +892,7 @@ struct flush_cmd_control {
 	struct task_struct *f2fs_issue_flush;	/* flush thread */
 	wait_queue_head_t flush_wait_queue;	/* waiting queue for wake-up */
 	atomic_t issued_flush;			/* # of issued flushes */
-	atomic_t issing_flush;			/* # of issing flushes */
+	atomic_t queued_flush;			/* # of queued flushes */
 	struct llist_head issue_list;		/* list for command issue */
 	struct llist_node *dispatch_list;	/* list for command dispatch */
 };

@@ -2167,8 +2167,8 @@ static inline bool is_idle(struct f2fs_sb_info *sbi, int type)
 		get_pages(sbi, F2FS_WB_CP_DATA) ||
 		get_pages(sbi, F2FS_DIO_READ) ||
 		get_pages(sbi, F2FS_DIO_WRITE) ||
-		atomic_read(&SM_I(sbi)->dcc_info->issing_discard) ||
-		atomic_read(&SM_I(sbi)->fcc_info->issing_flush))
+		atomic_read(&SM_I(sbi)->dcc_info->queued_discard) ||
+		atomic_read(&SM_I(sbi)->fcc_info->queued_flush))
 		return false;
 	return f2fs_time_over(sbi, type);
 }
fs/f2fs/segment.c +13 -13
index 98649d304a2f..49ea9009ab5a 100644
@@ -620,16 +620,16 @@ int f2fs_issue_flush(struct f2fs_sb_info *sbi, nid_t ino)
 		return 0;
 
 	if (!test_opt(sbi, FLUSH_MERGE)) {
-		atomic_inc(&fcc->issing_flush);
+		atomic_inc(&fcc->queued_flush);
 		ret = submit_flush_wait(sbi, ino);
-		atomic_dec(&fcc->issing_flush);
+		atomic_dec(&fcc->queued_flush);
 		atomic_inc(&fcc->issued_flush);
 		return ret;
 	}
 
-	if (atomic_inc_return(&fcc->issing_flush) == 1 || sbi->s_ndevs > 1) {
+	if (atomic_inc_return(&fcc->queued_flush) == 1 || sbi->s_ndevs > 1) {
 		ret = submit_flush_wait(sbi, ino);
-		atomic_dec(&fcc->issing_flush);
+		atomic_dec(&fcc->queued_flush);
 
 		atomic_inc(&fcc->issued_flush);
 		return ret;

@@ -648,14 +648,14 @@ int f2fs_issue_flush(struct f2fs_sb_info *sbi, nid_t ino)
 
 	if (fcc->f2fs_issue_flush) {
 		wait_for_completion(&cmd.wait);
-		atomic_dec(&fcc->issing_flush);
+		atomic_dec(&fcc->queued_flush);
 	} else {
 		struct llist_node *list;
 
 		list = llist_del_all(&fcc->issue_list);
 		if (!list) {
 			wait_for_completion(&cmd.wait);
-			atomic_dec(&fcc->issing_flush);
+			atomic_dec(&fcc->queued_flush);
 		} else {
 			struct flush_cmd *tmp, *next;
 

@@ -664,7 +664,7 @@ int f2fs_issue_flush(struct f2fs_sb_info *sbi, nid_t ino)
 			llist_for_each_entry_safe(tmp, next, list, llnode) {
 				if (tmp == &cmd) {
 					cmd.ret = ret;
-					atomic_dec(&fcc->issing_flush);
+					atomic_dec(&fcc->queued_flush);
 					continue;
 				}
 				tmp->ret = ret;

@@ -693,7 +693,7 @@ int f2fs_create_flush_cmd_control(struct f2fs_sb_info *sbi)
 	if (!fcc)
 		return -ENOMEM;
 	atomic_set(&fcc->issued_flush, 0);
-	atomic_set(&fcc->issing_flush, 0);
+	atomic_set(&fcc->queued_flush, 0);
 	init_waitqueue_head(&fcc->flush_wait_queue);
 	init_llist_head(&fcc->issue_list);
 	SM_I(sbi)->fcc_info = fcc;

@@ -909,7 +909,7 @@ static struct discard_cmd *__create_discard_cmd(struct f2fs_sb_info *sbi,
 	dc->len = len;
 	dc->ref = 0;
 	dc->state = D_PREP;
-	dc->issuing = 0;
+	dc->queued = 0;
 	dc->error = 0;
 	init_completion(&dc->wait);
 	list_add_tail(&dc->list, pend_list);

@@ -942,7 +942,7 @@ static void __detach_discard_cmd(struct discard_cmd_control *dcc,
 							struct discard_cmd *dc)
 {
 	if (dc->state == D_DONE)
-		atomic_sub(dc->issuing, &dcc->issing_discard);
+		atomic_sub(dc->queued, &dcc->queued_discard);
 
 	list_del(&dc->list);
 	rb_erase_cached(&dc->rb_node, &dcc->root);

@@ -1145,8 +1145,8 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
 		dc->bio_ref++;
 		spin_unlock_irqrestore(&dc->lock, flags);
 
-		atomic_inc(&dcc->issing_discard);
-		dc->issuing++;
+		atomic_inc(&dcc->queued_discard);
+		dc->queued++;
 		list_move_tail(&dc->list, wait_list);
 
 		/* sanity check on discard range */

@@ -1996,7 +1996,7 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
 	INIT_LIST_HEAD(&dcc->fstrim_list);
 	mutex_init(&dcc->cmd_lock);
 	atomic_set(&dcc->issued_discard, 0);
-	atomic_set(&dcc->issing_discard, 0);
+	atomic_set(&dcc->queued_discard, 0);
 	atomic_set(&dcc->discard_cmd_cnt, 0);
 	dcc->nr_discards = 0;
 	dcc->max_discards = MAIN_SEGS(sbi) << sbi->log_blocks_per_seg;
Signature · 1 line
2.19.0.605.g01d371f741-goog
author maintainer
#3 lore raw

PATCH 3/3 f2fs: flush stale issued discard candidates

In reply to Jaegeuk Kim

Sometimes, I could observe # of issuing_discard to be 1 which blocks background jobs due to is_idle()=false.
The only way to get out of it was to trigger gc_urgent. This patch avoids that by checking any candidates as done in the list.

Signed-off-by Jaegeuk Kim <[email protected]>
---
 fs/f2fs/segment.c | 4 ++++
 1 file changed, 4 insertions(+)
fs/f2fs/segment.c +4 -0
index 49ea9009ab5a..acbbc924e518 100644
@@ -1651,6 +1651,10 @@ static int issue_discard_thread(void *data)
 		if (dcc->discard_wake)
 			dcc->discard_wake = 0;
 
+		/* clean up pending candidates before going to sleep */
+		if (atomic_read(&dcc->queued_discard))
+			__wait_all_discard_cmd(sbi, NULL);
+
 		if (try_to_freeze())
 			continue;
 		if (f2fs_readonly(sbi->sb))
Signature · 1 line
2.19.0.605.g01d371f741-goog
#4 lore raw

Re: [f2fs-dev] [PATCH 3/3] f2fs: flush stale issued discard candidates

Quoted 4 lines · Jaegeuk Kim

Sometimes, I could observe # of issuing_discard to be 1 which blocks background jobs due to is_idle()=false.
The only way to get out of it was to trigger gc_urgent. This patch avoids that by checking any candidates as done in the list.

Well, as below code, once we issued discard commands, we will wait all queued discard end their IO, so do you know what flow can cause such condition...?

		issued = __issue_discard_cmd(sbi, &dpolicy);
		if (issued > 0) {
			__wait_all_discard_cmd(sbi, &dpolicy);

Or, I doubt that 'issued' statistical info could be wrong.

Thanks,

Quoted 22 lines
Signed-off-by Jaegeuk Kim <[email protected]>
---
 fs/f2fs/segment.c | 4 ++++
 1 file changed, 4 insertions(+)
fs/f2fs/segment.c +4 -0
index 49ea9009ab5a..acbbc924e518 100644
@@ -1651,6 +1651,10 @@ static int issue_discard_thread(void *data)
 		if (dcc->discard_wake)
 			dcc->discard_wake = 0;
 
+		/* clean up pending candidates before going to sleep */
+		if (atomic_read(&dcc->queued_discard))
+			__wait_all_discard_cmd(sbi, NULL);
+
 		if (try_to_freeze())
 			continue;
 		if (f2fs_readonly(sbi->sb))
author maintainer
#5 lore raw

Re: [f2fs-dev] [PATCH 3/3] f2fs: flush stale issued discard candidates

Quoted 9 lines · Chao Yu
Quoted 4 lines · Jaegeuk Kim

Sometimes, I could observe # of issuing_discard to be 1 which blocks background jobs due to is_idle()=false.
The only way to get out of it was to trigger gc_urgent. This patch avoids that by checking any candidates as done in the list.

Well, as below code, once we issued discard commands, we will wait all queued discard end their IO, so do you know what flow can cause such condition...?

It's very subtle, and I suspect somehow race condition not from here.

Quoted 6 lines
		issued = __issue_discard_cmd(sbi, &dpolicy);
		if (issued > 0) {
			__wait_all_discard_cmd(sbi, &dpolicy);

Or, I doubt that 'issued' statistical info could be wrong.

No, it simply came back to 0 with this patch. So, something is pending in the queue even if it is done.

Thanks,

Quoted 25 lines

Thanks,

Quoted 22 lines
Signed-off-by Jaegeuk Kim <[email protected]>
---
 fs/f2fs/segment.c | 4 ++++
 1 file changed, 4 insertions(+)
fs/f2fs/segment.c +4 -0
index 49ea9009ab5a..acbbc924e518 100644
@@ -1651,6 +1651,10 @@ static int issue_discard_thread(void *data)
 		if (dcc->discard_wake)
 			dcc->discard_wake = 0;
 
+		/* clean up pending candidates before going to sleep */
+		if (atomic_read(&dcc->queued_discard))
+			__wait_all_discard_cmd(sbi, NULL);
+
 		if (try_to_freeze())
 			continue;
 		if (f2fs_readonly(sbi->sb))
#6 lore raw

Re: [f2fs-dev] [PATCH 3/3] f2fs: flush stale issued discard candidates

Quoted 14 lines · Jaegeuk Kim
Quoted 9 lines · Chao Yu
Quoted 4 lines · Jaegeuk Kim

Sometimes, I could observe # of issuing_discard to be 1 which blocks background jobs due to is_idle()=false.
The only way to get out of it was to trigger gc_urgent. This patch avoids that by checking any candidates as done in the list.

Well, as below code, once we issued discard commands, we will wait all queued discard end their IO, so do you know what flow can cause such condition...?

It's very subtle, and I suspect somehow race condition not from here.

Quoted 1 line
Quoted 8 lines
Quoted 5 lines
		issued = __issue_discard_cmd(sbi, &dpolicy);
		if (issued > 0) {
			__wait_all_discard_cmd(sbi, &dpolicy);

Or, I doubt that 'issued' statistical info could be wrong.

No, it simply came back to 0 with this patch. So, something is pending in the queue even if it is done.

__submit_discard_cmd()
{
....

	dc->len = 0;   <---- reset discard length here

	while (total_len && *issued < dpolicy->max_requests && !err) {
		struct bio *bio = NULL;
		unsigned long flags;
		bool last = true;

		if (len > max_discard_blocks) {
			len = max_discard_blocks;  <---- will truncate len to max_discard_blocks
			last = false;
		}

		(*issued)++;
		if (*issued == dpolicy->max_requests)
			last = true;

		dc->len += len;      <----- will update len into dc->len

...
}

__wait_discard_cmd_range()
{
...

		if (dc->len < dpolicy->granularity)
			continue;

So we can only remove discard entry which size is not smaller than dpolicy->granularity, if max_discard_blocks is smaller than granularity, after splitting discard in __submit_discard_cmd(), we may left small-sized entry there.

...
}

Could you check that whether the left discard was skipped being removed due to above reason?

Thanks

Quoted 31 lines

Thanks,

Quoted 17 lines

Thanks,

Quoted 14 lines
Signed-off-by Jaegeuk Kim <[email protected]>
---
 fs/f2fs/segment.c | 4 ++++
 1 file changed, 4 insertions(+)
fs/f2fs/segment.c +0 -0
index 49ea9009ab5a..acbbc924e518 100644
@@ -1651,6 +1651,10 @@ static int issue_discard_thread(void *data)
 		if (dcc->discard_wake)
 			dcc->discard_wake = 0;
Quoted 8 lines

fs/f2fs/segment.c static int issue_discard_thread(void *data) · line 1660

Quoted 8 lines
fs/f2fs/segment.c +4 -0
+		/* clean up pending candidates before going to sleep */
+		if (atomic_read(&dcc->queued_discard))
+			__wait_all_discard_cmd(sbi, NULL);
+
 		if (try_to_freeze())
 			continue;
 		if (f2fs_readonly(sbi->sb))

.

Reviewed-by on v1 1/3
#7 lore raw
In reply to Jaegeuk Kim
Quoted 17 lines · Jaegeuk Kim

One report says memalloc failure during mount.

(unwind_backtrace) from [<c010cd4c>] (show_stack+0x10/0x14) (show_stack) from [<c049c6b8>] (dump_stack+0x8c/0xa0) (dump_stack) from [<c024fcf0>] (warn_alloc+0xc4/0x160) (warn_alloc) from [<c0250218>] (__alloc_pages_nodemask+0x3f4/0x10d0) (__alloc_pages_nodemask) from [<c0270450>] (kmalloc_order_trace+0x2c/0x120) (kmalloc_order_trace) from [<c03fa748>] (build_node_manager+0x35c/0x688) (build_node_manager) from [<c03de494>] (f2fs_fill_super+0xf0c/0x16cc) (f2fs_fill_super) from [<c02a5864>] (mount_bdev+0x15c/0x188) (mount_bdev) from [<c03da624>] (f2fs_mount+0x18/0x20) (f2fs_mount) from [<c02a68b8>] (mount_fs+0x158/0x19c) (mount_fs) from [<c02c3c9c>] (vfs_kern_mount+0x78/0x134) (vfs_kern_mount) from [<c02c76ac>] (do_mount+0x474/0xca4) (do_mount) from [<c02c8264>] (SyS_mount+0x94/0xbc)
(SyS_mount) from [<c0108180>] (ret_fast_syscall+0x0/0x48)

Quoted 1 line
Signed-off-by Jaegeuk Kim <[email protected]>
Reviewed-by Chao Yu <[email protected]>

Thanks,