Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ryusuke/nilfs2

* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ryusuke/nilfs2:
  nilfs2: check size of array structured data exchanged via ioctls
  nilfs2: fix lock order reversal in nilfs_clean_segments ioctl
  nilfs2: fix possible circular locking for get information ioctls
  nilfs2: ensure to clear dirty state when deleting metadata file block
  nilfs2: fix circular locking dependency of writer mutex
  nilfs2: fix possible recovery failure due to block creation without writer

7 files changed, 167 insertions, 149 deletions

diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
index 108d281ebca5..50ff3f2cdf24 100644
--- a/fs/nilfs2/ioctl.c
+++ b/fs/nilfs2/ioctl.c
@@ -25,6 +25,7 @@
 #include <linux/smp_lock.h>	/* lock_kernel(), unlock_kernel() */
 #include <linux/capability.h>	/* capable() */
 #include <linux/uaccess.h>	/* copy_from_user(), copy_to_user() */
+#include <linux/vmalloc.h>
 #include <linux/nilfs2_fs.h>
 #include "nilfs.h"
 #include "segment.h"
@@ -147,29 +148,12 @@ static ssize_t
 nilfs_ioctl_do_get_cpinfo(struct the_nilfs *nilfs, __u64 *posp, int flags,
 			  void *buf, size_t size, size_t nmembs)
 {
-	return nilfs_cpfile_get_cpinfo(nilfs->ns_cpfile, posp, flags, buf,
-				       nmembs);
-}
-
-static int nilfs_ioctl_get_cpinfo(struct inode *inode, struct file *filp,
-				  unsigned int cmd, void __user *argp)
-{
-	struct the_nilfs *nilfs = NILFS_SB(inode->i_sb)->s_nilfs;
-	struct nilfs_argv argv;
 	int ret;
 
-	if (copy_from_user(&argv, argp, sizeof(argv)))
-		return -EFAULT;
-
 	down_read(&nilfs->ns_segctor_sem);
-	ret = nilfs_ioctl_wrap_copy(nilfs, &argv, _IOC_DIR(cmd),
-				    nilfs_ioctl_do_get_cpinfo);
+	ret = nilfs_cpfile_get_cpinfo(nilfs->ns_cpfile, posp, flags, buf,
+				      nmembs);
 	up_read(&nilfs->ns_segctor_sem);
-	if (ret < 0)
-		return ret;
-
-	if (copy_to_user(argp, &argv, sizeof(argv)))
-		ret = -EFAULT;
 	return ret;
 }
 
@@ -195,28 +179,11 @@ static ssize_t
 nilfs_ioctl_do_get_suinfo(struct the_nilfs *nilfs, __u64 *posp, int flags,
 			  void *buf, size_t size, size_t nmembs)
 {
-	return nilfs_sufile_get_suinfo(nilfs->ns_sufile, *posp, buf, nmembs);
-}
-
-static int nilfs_ioctl_get_suinfo(struct inode *inode, struct file *filp,
-				  unsigned int cmd, void __user *argp)
-{
-	struct the_nilfs *nilfs = NILFS_SB(inode->i_sb)->s_nilfs;
-	struct nilfs_argv argv;
 	int ret;
 
-	if (copy_from_user(&argv, argp, sizeof(argv)))
-		return -EFAULT;
-
 	down_read(&nilfs->ns_segctor_sem);
-	ret = nilfs_ioctl_wrap_copy(nilfs, &argv, _IOC_DIR(cmd),
-				    nilfs_ioctl_do_get_suinfo);
+	ret = nilfs_sufile_get_suinfo(nilfs->ns_sufile, *posp, buf, nmembs);
 	up_read(&nilfs->ns_segctor_sem);
-	if (ret < 0)
-		return ret;
-
-	if (copy_to_user(argp, &argv, sizeof(argv)))
-		ret = -EFAULT;
 	return ret;
 }
 
@@ -242,28 +209,11 @@ static ssize_t
 nilfs_ioctl_do_get_vinfo(struct the_nilfs *nilfs, __u64 *posp, int flags,
 			 void *buf, size_t size, size_t nmembs)
 {
-	return nilfs_dat_get_vinfo(nilfs_dat_inode(nilfs), buf, nmembs);
-}
-
-static int nilfs_ioctl_get_vinfo(struct inode *inode, struct file *filp,
-				 unsigned int cmd, void __user *argp)
-{
-	struct the_nilfs *nilfs = NILFS_SB(inode->i_sb)->s_nilfs;
-	struct nilfs_argv argv;
 	int ret;
 
-	if (copy_from_user(&argv, argp, sizeof(argv)))
-		return -EFAULT;
-
 	down_read(&nilfs->ns_segctor_sem);
-	ret = nilfs_ioctl_wrap_copy(nilfs, &argv, _IOC_DIR(cmd),
-				    nilfs_ioctl_do_get_vinfo);
+	ret = nilfs_dat_get_vinfo(nilfs_dat_inode(nilfs), buf, nmembs);
 	up_read(&nilfs->ns_segctor_sem);
-	if (ret < 0)
-		return ret;
-
-	if (copy_to_user(argp, &argv, sizeof(argv)))
-		ret = -EFAULT;
 	return ret;
 }
 
@@ -276,17 +226,21 @@ nilfs_ioctl_do_get_bdescs(struct the_nilfs *nilfs, __u64 *posp, int flags,
 	struct nilfs_bdesc *bdescs = buf;
 	int ret, i;
 
+	down_read(&nilfs->ns_segctor_sem);
 	for (i = 0; i < nmembs; i++) {
 		ret = nilfs_bmap_lookup_at_level(bmap,
 						 bdescs[i].bd_offset,
 						 bdescs[i].bd_level + 1,
 						 &bdescs[i].bd_blocknr);
 		if (ret < 0) {
-			if (ret != -ENOENT)
+			if (ret != -ENOENT) {
+				up_read(&nilfs->ns_segctor_sem);
 				return ret;
+			}
 			bdescs[i].bd_blocknr = 0;
 		}
 	}
+	up_read(&nilfs->ns_segctor_sem);
 	return nmembs;
 }
 
@@ -300,10 +254,11 @@ static int nilfs_ioctl_get_bdescs(struct inode *inode, struct file *filp,
 	if (copy_from_user(&argv, argp, sizeof(argv)))
 		return -EFAULT;
 
-	down_read(&nilfs->ns_segctor_sem);
+	if (argv.v_size != sizeof(struct nilfs_bdesc))
+		return -EINVAL;
+
 	ret = nilfs_ioctl_wrap_copy(nilfs, &argv, _IOC_DIR(cmd),
 				    nilfs_ioctl_do_get_bdescs);
-	up_read(&nilfs->ns_segctor_sem);
 	if (ret < 0)
 		return ret;
 
@@ -346,10 +301,10 @@ static int nilfs_ioctl_move_inode_block(struct inode *inode,
 	return 0;
 }
 
-static ssize_t
-nilfs_ioctl_do_move_blocks(struct the_nilfs *nilfs, __u64 *posp, int flags,
-			   void *buf, size_t size, size_t nmembs)
+static int nilfs_ioctl_move_blocks(struct the_nilfs *nilfs,
+				   struct nilfs_argv *argv, void *buf)
 {
+	size_t nmembs = argv->v_nmembs;
 	struct inode *inode;
 	struct nilfs_vdesc *vdesc;
 	struct buffer_head *bh, *n;
@@ -410,19 +365,10 @@ nilfs_ioctl_do_move_blocks(struct the_nilfs *nilfs, __u64 *posp, int flags,
 	return ret;
 }
 
-static inline int nilfs_ioctl_move_blocks(struct the_nilfs *nilfs,
-					  struct nilfs_argv *argv,
-					  int dir)
-{
-	return nilfs_ioctl_wrap_copy(nilfs, argv, dir,
-				     nilfs_ioctl_do_move_blocks);
-}
-
-static ssize_t
-nilfs_ioctl_do_delete_checkpoints(struct the_nilfs *nilfs, __u64 *posp,
-				  int flags, void *buf, size_t size,
-				  size_t nmembs)
+static int nilfs_ioctl_delete_checkpoints(struct the_nilfs *nilfs,
+					  struct nilfs_argv *argv, void *buf)
 {
+	size_t nmembs = argv->v_nmembs;
 	struct inode *cpfile = nilfs->ns_cpfile;
 	struct nilfs_period *periods = buf;
 	int ret, i;
@@ -436,36 +382,21 @@ nilfs_ioctl_do_delete_checkpoints(struct the_nilfs *nilfs, __u64 *posp,
 	return nmembs;
 }
 
-static inline int nilfs_ioctl_delete_checkpoints(struct the_nilfs *nilfs,
-						 struct nilfs_argv *argv,
-						 int dir)
+static int nilfs_ioctl_free_vblocknrs(struct the_nilfs *nilfs,
+				      struct nilfs_argv *argv, void *buf)
 {
-	return nilfs_ioctl_wrap_copy(nilfs, argv, dir,
-				     nilfs_ioctl_do_delete_checkpoints);
-}
+	size_t nmembs = argv->v_nmembs;
+	int ret;
 
-static ssize_t
-nilfs_ioctl_do_free_vblocknrs(struct the_nilfs *nilfs, __u64 *posp, int flags,
-			      void *buf, size_t size, size_t nmembs)
-{
-	int ret = nilfs_dat_freev(nilfs_dat_inode(nilfs), buf, nmembs);
+	ret = nilfs_dat_freev(nilfs_dat_inode(nilfs), buf, nmembs);
 
 	return (ret < 0) ? ret : nmembs;
 }
 
-static inline int nilfs_ioctl_free_vblocknrs(struct the_nilfs *nilfs,
-					     struct nilfs_argv *argv,
-					     int dir)
-{
-	return nilfs_ioctl_wrap_copy(nilfs, argv, dir,
-				     nilfs_ioctl_do_free_vblocknrs);
-}
-
-static ssize_t
-nilfs_ioctl_do_mark_blocks_dirty(struct the_nilfs *nilfs, __u64 *posp,
-				 int flags, void *buf, size_t size,
-				 size_t nmembs)
+static int nilfs_ioctl_mark_blocks_dirty(struct the_nilfs *nilfs,
+					 struct nilfs_argv *argv, void *buf)
 {
+	size_t nmembs = argv->v_nmembs;
 	struct inode *dat = nilfs_dat_inode(nilfs);
 	struct nilfs_bmap *bmap = NILFS_I(dat)->i_bmap;
 	struct nilfs_bdesc *bdescs = buf;
@@ -504,55 +435,37 @@ nilfs_ioctl_do_mark_blocks_dirty(struct the_nilfs *nilfs, __u64 *posp,
 	return nmembs;
 }
 
-static inline int nilfs_ioctl_mark_blocks_dirty(struct the_nilfs *nilfs,
-						struct nilfs_argv *argv,
-						int dir)
+static int nilfs_ioctl_free_segments(struct the_nilfs *nilfs,
+				     struct nilfs_argv *argv, void *buf)
 {
-	return nilfs_ioctl_wrap_copy(nilfs, argv, dir,
-				     nilfs_ioctl_do_mark_blocks_dirty);
-}
-
-static ssize_t
-nilfs_ioctl_do_free_segments(struct the_nilfs *nilfs, __u64 *posp, int flags,
-			     void *buf, size_t size, size_t nmembs)
-{
-	struct nilfs_sb_info *sbi = nilfs_get_writer(nilfs);
+	size_t nmembs = argv->v_nmembs;
+	struct nilfs_sb_info *sbi = nilfs->ns_writer;
 	int ret;
 
-	if (unlikely(!sbi))
+	if (unlikely(!sbi)) {
+		/* never happens because called for a writable mount */
+		WARN_ON(1);
 		return -EROFS;
+	}
 	ret = nilfs_segctor_add_segments_to_be_freed(
 		NILFS_SC(sbi), buf, nmembs);
-	nilfs_put_writer(nilfs);
 
 	return (ret < 0) ? ret : nmembs;
 }
 
-static inline int nilfs_ioctl_free_segments(struct the_nilfs *nilfs,
-					     struct nilfs_argv *argv,
-					     int dir)
-{
-	return nilfs_ioctl_wrap_copy(nilfs, argv, dir,
-				     nilfs_ioctl_do_free_segments);
-}
-
 int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *nilfs,
-				       void __user *argp)
+				       struct nilfs_argv *argv, void **kbufs)
 {
-	struct nilfs_argv argv[5];
 	const char *msg;
-	int dir, ret;
-
-	if (copy_from_user(argv, argp, sizeof(argv)))
-		return -EFAULT;
+	int ret;
 
-	dir = _IOC_WRITE;
-	ret = nilfs_ioctl_move_blocks(nilfs, &argv[0], dir);
+	ret = nilfs_ioctl_move_blocks(nilfs, &argv[0], kbufs[0]);
 	if (ret < 0) {
 		msg = "cannot read source blocks";
 		goto failed;
 	}
-	ret = nilfs_ioctl_delete_checkpoints(nilfs, &argv[1], dir);
+
+	ret = nilfs_ioctl_delete_checkpoints(nilfs, &argv[1], kbufs[1]);
 	if (ret < 0) {
 		/*
 		 * can safely abort because checkpoints can be removed
@@ -561,7 +474,7 @@ int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *nilfs,
 		msg = "cannot delete checkpoints";
 		goto failed;
 	}
-	ret = nilfs_ioctl_free_vblocknrs(nilfs, &argv[2], dir);
+	ret = nilfs_ioctl_free_vblocknrs(nilfs, &argv[2], kbufs[2]);
 	if (ret < 0) {
 		/*
 		 * can safely abort because DAT file is updated atomically
@@ -570,7 +483,7 @@ int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *nilfs,
 		msg = "cannot delete virtual blocks from DAT file";
 		goto failed;
 	}
-	ret = nilfs_ioctl_mark_blocks_dirty(nilfs, &argv[3], dir);
+	ret = nilfs_ioctl_mark_blocks_dirty(nilfs, &argv[3], kbufs[3]);
 	if (ret < 0) {
 		/*
 		 * can safely abort because the operation is nondestructive.
@@ -578,7 +491,7 @@ int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *nilfs,
 		msg = "cannot mark copying blocks dirty";
 		goto failed;
 	}
-	ret = nilfs_ioctl_free_segments(nilfs, &argv[4], dir);
+	ret = nilfs_ioctl_free_segments(nilfs, &argv[4], kbufs[4]);
 	if (ret < 0) {
 		/*
 		 * can safely abort because this operation is atomic.
@@ -598,9 +511,75 @@ int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *nilfs,
 static int nilfs_ioctl_clean_segments(struct inode *inode, struct file *filp,
 				      unsigned int cmd, void __user *argp)
 {
+	struct nilfs_argv argv[5];
+	const static size_t argsz[5] = {
+		sizeof(struct nilfs_vdesc),
+		sizeof(struct nilfs_period),
+		sizeof(__u64),
+		sizeof(struct nilfs_bdesc),
+		sizeof(__u64),
+	};
+	void __user *base;
+	void *kbufs[5];
+	struct the_nilfs *nilfs;
+	size_t len, nsegs;
+	int n, ret;
+
 	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
-	return nilfs_clean_segments(inode->i_sb, argp);
+
+	if (copy_from_user(argv, argp, sizeof(argv)))
+		return -EFAULT;
+
+	nsegs = argv[4].v_nmembs;
+	if (argv[4].v_size != argsz[4])
+		return -EINVAL;
+	/*
+	 * argv[4] points to segment numbers this ioctl cleans.  We
+	 * use kmalloc() for its buffer because memory used for the
+	 * segment numbers is enough small.
+	 */
+	kbufs[4] = memdup_user((void __user *)(unsigned long)argv[4].v_base,
+			       nsegs * sizeof(__u64));
+	if (IS_ERR(kbufs[4]))
+		return PTR_ERR(kbufs[4]);
+
+	nilfs = NILFS_SB(inode->i_sb)->s_nilfs;
+
+	for (n = 0; n < 4; n++) {
+		ret = -EINVAL;
+		if (argv[n].v_size != argsz[n])
+			goto out_free;
+
+		if (argv[n].v_nmembs > nsegs * nilfs->ns_blocks_per_segment)
+			goto out_free;
+
+		len = argv[n].v_size * argv[n].v_nmembs;
+		base = (void __user *)(unsigned long)argv[n].v_base;
+		if (len == 0) {
+			kbufs[n] = NULL;
+			continue;
+		}
+
+		kbufs[n] = vmalloc(len);
+		if (!kbufs[n]) {
+			ret = -ENOMEM;
+			goto out_free;
+		}
+		if (copy_from_user(kbufs[n], base, len)) {
+			ret = -EFAULT;
+			vfree(kbufs[n]);
+			goto out_free;
+		}
+	}
+
+	ret = nilfs_clean_segments(inode->i_sb, argv, kbufs);
+
+ out_free:
+	while (--n > 0)
+		vfree(kbufs[n]);
+	kfree(kbufs[4]);
+	return ret;
 }
 
 static int nilfs_ioctl_sync(struct inode *inode, struct file *filp,
@@ -621,6 +600,33 @@ static int nilfs_ioctl_sync(struct inode *inode, struct file *filp,
 	return 0;
 }
 
+static int nilfs_ioctl_get_info(struct inode *inode, struct file *filp,
+				unsigned int cmd, void __user *argp,
+				size_t membsz,
+				ssize_t (*dofunc)(struct the_nilfs *,
+						  __u64 *, int,
+						  void *, size_t, size_t))
+
+{
+	struct the_nilfs *nilfs = NILFS_SB(inode->i_sb)->s_nilfs;
+	struct nilfs_argv argv;
+	int ret;
+
+	if (copy_from_user(&argv, argp, sizeof(argv)))
+		return -EFAULT;
+
+	if (argv.v_size != membsz)
+		return -EINVAL;
+
+	ret = nilfs_ioctl_wrap_copy(nilfs, &argv, _IOC_DIR(cmd), dofunc);
+	if (ret < 0)
+		return ret;
+
+	if (copy_to_user(argp, &argv, sizeof(argv)))
+		ret = -EFAULT;
+	return ret;
+}
+
 long nilfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	struct inode *inode = filp->f_dentry->d_inode;
@@ -632,16 +638,21 @@ long nilfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	case NILFS_IOCTL_DELETE_CHECKPOINT:
 		return nilfs_ioctl_delete_checkpoint(inode, filp, cmd, argp);
 	case NILFS_IOCTL_GET_CPINFO:
-		return nilfs_ioctl_get_cpinfo(inode, filp, cmd, argp);
+		return nilfs_ioctl_get_info(inode, filp, cmd, argp,
+					    sizeof(struct nilfs_cpinfo),
+					    nilfs_ioctl_do_get_cpinfo);
 	case NILFS_IOCTL_GET_CPSTAT:
 		return nilfs_ioctl_get_cpstat(inode, filp, cmd, argp);
 	case NILFS_IOCTL_GET_SUINFO:
-		return nilfs_ioctl_get_suinfo(inode, filp, cmd, argp);
+		return nilfs_ioctl_get_info(inode, filp, cmd, argp,
+					    sizeof(struct nilfs_suinfo),
+					    nilfs_ioctl_do_get_suinfo);
 	case NILFS_IOCTL_GET_SUSTAT:
 		return nilfs_ioctl_get_sustat(inode, filp, cmd, argp);
 	case NILFS_IOCTL_GET_VINFO:
-		/* XXX: rename to ??? */
-		return nilfs_ioctl_get_vinfo(inode, filp, cmd, argp);
+		return nilfs_ioctl_get_info(inode, filp, cmd, argp,
+					    sizeof(struct nilfs_vinfo),
+					    nilfs_ioctl_do_get_vinfo);
 	case NILFS_IOCTL_GET_BDESCS:
 		return nilfs_ioctl_get_bdescs(inode, filp, cmd, argp);
 	case NILFS_IOCTL_CLEAN_SEGMENTS:
diff --git a/fs/nilfs2/mdt.c b/fs/nilfs2/mdt.c
index 47dd815433fd..bb78745a0e30 100644
--- a/fs/nilfs2/mdt.c
+++ b/fs/nilfs2/mdt.c
@@ -77,19 +77,22 @@ static int nilfs_mdt_create_block(struct inode *inode, unsigned long block,
 						     void *))
 {
 	struct the_nilfs *nilfs = NILFS_MDT(inode)->mi_nilfs;
-	struct nilfs_sb_info *writer = NULL;
 	struct super_block *sb = inode->i_sb;
 	struct nilfs_transaction_info ti;
 	struct buffer_head *bh;
 	int err;
 
 	if (!sb) {
-		writer = nilfs_get_writer(nilfs);
-		if (!writer) {
+		/*
+		 * Make sure this function is not called from any
+		 * read-only context.
+		 */
+		if (!nilfs->ns_writer) {
+			WARN_ON(1);
 			err = -EROFS;
 			goto out;
 		}
-		sb = writer->s_super;
+		sb = nilfs->ns_writer->s_super;
 	}
 
 	nilfs_transaction_begin(sb, &ti, 0);
@@ -127,8 +130,6 @@ static int nilfs_mdt_create_block(struct inode *inode, unsigned long block,
 		err = nilfs_transaction_commit(sb);
 	else
 		nilfs_transaction_abort(sb);
-	if (writer)
-		nilfs_put_writer(nilfs);
  out:
 	return err;
 }
@@ -299,7 +300,7 @@ int nilfs_mdt_delete_block(struct inode *inode, unsigned long block)
 	int err;
 
 	err = nilfs_bmap_delete(ii->i_bmap, block);
-	if (likely(!err)) {
+	if (!err || err == -ENOENT) {
 		nilfs_mdt_mark_dirty(inode);
 		nilfs_mdt_forget_block(inode, block);
 	}
diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index 3d0c18a16db1..da6fc0bba2e5 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -236,7 +236,8 @@ extern int nilfs_sync_file(struct file *, struct dentry *, int);
 
 /* ioctl.c */
 long nilfs_ioctl(struct file *, unsigned int, unsigned long);
-int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *, void __user *);
+int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *, struct nilfs_argv *,
+				       void **);
 
 /* inode.c */
 extern struct inode *nilfs_new_inode(struct inode *, int);
diff --git a/fs/nilfs2/page.c b/fs/nilfs2/page.c
index 1bfbba9c0e9a..a2692bbc7b50 100644
--- a/fs/nilfs2/page.c
+++ b/fs/nilfs2/page.c
@@ -128,7 +128,8 @@ void nilfs_forget_buffer(struct buffer_head *bh)
 
 	lock_buffer(bh);
 	clear_buffer_nilfs_volatile(bh);
-	if (test_clear_buffer_dirty(bh) && nilfs_page_buffers_clean(page))
+	clear_buffer_dirty(bh);
+	if (nilfs_page_buffers_clean(page))
 		__nilfs_clear_page_dirty(page);
 
 	clear_buffer_uptodate(bh);
diff --git a/fs/nilfs2/recovery.c b/fs/nilfs2/recovery.c
index 4fc081e47d70..57afa9d24061 100644
--- a/fs/nilfs2/recovery.c
+++ b/fs/nilfs2/recovery.c
@@ -407,6 +407,7 @@ void nilfs_dispose_segment_list(struct list_head *head)
 }
 
 static int nilfs_prepare_segment_for_recovery(struct the_nilfs *nilfs,
+					      struct nilfs_sb_info *sbi,
 					      struct nilfs_recovery_info *ri)
 {
 	struct list_head *head = &ri->ri_used_segments;
@@ -421,6 +422,7 @@ static int nilfs_prepare_segment_for_recovery(struct the_nilfs *nilfs,
 	segnum[2] = ri->ri_segnum;
 	segnum[3] = ri->ri_nextnum;
 
+	nilfs_attach_writer(nilfs, sbi);
 	/*
 	 * Releasing the next segment of the latest super root.
 	 * The next segment is invalidated by this recovery.
@@ -459,10 +461,10 @@ static int nilfs_prepare_segment_for_recovery(struct the_nilfs *nilfs,
 	nilfs->ns_pseg_offset = 0;
 	nilfs->ns_seg_seq = ri->ri_seq + 2;
 	nilfs->ns_nextnum = nilfs->ns_segnum = segnum[0];
-	return 0;
 
  failed:
 	/* No need to recover sufile because it will be destroyed on error */
+	nilfs_detach_writer(nilfs, sbi);
 	return err;
 }
 
@@ -728,7 +730,7 @@ int nilfs_recover_logical_segments(struct the_nilfs *nilfs,
 		goto failed;
 
 	if (ri->ri_need_recovery == NILFS_RECOVERY_ROLLFORWARD_DONE) {
-		err = nilfs_prepare_segment_for_recovery(nilfs, ri);
+		err = nilfs_prepare_segment_for_recovery(nilfs, sbi, ri);
 		if (unlikely(err)) {
 			printk(KERN_ERR "NILFS: Error preparing segments for "
 			       "recovery.\n");
diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index fb70ec3be20e..22c7f65c2403 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -2589,7 +2589,8 @@ nilfs_remove_written_gcinodes(struct the_nilfs *nilfs, struct list_head *head)
 	}
 }
 
-int nilfs_clean_segments(struct super_block *sb, void __user *argp)
+int nilfs_clean_segments(struct super_block *sb, struct nilfs_argv *argv,
+			 void **kbufs)
 {
 	struct nilfs_sb_info *sbi = NILFS_SB(sb);
 	struct nilfs_sc_info *sci = NILFS_SC(sbi);
@@ -2606,7 +2607,7 @@ int nilfs_clean_segments(struct super_block *sb, void __user *argp)
 	err = nilfs_init_gcdat_inode(nilfs);
 	if (unlikely(err))
 		goto out_unlock;
-	err = nilfs_ioctl_prepare_clean_segments(nilfs, argp);
+	err = nilfs_ioctl_prepare_clean_segments(nilfs, argv, kbufs);
 	if (unlikely(err))
 		goto out_unlock;
 
diff --git a/fs/nilfs2/segment.h b/fs/nilfs2/segment.h
index a98fc1ed0bbb..476bdd5df5be 100644
--- a/fs/nilfs2/segment.h
+++ b/fs/nilfs2/segment.h
@@ -222,7 +222,8 @@ extern int nilfs_construct_segment(struct super_block *);
 extern int nilfs_construct_dsync_segment(struct super_block *, struct inode *,
 					 loff_t, loff_t);
 extern void nilfs_flush_segment(struct super_block *, ino_t);
-extern int nilfs_clean_segments(struct super_block *, void __user *);
+extern int nilfs_clean_segments(struct super_block *, struct nilfs_argv *,
+				void **);
 
 extern int nilfs_segctor_add_segments_to_be_freed(struct nilfs_sc_info *,
 						  __u64 *, size_t);