diff mbox

[5/5] Btrfs: fix remount vs autodefrag

Message ID 50B3362D.1030409@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Miao Xie Nov. 26, 2012, 9:28 a.m. UTC 
If we remount the fs to close the auto defragment or make the fs R/O, we should
stop the auto defragment.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ctree.h |  1 +
 fs/btrfs/file.c  | 13 +++++++++++++
 fs/btrfs/super.c | 29 +++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+)

Comments

Josef Bacik Dec. 14, 2012, 5:51 p.m. UTC | #1 
On Mon, Nov 26, 2012 at 02:28:13AM -0700, Miao Xie wrote:
> If we remount the fs to close the auto defragment or make the fs R/O, we should
> stop the auto defragment.
> 
> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>

I'm dropping this patch, it causes a deadlock since defrag will need to reserve
metadata which could call writeback_sb_nr_if_idle which does a
down_read(&sb->s_umount).  Figure out another way to fix this and I'll apply it.
Thanks,

Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Miao Xie Dec. 17, 2012, 7:24 a.m. UTC | #2 
On 	fri, 14 Dec 2012 12:51:06 -0500, Josef Bacik wrote:
> On Mon, Nov 26, 2012 at 02:28:13AM -0700, Miao Xie wrote:
>> If we remount the fs to close the auto defragment or make the fs R/O, we should
>> stop the auto defragment.
>>
>> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
> 
> I'm dropping this patch, it causes a deadlock since defrag will need to reserve
> metadata which could call writeback_sb_nr_if_idle which does a
> down_read(&sb->s_umount).  Figure out another way to fix this and I'll apply it.
> Thanks,

Hi, Josef

I forget to point out this patch is based on my patches:
	[PATCH 1/2 RESEND] vfs: re-implement writeback_inodes_sb(_nr)_if_idle() and rename them
	[PATCH 2/2 RESEND] Btrfs: flush all the dirty pages if try_to_writeback_inodes_sb_nr() fails

But I found you implemented a new writeback_sb_nr_if_idle()(Btrfs: fix autodefrag and umount lockup),
with this new function, my patch(Btrfs: fix remount vs autodefrag) also can wrok well.

Thanks
Miao

> 
> Josef
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josef Bacik Dec. 18, 2012, 1:38 p.m. UTC | #3 
On Mon, Dec 17, 2012 at 12:24:53AM -0700, Miao Xie wrote:
> On 	fri, 14 Dec 2012 12:51:06 -0500, Josef Bacik wrote:
> > On Mon, Nov 26, 2012 at 02:28:13AM -0700, Miao Xie wrote:
> >> If we remount the fs to close the auto defragment or make the fs R/O, we should
> >> stop the auto defragment.
> >>
> >> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
> > 
> > I'm dropping this patch, it causes a deadlock since defrag will need to reserve
> > metadata which could call writeback_sb_nr_if_idle which does a
> > down_read(&sb->s_umount).  Figure out another way to fix this and I'll apply it.
> > Thanks,
> 
> Hi, Josef
> 
> I forget to point out this patch is based on my patches:
> 	[PATCH 1/2 RESEND] vfs: re-implement writeback_inodes_sb(_nr)_if_idle() and rename them
> 	[PATCH 2/2 RESEND] Btrfs: flush all the dirty pages if try_to_writeback_inodes_sb_nr() fails
> 
> But I found you implemented a new writeback_sb_nr_if_idle()(Btrfs: fix autodefrag and umount lockup),
> with this new function, my patch(Btrfs: fix remount vs autodefrag) also can wrok well.
> 

Yeah I'll pull them on now.  Once Al takes the vfs patch we can drop my local
function, so when that happens send a patch to remove it please.  Thanks,

Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Miao Xie Jan. 31, 2013, 5:54 a.m. UTC | #4 
Any comments about this patch?

Thanks
Miao

