f2fs: Fix deadlock in shutdown ioctl
PATCH 1/1 f2fs: Fix deadlock in shutdown ioctl
f2fs_ioc_shutdown() ioctl gets stuck in the below path when going down with full sync (F2FS_GOING_DOWN_FULLSYNC) option.
__switch_to+0x90/0xc4 percpu_down_write+0x8c/0xc0 freeze_super+0xec/0x1e4 freeze_bdev+0xc4/0xcc f2fs_ioctl+0xc0c/0x1ce0 f2fs_compat_ioctl+0x98/0x1f0
Fix this by not holding write access during this ioctl.
--- fs/f2fs/file.c | 5 ----- 1 file changed, 5 deletions(-)
fs/f2fs/file.c +0 -5
@@ -1835,10 +1835,6 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)if (get_user (in, (__u32 __user *)arg))return -EFAULT ; - ret =mnt_want_write_file (filp); -if (ret) -return ret; -switch (in) {case F2FS_GOING_DOWN_FULLSYNC : sb =freeze_bdev (sb->s_bdev);
@@ -1878,7 +1874,6 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)f2fs_update_time (sbi,REQ_TIME ); out: -mnt_drop_write_file (filp);return ret; }
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Quoted 4 lines · Sahitya Tummala
f2fs_ioc_shutdown()ioctl gets stuck in the below path when going down with full sync (F2FS_GOING_DOWN_FULLSYNC) option.
Quoted 8 lines
__switch_to+0x90/0xc4 percpu_down_write+0x8c/0xc0 freeze_super+0xec/0x1e4 freeze_bdev+0xc4/0xcc f2fs_ioctl+0xc0c/0x1ce0 f2fs_compat_ioctl+0x98/0x1f0Fix this by not holding write access during this ioctl.
I think we can just remove lock coverage for F2FS_GOING_DOWN_FULLSYNC path, for other path, we need to keep as it is.
Thanks,
Quoted 30 lines
Signed-off-by Sahitya Tummala <[email protected]>--- fs/f2fs/file.c | 5 ----- 1 file changed, 5 deletions(-)fs/f2fs/file.c +0
-5@@ -1835,10 +1835,6 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)if (get_user (in, (__u32 __user *)arg))return -EFAULT ; - ret =mnt_want_write_file (filp); -if (ret) -return ret; -switch (in) {case F2FS_GOING_DOWN_FULLSYNC : sb =freeze_bdev (sb->s_bdev);@@ -1878,7 +1874,6 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)f2fs_update_time (sbi,REQ_TIME ); out: -mnt_drop_write_file (filp);return ret; }
Quoted 12 lines · Chao Yu
Quoted 11 lines · Sahitya Tummala
f2fs_ioc_shutdown()ioctl gets stuck in the below path when going down with full sync (F2FS_GOING_DOWN_FULLSYNC) option.__switch_to+0x90/0xc4 percpu_down_write+0x8c/0xc0 freeze_super+0xec/0x1e4 freeze_bdev+0xc4/0xcc f2fs_ioctl+0xc0c/0x1ce0 f2fs_compat_ioctl+0x98/0x1f0
Quoted 5 lines
Quoted 1 line
Fix this by not holding write access during this ioctl.
I think we can just remove lock coverage for
F2FS_GOING_DOWN_FULLSYNCpath, for other path, we need to keep as it is.
Thanks, I thought about it too but then I checked that XFS shutdown ioctl is not taking any lock for this ioctl. Hence, I followed the same in F2FS. Do you know why XFS is not taking any lock? Is it really needed in shutdown ioctl?
--
Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
Quoted 15 lines · Sahitya Tummala
Quoted 14 lines · Chao Yu
Quoted 12 lines · Sahitya Tummala
f2fs_ioc_shutdown()ioctl gets stuck in the below path when going down with full sync (F2FS_GOING_DOWN_FULLSYNC) option.__switch_to+0x90/0xc4 percpu_down_write+0x8c/0xc0 freeze_super+0xec/0x1e4 freeze_bdev+0xc4/0xcc f2fs_ioctl+0xc0c/0x1ce0 f2fs_compat_ioctl+0x98/0x1f0Fix this by not holding write access during this ioctl.
Quoted 7 lines
Quoted 3 lines
I think we can just remove lock coverage for
F2FS_GOING_DOWN_FULLSYNCpath, for other path, we need to keep as it is.Thanks, I thought about it too but then I checked that XFS shutdown ioctl is not taking any lock for this ioctl. Hence, I followed the same in F2FS. Do you know why XFS is not taking any lock?
I don't know. :(
Quoted 1 line
Is it really needed in shutdown ioctl?
IMO, yes, we should keep freeze and remount be aware of the shutdown operation.
Thanks,
Quoted 6 lines
--
Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
Quoted 6 lines · Chao Yu
Quoted 4 lines
Thanks, I thought about it too but then I checked that XFS shutdown ioctl is not taking any lock for this ioctl. Hence, I followed the same in F2FS. Do you know why XFS is not taking any lock?
I don't know. :(
No problem :)
Quoted 4 lines
Quoted 2 lines
Is it really needed in shutdown ioctl?
IMO, yes, we should keep freeze and remount be aware of the shutdown operation.
Thanks for your inputs. I will send the updated patch set.
--
Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.