f2fs: quota: fix to avoid warning in dquot_writeback_dquots()

F2FS-fs (dm-59): checkpoint=enable has some unwritten data.

------------[ cut here ]------------
WARNING: CPU: 6 PID: 8013 at fs/quota/dquot.c:691 dquot_writeback_dquots+0x2fc/0x308
pc : dquot_writeback_dquots+0x2fc/0x308
lr : f2fs_quota_sync+0xcc/0x1c4
Call trace:
dquot_writeback_dquots+0x2fc/0x308
f2fs_quota_sync+0xcc/0x1c4
f2fs_write_checkpoint+0x3d4/0x9b0
f2fs_issue_checkpoint+0x1bc/0x2c0
f2fs_sync_fs+0x54/0x150
f2fs_do_sync_file+0x2f8/0x814
__f2fs_ioctl+0x1960/0x3244
f2fs_ioctl+0x54/0xe0
__arm64_sys_ioctl+0xa8/0xe4
invoke_syscall+0x58/0x114

checkpoint and f2fs_remount may race as below, resulting triggering warning
in dquot_writeback_dquots().

atomic write remount
- do_remount
- down_write(&sb->s_umount);
- f2fs_remount
- ioctl
- f2fs_do_sync_file
- f2fs_sync_fs
- f2fs_write_checkpoint
- block_operations
- locked = down_read_trylock(&sbi->sb->s_umount)
: fail to lock due to the write lock was held by remount
- up_write(&sb->s_umount);
- f2fs_quota_sync
- dquot_writeback_dquots
- WARN_ON_ONCE(!rwsem_is_locked(&sb->s_umount))
: trigger warning because s_umount lock was unlocked by remount

If checkpoint comes from mount/umount/remount/freeze/quotactl, caller of
checkpoint has already held s_umount lock, calling dquot_writeback_dquots()
in the context should be safe.

So let's record task to sbi->umount_lock_holder, so that checkpoint can
know whether the lock has held in the context or not by checking current
w/ it.

In addition, in order to not misrepresent caller of checkpoint, we should
not allow to trigger async checkpoint for those callers: mount/umount/remount/
freeze/quotactl.

Fixes: af033b2aa8a8 ("f2fs: guarantee journalled quota data by checkpoint")
Signed-off-by: Chao Yu <chao@kernel.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

authored by

Chao Yu and committed by

Jaegeuk Kim 1 year ago eb85c241 53333cdf

+61 -22

3 changed files

expand all

f2fs

checkpoint.c

f2fs.h

super.c

+9 -6

fs/f2fs/checkpoint.c

··· 1237 1237 retry_flush_quotas: 1238 1238 f2fs_lock_all(sbi); 1239 1239 if (__need_flush_quota(sbi)) { 1240 - int locked; 1240 + bool need_lock = sbi->umount_lock_holder != current; 1241 1241 1242 1242 if (++cnt > DEFAULT_RETRY_QUOTA_FLUSH_COUNT) { 1243 1243 set_sbi_flag(sbi, SBI_QUOTA_SKIP_FLUSH); ··· 1246 1246 } 1247 1247 f2fs_unlock_all(sbi); 1248 1248 1249 - /* only failed during mount/umount/freeze/quotactl */ 1250 - locked = down_read_trylock(&sbi->sb->s_umount); 1251 - f2fs_quota_sync(sbi->sb, -1); 1252 - if (locked) 1249 + /* don't grab s_umount lock during mount/umount/remount/freeze/quotactl */ 1250 + if (!need_lock) { 1251 + f2fs_do_quota_sync(sbi->sb, -1); 1252 + } else if (down_read_trylock(&sbi->sb->s_umount)) { 1253 + f2fs_do_quota_sync(sbi->sb, -1); 1253 1254 up_read(&sbi->sb->s_umount); 1255 + } 1254 1256 cond_resched(); 1255 1257 goto retry_flush_quotas; 1256 1258 } ··· 1869 1867 struct cp_control cpc; 1870 1868 1871 1869 cpc.reason = __get_cp_reason(sbi); 1872 - if (!test_opt(sbi, MERGE_CHECKPOINT) || cpc.reason != CP_SYNC) { 1870 + if (!test_opt(sbi, MERGE_CHECKPOINT) || cpc.reason != CP_SYNC || 1871 + sbi->umount_lock_holder == current) { 1873 1872 int ret; 1874 1873 1875 1874 f2fs_down_write(&sbi->gc_lock);

+2 -1

fs/f2fs/f2fs.h

··· 1659 1659 1660 1660 unsigned int nquota_files; /* # of quota sysfile */ 1661 1661 struct f2fs_rwsem quota_sem; /* blocking cp for flags */ 1662 + struct task_struct *umount_lock_holder; /* s_umount lock holder */ 1662 1663 1663 1664 /* # of pages, see count_type */ 1664 1665 atomic_t nr_pages[NR_COUNT_TYPE]; ··· 3625 3624 void f2fs_inode_synced(struct inode *inode); 3626 3625 int f2fs_dquot_initialize(struct inode *inode); 3627 3626 int f2fs_enable_quota_files(struct f2fs_sb_info *sbi, bool rdonly); 3628 - int f2fs_quota_sync(struct super_block *sb, int type); 3627 + int f2fs_do_quota_sync(struct super_block *sb, int type); 3629 3628 loff_t max_file_blocks(struct inode *inode); 3630 3629 void f2fs_quota_off_umount(struct super_block *sb); 3631 3630 void f2fs_save_errors(struct f2fs_sb_info *sbi, unsigned char flag);

+50 -15

fs/f2fs/super.c

··· 1737 1737 1738 1738 static int f2fs_freeze(struct super_block *sb) 1739 1739 { 1740 + struct f2fs_sb_info *sbi = F2FS_SB(sb); 1741 + 1740 1742 if (f2fs_readonly(sb)) 1741 1743 return 0; 1742 1744 1743 1745 /* IO error happened before */ 1744 - if (unlikely(f2fs_cp_error(F2FS_SB(sb)))) 1746 + if (unlikely(f2fs_cp_error(sbi))) 1745 1747 return -EIO; 1746 1748 1747 1749 /* must be clean, since sync_filesystem() was already called */ 1748 - if (is_sbi_flag_set(F2FS_SB(sb), SBI_IS_DIRTY)) 1750 + if (is_sbi_flag_set(sbi, SBI_IS_DIRTY)) 1749 1751 return -EINVAL; 1750 1752 1753 + sbi->umount_lock_holder = current; 1754 + 1751 1755 /* Let's flush checkpoints and stop the thread. */ 1752 - f2fs_flush_ckpt_thread(F2FS_SB(sb)); 1756 + f2fs_flush_ckpt_thread(sbi); 1757 + 1758 + sbi->umount_lock_holder = NULL; 1753 1759 1754 1760 /* to avoid deadlock on f2fs_evict_inode->SB_FREEZE_FS */ 1755 - set_sbi_flag(F2FS_SB(sb), SBI_IS_FREEZING); 1761 + set_sbi_flag(sbi, SBI_IS_FREEZING); 1756 1762 return 0; 1757 1763 } 1758 1764 ··· 2335 2329 org_mount_opt = sbi->mount_opt; 2336 2330 old_sb_flags = sb->s_flags; 2337 2331 2332 + sbi->umount_lock_holder = current; 2333 + 2338 2334 #ifdef CONFIG_QUOTA 2339 2335 org_mount_opt.s_jquota_fmt = F2FS_OPTION(sbi).s_jquota_fmt; 2340 2336 for (i = 0; i < MAXQUOTAS; i++) { ··· 2560 2552 2561 2553 limit_reserve_root(sbi); 2562 2554 *flags = (*flags & ~SB_LAZYTIME) | (sb->s_flags & SB_LAZYTIME); 2555 + 2556 + sbi->umount_lock_holder = NULL; 2563 2557 return 0; 2564 2558 restore_checkpoint: 2565 2559 if (need_enable_checkpoint) { ··· 2602 2592 #endif 2603 2593 sbi->mount_opt = org_mount_opt; 2604 2594 sb->s_flags = old_sb_flags; 2595 + 2596 + sbi->umount_lock_holder = NULL; 2605 2597 return err; 2606 2598 } 2607 2599 ··· 2920 2908 return ret; 2921 2909 } 2922 2910 2923 - int f2fs_quota_sync(struct super_block *sb, int type) 2911 + int f2fs_do_quota_sync(struct super_block *sb, int type) 2924 2912 { 2925 2913 struct f2fs_sb_info *sbi = F2FS_SB(sb); 2926 2914 struct quota_info *dqopt = sb_dqopt(sb); ··· 2968 2956 return ret; 2969 2957 } 2970 2958 2959 + static int f2fs_quota_sync(struct super_block *sb, int type) 2960 + { 2961 + int ret; 2962 + 2963 + F2FS_SB(sb)->umount_lock_holder = current; 2964 + ret = f2fs_do_quota_sync(sb, type); 2965 + F2FS_SB(sb)->umount_lock_holder = NULL; 2966 + return ret; 2967 + } 2968 + 2971 2969 static int f2fs_quota_on(struct super_block *sb, int type, int format_id, 2972 2970 const struct path *path) 2973 2971 { 2974 2972 struct inode *inode; 2975 - int err; 2973 + int err = 0; 2976 2974 2977 2975 /* if quota sysfile exists, deny enabling quota with specific file */ 2978 2976 if (f2fs_sb_has_quota_ino(F2FS_SB(sb))) { ··· 2993 2971 if (path->dentry->d_sb != sb) 2994 2972 return -EXDEV; 2995 2973 2996 - err = f2fs_quota_sync(sb, type); 2974 + F2FS_SB(sb)->umount_lock_holder = current; 2975 + 2976 + err = f2fs_do_quota_sync(sb, type); 2997 2977 if (err) 2998 - return err; 2978 + goto out; 2999 2979 3000 2980 inode = d_inode(path->dentry); 3001 2981 3002 2982 err = filemap_fdatawrite(inode->i_mapping); 3003 2983 if (err) 3004 - return err; 2984 + goto out; 3005 2985 3006 2986 err = filemap_fdatawait(inode->i_mapping); 3007 2987 if (err) 3008 - return err; 2988 + goto out; 3009 2989 3010 2990 err = dquot_quota_on(sb, type, format_id, path); 3011 2991 if (err) 3012 - return err; 2992 + goto out; 3013 2993 3014 2994 inode_lock(inode); 3015 2995 F2FS_I(inode)->i_flags |= F2FS_QUOTA_DEFAULT_FL; 3016 2996 f2fs_set_inode_flags(inode); 3017 2997 inode_unlock(inode); 3018 2998 f2fs_mark_inode_dirty_sync(inode, false); 3019 - 3020 - return 0; 2999 + out: 3000 + F2FS_SB(sb)->umount_lock_holder = NULL; 3001 + return err; 3021 3002 } 3022 3003 3023 3004 static int __f2fs_quota_off(struct super_block *sb, int type) ··· 3031 3006 if (!inode || !igrab(inode)) 3032 3007 return dquot_quota_off(sb, type); 3033 3008 3034 - err = f2fs_quota_sync(sb, type); 3009 + err = f2fs_do_quota_sync(sb, type); 3035 3010 if (err) 3036 3011 goto out_put; 3037 3012 ··· 3054 3029 struct f2fs_sb_info *sbi = F2FS_SB(sb); 3055 3030 int err; 3056 3031 3032 + F2FS_SB(sb)->umount_lock_holder = current; 3033 + 3057 3034 err = __f2fs_quota_off(sb, type); 3058 3035 3059 3036 /* ··· 3065 3038 */ 3066 3039 if (is_journalled_quota(sbi)) 3067 3040 set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR); 3041 + 3042 + F2FS_SB(sb)->umount_lock_holder = NULL; 3043 + 3068 3044 return err; 3069 3045 } 3070 3046 ··· 3200 3170 return 0; 3201 3171 } 3202 3172 3203 - int f2fs_quota_sync(struct super_block *sb, int type) 3173 + int f2fs_do_quota_sync(struct super_block *sb, int type) 3204 3174 { 3205 3175 return 0; 3206 3176 } ··· 4733 4703 if (err) 4734 4704 goto free_compress_inode; 4735 4705 4706 + sbi->umount_lock_holder = current; 4736 4707 #ifdef CONFIG_QUOTA 4737 4708 /* Enable quota usage during mount */ 4738 4709 if (f2fs_sb_has_quota_ino(sbi) && !f2fs_readonly(sb)) { ··· 4860 4829 f2fs_update_time(sbi, CP_TIME); 4861 4830 f2fs_update_time(sbi, REQ_TIME); 4862 4831 clear_sbi_flag(sbi, SBI_CP_DISABLED_QUICK); 4832 + 4833 + sbi->umount_lock_holder = NULL; 4863 4834 return 0; 4864 4835 4865 4836 sync_free_meta: ··· 4964 4931 struct f2fs_sb_info *sbi = F2FS_SB(sb); 4965 4932 4966 4933 if (sb->s_root) { 4934 + sbi->umount_lock_holder = current; 4935 + 4967 4936 set_sbi_flag(sbi, SBI_IS_CLOSE); 4968 4937 f2fs_stop_gc_thread(sbi); 4969 4938 f2fs_stop_discard_thread(sbi);

Configure Feed

Configure Feed