Re: [PATCH v2 1/2] f2fs: quota: do not use GFP_NOFS in f2fs_quota_read()

[Jan Kara]

On Tue 07-07-26 19:39:33, Chao Yu wrote:
> Commit 02117b8ae9c0 ("f2fs: Set GF_NOFS in read_cache_page_gfp while doing
> f2fs_quota_read") adds GFP_NOFS in f2fs_quota_read() to avoid below deadlock:
> 
> - do_sys_open
>  - vfs_open
>   - dquot_file_open
>    - dquot_initialize
>     - dqget
>      - dquot_acquire
>       : locks &dqopt->dqio_mutex (VFS Quota Mutex)
>       - qtree_read_dquot
>        - f2fs_quota_read
>         - read_mapping_page (GFP_KERNEL / allows GFP_FS)
>          - __alloc_pages_nodemask
>           - try_to_free_pages (Direct Reclaim)
>            - prune_icache_sb
>             - evict
>              - f2fs_evict_inode
>               - dquot_drop
>                - dqput
>                 - dquot_commit
>                  : tries to lock &dqopt->dqio_mutex again
>                  ==> DEADLOCK (waiting for itself)
> 
> As Jan Kara mentioned, quota system has fixed this issue w/ commit
> 537e11cdc7a6 ("quota: Prevent memory allocation recursion while holding
> dq_lock"), so this GFP_NOFS flag should be relic, let's use GFP_KERNEL
> instead.
> 
> Cc: Jan Kara <jack@xxxxxxx>
> Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> Signed-off-by: Chao Yu <chao@xxxxxxxxxx>

Looks sound to me. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
> v2:
> - fix to use GFP_KERNEL instead of 0.
>  fs/f2fs/super.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index da468df058eb..b1725ca68010 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -3168,7 +3168,7 @@ static ssize_t f2fs_quota_read(struct super_block *sb, int type, char *data,
>  
>  repeat:
>  		folio = mapping_read_folio_gfp(mapping, off >> PAGE_SHIFT,
> -				GFP_NOFS);
> +				GFP_KERNEL);
>  		if (IS_ERR(folio)) {
>  			if (PTR_ERR(folio) == -ENOMEM) {
>  				memalloc_retry_wait(GFP_NOFS);
> -- 
> 2.49.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR