Commit 572d8b39 authored by Ryusuke Konishi's avatar Ryusuke Konishi Committed by Linus Torvalds

nilfs2: fix deadlock issue between chcp and thaw ioctls

An fs-thaw ioctl causes deadlock with a chcp or mkcp -s command:

 chcp            D ffff88013870f3d0     0  1325   1324 0x00000004
 ...
 Call Trace:
   nilfs_transaction_begin+0x11c/0x1a0 [nilfs2]
   wake_up_bit+0x20/0x20
   copy_from_user+0x18/0x30 [nilfs2]
   nilfs_ioctl_change_cpmode+0x7d/0xcf [nilfs2]
   nilfs_ioctl+0x252/0x61a [nilfs2]
   do_page_fault+0x311/0x34c
   get_unmapped_area+0x132/0x14e
   do_vfs_ioctl+0x44b/0x490
   __set_task_blocked+0x5a/0x61
   vm_mmap_pgoff+0x76/0x87
   __set_current_blocked+0x30/0x4a
   sys_ioctl+0x4b/0x6f
   system_call_fastpath+0x16/0x1b
 thaw            D ffff88013870d890     0  1352   1351 0x00000004
 ...
 Call Trace:
   rwsem_down_failed_common+0xdb/0x10f
   call_rwsem_down_write_failed+0x13/0x20
   down_write+0x25/0x27
   thaw_super+0x13/0x9e
   do_vfs_ioctl+0x1f5/0x490
   vm_mmap_pgoff+0x76/0x87
   sys_ioctl+0x4b/0x6f
   filp_close+0x64/0x6c
   system_call_fastpath+0x16/0x1b

where the thaw ioctl deadlocked at thaw_super() when called while chcp was
waiting at nilfs_transaction_begin() called from
nilfs_ioctl_change_cpmode().  This deadlock is 100% reproducible.

This is because nilfs_ioctl_change_cpmode() first locks sb->s_umount in
read mode and then waits for unfreezing in nilfs_transaction_begin(),
whereas thaw_super() locks sb->s_umount in write mode.  The locking of
sb->s_umount here was intended to make snapshot mounts and the downgrade
of snapshots to checkpoints exclusive.

This fixes the deadlock issue by replacing the sb->s_umount usage in
nilfs_ioctl_change_cpmode() with a dedicated mutex which protects snapshot
mounts.
Signed-off-by: default avatarRyusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
Cc: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
Tested-by: default avatarRyusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent fe0627e7
...@@ -182,7 +182,7 @@ static int nilfs_ioctl_change_cpmode(struct inode *inode, struct file *filp, ...@@ -182,7 +182,7 @@ static int nilfs_ioctl_change_cpmode(struct inode *inode, struct file *filp,
if (copy_from_user(&cpmode, argp, sizeof(cpmode))) if (copy_from_user(&cpmode, argp, sizeof(cpmode)))
goto out; goto out;
down_read(&inode->i_sb->s_umount); mutex_lock(&nilfs->ns_snapshot_mount_mutex);
nilfs_transaction_begin(inode->i_sb, &ti, 0); nilfs_transaction_begin(inode->i_sb, &ti, 0);
ret = nilfs_cpfile_change_cpmode( ret = nilfs_cpfile_change_cpmode(
...@@ -192,7 +192,7 @@ static int nilfs_ioctl_change_cpmode(struct inode *inode, struct file *filp, ...@@ -192,7 +192,7 @@ static int nilfs_ioctl_change_cpmode(struct inode *inode, struct file *filp,
else else
nilfs_transaction_commit(inode->i_sb); /* never fails */ nilfs_transaction_commit(inode->i_sb); /* never fails */
up_read(&inode->i_sb->s_umount); mutex_unlock(&nilfs->ns_snapshot_mount_mutex);
out: out:
mnt_drop_write_file(filp); mnt_drop_write_file(filp);
return ret; return ret;
......
...@@ -945,6 +945,8 @@ static int nilfs_attach_snapshot(struct super_block *s, __u64 cno, ...@@ -945,6 +945,8 @@ static int nilfs_attach_snapshot(struct super_block *s, __u64 cno,
struct nilfs_root *root; struct nilfs_root *root;
int ret; int ret;
mutex_lock(&nilfs->ns_snapshot_mount_mutex);
down_read(&nilfs->ns_segctor_sem); down_read(&nilfs->ns_segctor_sem);
ret = nilfs_cpfile_is_snapshot(nilfs->ns_cpfile, cno); ret = nilfs_cpfile_is_snapshot(nilfs->ns_cpfile, cno);
up_read(&nilfs->ns_segctor_sem); up_read(&nilfs->ns_segctor_sem);
...@@ -969,6 +971,7 @@ static int nilfs_attach_snapshot(struct super_block *s, __u64 cno, ...@@ -969,6 +971,7 @@ static int nilfs_attach_snapshot(struct super_block *s, __u64 cno,
ret = nilfs_get_root_dentry(s, root, root_dentry); ret = nilfs_get_root_dentry(s, root, root_dentry);
nilfs_put_root(root); nilfs_put_root(root);
out: out:
mutex_unlock(&nilfs->ns_snapshot_mount_mutex);
return ret; return ret;
} }
......
...@@ -76,6 +76,7 @@ struct the_nilfs *alloc_nilfs(struct block_device *bdev) ...@@ -76,6 +76,7 @@ struct the_nilfs *alloc_nilfs(struct block_device *bdev)
nilfs->ns_bdev = bdev; nilfs->ns_bdev = bdev;
atomic_set(&nilfs->ns_ndirtyblks, 0); atomic_set(&nilfs->ns_ndirtyblks, 0);
init_rwsem(&nilfs->ns_sem); init_rwsem(&nilfs->ns_sem);
mutex_init(&nilfs->ns_snapshot_mount_mutex);
INIT_LIST_HEAD(&nilfs->ns_dirty_files); INIT_LIST_HEAD(&nilfs->ns_dirty_files);
INIT_LIST_HEAD(&nilfs->ns_gc_inodes); INIT_LIST_HEAD(&nilfs->ns_gc_inodes);
spin_lock_init(&nilfs->ns_inode_lock); spin_lock_init(&nilfs->ns_inode_lock);
......
...@@ -47,6 +47,7 @@ enum { ...@@ -47,6 +47,7 @@ enum {
* @ns_flags: flags * @ns_flags: flags
* @ns_bdev: block device * @ns_bdev: block device
* @ns_sem: semaphore for shared states * @ns_sem: semaphore for shared states
* @ns_snapshot_mount_mutex: mutex to protect snapshot mounts
* @ns_sbh: buffer heads of on-disk super blocks * @ns_sbh: buffer heads of on-disk super blocks
* @ns_sbp: pointers to super block data * @ns_sbp: pointers to super block data
* @ns_sbwtime: previous write time of super block * @ns_sbwtime: previous write time of super block
...@@ -100,6 +101,7 @@ struct the_nilfs { ...@@ -100,6 +101,7 @@ struct the_nilfs {
struct block_device *ns_bdev; struct block_device *ns_bdev;
struct rw_semaphore ns_sem; struct rw_semaphore ns_sem;
struct mutex ns_snapshot_mount_mutex;
/* /*
* used for * used for
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment