On 04/13, Rokudo Yan wrote:
> There is a potential deadlock in gc thread may happen
> under low memory as below:
>
> gc_thread_func
> -f2fs_gc
> -do_garbage_collect
> -gc_data_segment
> -move_data_block
> -set_page_writeback(fio.encrypted_page);
> -f2fs_submit_page_write
> as f2fs_submit_page_write try to do io merge when possible, so the
> encrypted_page is marked PG_writeback but may not submit to block
> layer immediately, if system enter low memory when gc thread try
> to move next data block, it may do direct reclaim and enter fs layer
> as below:
> -move_data_block
> -f2fs_grab_cache_page(index=?, for_write=false)
> -grab_cache_page
> -find_or_create_page
> -pagecache_get_page
> -__page_cache_alloc -- __GFP_FS is set
> -alloc_pages_node
> -__alloc_pages
> -__alloc_pages_slowpath
> -__alloc_pages_direct_reclaim
> -__perform_reclaim
> -try_to_free_pages
> -do_try_to_free_pages
> -shrink_zones
> -mem_cgroup_soft_limit_reclaim
> -mem_cgroup_soft_reclaim
> -mem_cgroup_shrink_node
> -shrink_node_memcg
> -shrink_list
> -shrink_inactive_list
> -shrink_page_list
> -wait_on_page_writeback -- the page is marked
> writeback during previous move_data_block call
>
> the gc thread wait for the encrypted_page writeback complete,
> but as gc thread held sbi->gc_lock, the writeback & sync thread
> may blocked waiting for sbi->gc_lock, so the bio contain the
> encrypted_page may nerver submit to block layer and complete the
> writeback, which cause deadlock. To avoid this deadlock condition,
> we mark the gc thread with PF_MEMALLOC_NOFS flag, then it will nerver
> enter fs layer when try to alloc cache page during move_data_block.
>
> Signed-off-by: Rokudo Yan <[email protected]>
> ---
> fs/f2fs/gc.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index e020804f7b07..cc71f77b98c8 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -38,6 +38,12 @@ static int gc_thread_func(void *data)
>
> wait_ms = gc_th->min_sleep_time;
>
> + /*
> + * Make sure that no allocations from gc thread will ever
> + * recurse to the fs layer to avoid deadlock as it will
> + * hold sbi->gc_lock during garbage collection
> + */
> + memalloc_nofs_save();
I think this cannot cover all the f2fs_gc() call cases. Can we just avoid by:
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1233,7 +1233,7 @@ static int move_data_block(struct inode *inode, block_t
bidx,
CURSEG_ALL_DATA_ATGC : CURSEG_COLD_DATA;
/* do not read out */
- page = f2fs_grab_cache_page(inode->i_mapping, bidx, false);
+ page = f2fs_grab_cache_page(inode->i_mapping, bidx, true);
if (!page)
return -ENOMEM;
Thanks,
> set_freezable();
> do {
> bool sync_mode, foreground = false;
> --
> 2.25.1
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