On mon, 26 Nov 2012 17:28:13 +0800, Miao Xie wrote:
> If we remount the fs to close the auto defragment or make the fs R/O, we should
> stop the auto defragment.
> 
> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
> ---
>  fs/btrfs/ctree.h |  1 +
>  fs/btrfs/file.c  | 13 +++++++++++++
>  fs/btrfs/super.c | 29 +++++++++++++++++++++++++++++
>  3 files changed, 43 insertions(+)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 4ce24ce..01d671c 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1759,6 +1759,7 @@ struct btrfs_ioctl_defrag_range_args {
>  
>  #define btrfs_clear_opt(o, opt)		((o) &= ~BTRFS_MOUNT_##opt)
>  #define btrfs_set_opt(o, opt)		((o) |= BTRFS_MOUNT_##opt)
> +#define btrfs_raw_test_opt(o, opt)	((o) & BTRFS_MOUNT_##opt)
>  #define btrfs_test_opt(root, opt)	((root)->fs_info->mount_opt & \
>  					 BTRFS_MOUNT_##opt)
>  /*
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 40b17d0..7aaae56 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -320,8 +320,21 @@ static int __btrfs_run_defrag_inode(struct btrfs_fs_info *fs_info,
>  	range.start = defrag->last_offset;
>  
>  	sb_start_write(fs_info->sb);
> +
> +	/* Avoid defraging files on R/O fs */
> +	if (!down_write_trylock(&fs_info->sb->s_umount)) {
> +		sb_end_write(fs_info->sb);
> +		btrfs_requeue_inode_defrag(inode, defrag);
> +		iput(inode);
> +		return -EBUSY;
> +	}
> +
> +	BUG_ON(fs_info->sb->s_flags & MS_RDONLY);
> +
>  	num_defrag = btrfs_defrag_file(inode, NULL, &range, defrag->transid,
>  				       BTRFS_DEFRAG_BATCH);
> +
> +	up_write(&fs_info->sb->s_umount);
>  	sb_end_write(fs_info->sb);
>  	/*
>  	 * if we filled the whole defrag batch, there
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index b3b041a..2e7beee 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1189,6 +1189,32 @@ static void btrfs_resize_thread_pool(struct btrfs_fs_info *fs_info,
>  	btrfs_set_max_workers(&fs_info->scrub_workers, new_pool_size);
>  }
>  
> +static inline void btrfs_remount_prepare(struct btrfs_fs_info *fs_info,
> +					 unsigned long old_opts, int flags)
> +{
> +	if (btrfs_raw_test_opt(old_opts, AUTO_DEFRAG) &&
> +	    (!btrfs_raw_test_opt(fs_info->mount_opt, AUTO_DEFRAG) ||
> +	     (flags & MS_RDONLY))) {
> +		/* wait for any defraggers to finish */
> +		wait_event(fs_info->transaction_wait,
> +			   (atomic_read(&fs_info->defrag_running) == 0));
> +	}
> +}
> +
> +static inline void btrfs_remount_cleanup(struct btrfs_fs_info *fs_info,
> +					 unsigned long old_opts, int flags)
> +{
> +	/*
> +	 * We remount the fs successfully, then we need cleanup all defragable
> +	 * inodes if the autodefragment is close or the fs is R/O.
> +	 */
> +	if (btrfs_raw_test_opt(old_opts, AUTO_DEFRAG) &&
> +	    (!btrfs_raw_test_opt(fs_info->mount_opt, AUTO_DEFRAG) ||
> +	     (flags & MS_RDONLY)))
> +		btrfs_cleanup_defrag_inodes(fs_info);
> +
> +}
> +
>  static int btrfs_remount(struct super_block *sb, int *flags, char *data)
>  {
>  	struct btrfs_fs_info *fs_info = btrfs_sb(sb);
> @@ -1214,6 +1240,8 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
>  	if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
>  		return 0;
>  
> +	btrfs_remount_prepare(fs_info, old_opts, *flags);
> +
>  	if (*flags & MS_RDONLY) {
>  		sb->s_flags |= MS_RDONLY;
>  
> @@ -1247,6 +1275,7 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
>  		sb->s_flags &= ~MS_RDONLY;
>  	}
>  
> +	btrfs_remount_cleanup(fs_info, old_opts, *flags);
>  	return 0;
>  
>  restore:
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 4ce24ce..01d671c 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1759,6 +1759,7 @@  struct btrfs_ioctl_defrag_range_args {
 
 #define btrfs_clear_opt(o, opt)		((o) &= ~BTRFS_MOUNT_##opt)
 #define btrfs_set_opt(o, opt)		((o) |= BTRFS_MOUNT_##opt)
+#define btrfs_raw_test_opt(o, opt)	((o) & BTRFS_MOUNT_##opt)
 #define btrfs_test_opt(root, opt)	((root)->fs_info->mount_opt & \
 					 BTRFS_MOUNT_##opt)
 /*
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 40b17d0..7aaae56 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -320,8 +320,21 @@  static int __btrfs_run_defrag_inode(struct btrfs_fs_info *fs_info,
 	range.start = defrag->last_offset;
 
 	sb_start_write(fs_info->sb);
+
+	/* Avoid defraging files on R/O fs */
+	if (!down_write_trylock(&fs_info->sb->s_umount)) {
+		sb_end_write(fs_info->sb);
+		btrfs_requeue_inode_defrag(inode, defrag);
+		iput(inode);
+		return -EBUSY;
+	}
+
+	BUG_ON(fs_info->sb->s_flags & MS_RDONLY);
+
 	num_defrag = btrfs_defrag_file(inode, NULL, &range, defrag->transid,
 				       BTRFS_DEFRAG_BATCH);
+
+	up_write(&fs_info->sb->s_umount);
 	sb_end_write(fs_info->sb);
 	/*
 	 * if we filled the whole defrag batch, there
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index b3b041a..2e7beee 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1189,6 +1189,32 @@  static void btrfs_resize_thread_pool(struct btrfs_fs_info *fs_info,
 	btrfs_set_max_workers(&fs_info->scrub_workers, new_pool_size);
 }
 
+static inline void btrfs_remount_prepare(struct btrfs_fs_info *fs_info,
+					 unsigned long old_opts, int flags)
+{
+	if (btrfs_raw_test_opt(old_opts, AUTO_DEFRAG) &&
+	    (!btrfs_raw_test_opt(fs_info->mount_opt, AUTO_DEFRAG) ||
+	     (flags & MS_RDONLY))) {
+		/* wait for any defraggers to finish */
+		wait_event(fs_info->transaction_wait,
+			   (atomic_read(&fs_info->defrag_running) == 0));
+	}
+}
+
+static inline void btrfs_remount_cleanup(struct btrfs_fs_info *fs_info,
+					 unsigned long old_opts, int flags)
+{
+	/*
+	 * We remount the fs successfully, then we need cleanup all defragable
+	 * inodes if the autodefragment is close or the fs is R/O.
+	 */
+	if (btrfs_raw_test_opt(old_opts, AUTO_DEFRAG) &&
+	    (!btrfs_raw_test_opt(fs_info->mount_opt, AUTO_DEFRAG) ||
+	     (flags & MS_RDONLY)))
+		btrfs_cleanup_defrag_inodes(fs_info);
+
+}
+
 static int btrfs_remount(struct super_block *sb, int *flags, char *data)
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(sb);
@@ -1214,6 +1240,8 @@  static int btrfs_remount(struct super_block *sb, int *flags, char *data)
 	if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
 		return 0;
 
+	btrfs_remount_prepare(fs_info, old_opts, *flags);
+
 	if (*flags & MS_RDONLY) {
 		sb->s_flags |= MS_RDONLY;
 
@@ -1247,6 +1275,7 @@  static int btrfs_remount(struct super_block *sb, int *flags, char *data)
 		sb->s_flags &= ~MS_RDONLY;
 	}
 
+	btrfs_remount_cleanup(fs_info, old_opts, *flags);
 	return 0;
 
 restore: