thr3ads.net - Btrfs devel - [PATCH 1/4] Btrfs: remove unnecessary ->s_umount in cleaner

If this information is useful, please help other people find it:
Share via:

Miao Xie

2013-May-14 10:20 UTC

[PATCH 1/4] Btrfs: remove unnecessary ->s_umount in cleaner_kthread()

In order to avoid the R/O remount, we acquired ->s_umount lock during
we deleted the dead snapshots and subvolumes. But it is unnecessary,
because we have cleaner_mutex.

We use cleaner_mutex to protect the process of the dead snapshots/subvolumes
deletion. And when we remount the fs to be R/O, we also acquire this mutex to
do cleanup after we change the status of the fs. That is this lock can serialize
the above operations, the cleaner can be aware of the status of the fs, and if
the cleaner is deleting the dead snapshots/subvolumes, the remount task will
wait for it. So it is safe to remove ->s_umount in cleaner_kthread().

Cc: David Sterba <dsterba@suse.cz>
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/disk-io.c | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index a9df562..cb2bfd1 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1676,24 +1676,40 @@ static void end_workqueue_fn(struct btrfs_work *work)
 	bio_endio(bio, error);
 }
 
+/*
+ * If we remount the fs to be R/O, the cleaner needn''t do anything
except
+ * sleeping. This function is used to check the status of the fs.
+ */
+static inline int need_cleaner_sleep(struct btrfs_root *root)
+{
+	return root->fs_info->sb->s_flags & MS_RDONLY;
+}
+
 static int cleaner_kthread(void *arg)
 {
 	struct btrfs_root *root = arg;
+	int again;
 
 	do {
-		int again = 0;
-
-		if (!(root->fs_info->sb->s_flags & MS_RDONLY) &&
-		    down_read_trylock(&root->fs_info->sb->s_umount)) {
-			if (mutex_trylock(&root->fs_info->cleaner_mutex)) {
-				btrfs_run_delayed_iputs(root);
-				again = btrfs_clean_one_deleted_snapshot(root);
-				mutex_unlock(&root->fs_info->cleaner_mutex);
-			}
-			btrfs_run_defrag_inodes(root->fs_info);
-			up_read(&root->fs_info->sb->s_umount);
-		}
+		again = 0;
 
+		/* Make the cleaner go to sleep early. */
+		if (need_cleaner_sleep(root))
+			goto sleep;
+
+		if (!mutex_trylock(&root->fs_info->cleaner_mutex))
+			goto sleep;
+
+		btrfs_run_delayed_iputs(root);
+		again = btrfs_clean_one_deleted_snapshot(root);
+		mutex_unlock(&root->fs_info->cleaner_mutex);
+
+		/*
+		 * The defragger has dealt with the R/O remount, needn''t
+		 * do anything special here.
+		 */
+		btrfs_run_defrag_inodes(root->fs_info);
+sleep:
 		if (!try_to_freeze() && !again) {
 			set_current_state(TASK_INTERRUPTIBLE);
 			if (!kthread_should_stop())
-- 
1.8.1.4

--
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

2013-May-14 10:20 UTC

head link

[PATCH 2/4] Btrfs: make the cleaner complete early when the fs is going to be umounted

Cc: David Sterba <dsterba@suse.cz>
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/disk-io.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index cb2bfd1..927da1a 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1677,12 +1677,14 @@ static void end_workqueue_fn(struct btrfs_work *work)
 }
 
 /*
- * If we remount the fs to be R/O, the cleaner needn''t do anything
except
- * sleeping. This function is used to check the status of the fs.
+ * If we remount the fs to be R/O or umount the fs, the cleaner
needn''t do
+ * anything except sleeping. This function is used to check the status of
+ * the fs.
  */
 static inline int need_cleaner_sleep(struct btrfs_root *root)
 {
-	return root->fs_info->sb->s_flags & MS_RDONLY;
+	return (root->fs_info->sb->s_flags & MS_RDONLY ||
+		btrfs_fs_closing(root->fs_info));
 }
 
 static int cleaner_kthread(void *arg)
@@ -1705,8 +1707,8 @@ static int cleaner_kthread(void *arg)
 		mutex_unlock(&root->fs_info->cleaner_mutex);
 
 		/*
-		 * The defragger has dealt with the R/O remount, needn''t
-		 * do anything special here.
+		 * The defragger has dealt with the R/O remount and umount,
+		 * needn''t do anything special here.
 		 */
 		btrfs_run_defrag_inodes(root->fs_info);
 sleep:
-- 
1.8.1.4

--
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

2013-May-14 10:20 UTC

head link

[PATCH 3/4] Btrfs: move the R/O check out of btrfs_clean_one_deleted_snapshot()

If the fs is remounted to be R/O, it is unnecessary to call
btrfs_clean_one_deleted_snapshot(), so move the R/O check out of
this function. And besides that, it can make the check logic in the
caller more clear.

Cc: David Sterba <dsterba@suse.cz>
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/disk-io.c     | 9 +++++++++
 fs/btrfs/transaction.c | 5 -----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 927da1a..c69ff46 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1702,6 +1702,15 @@ static int cleaner_kthread(void *arg)
 		if (!mutex_trylock(&root->fs_info->cleaner_mutex))
 			goto sleep;
 
+		/*
+		 * Avoid the problem that we change the status of the fs
+		 * during the above check and trylock.
+		 */
+		if (need_cleaner_sleep(root)) {
+			mutex_unlock(&root->fs_info->cleaner_mutex);
+			goto sleep;
+		}
+
 		btrfs_run_delayed_iputs(root);
 		again = btrfs_clean_one_deleted_snapshot(root);
 		mutex_unlock(&root->fs_info->cleaner_mutex);
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 89fad06..4b63111 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1885,11 +1885,6 @@ int btrfs_clean_one_deleted_snapshot(struct btrfs_root
*root)
 	int ret;
 	struct btrfs_fs_info *fs_info = root->fs_info;
 
-	if (fs_info->sb->s_flags & MS_RDONLY) {
-		pr_debug("btrfs: cleaner called for RO fs!\n");
-		return 0;
-	}
-
 	spin_lock(&fs_info->trans_lock);
 	if (list_empty(&fs_info->dead_roots)) {
 		spin_unlock(&fs_info->trans_lock);
-- 
1.8.1.4

--
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

2013-May-14 10:20 UTC

head link

[PATCH 4/4] Btrfs: make the snap/subv deletion end more early when the fs is R/O

The snapshot/subvolume deletion might spend lots of time, it would make
the remount task wait for a long time. This patch improve this problem,
we will break the deletion if the fs is remounted to be R/O. It will make
the users happy.

Cc: David Sterba <dsterba@suse.cz>
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ctree.h       | 12 ++++++++++++
 fs/btrfs/disk-io.c     | 15 ++-------------
 fs/btrfs/extent-tree.c |  2 +-
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index df9824b..067233f 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3318,6 +3318,18 @@ static inline int btrfs_fs_closing(struct btrfs_fs_info
*fs_info)
 	smp_mb();
 	return fs_info->closing;
 }
+
+/*
+ * If we remount the fs to be R/O or umount the fs, the cleaner
needn''t do
+ * anything except sleeping. This function is used to check the status of
+ * the fs.
+ */
+static inline int btrfs_need_cleaner_sleep(struct btrfs_root *root)
+{
+	return (root->fs_info->sb->s_flags & MS_RDONLY ||
+		btrfs_fs_closing(root->fs_info));
+}
+
 static inline void free_fs_info(struct btrfs_fs_info *fs_info)
 {
 	kfree(fs_info->balance_ctl);
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index c69ff46..78e2dfb 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1676,17 +1676,6 @@ static void end_workqueue_fn(struct btrfs_work *work)
 	bio_endio(bio, error);
 }
 
-/*
- * If we remount the fs to be R/O or umount the fs, the cleaner
needn''t do
- * anything except sleeping. This function is used to check the status of
- * the fs.
- */
-static inline int need_cleaner_sleep(struct btrfs_root *root)
-{
-	return (root->fs_info->sb->s_flags & MS_RDONLY ||
-		btrfs_fs_closing(root->fs_info));
-}
-
 static int cleaner_kthread(void *arg)
 {
 	struct btrfs_root *root = arg;
@@ -1696,7 +1685,7 @@ static int cleaner_kthread(void *arg)
 		again = 0;
 
 		/* Make the cleaner go to sleep early. */
-		if (need_cleaner_sleep(root))
+		if (btrfs_need_cleaner_sleep(root))
 			goto sleep;
 
 		if (!mutex_trylock(&root->fs_info->cleaner_mutex))
@@ -1706,7 +1695,7 @@ static int cleaner_kthread(void *arg)
 		 * Avoid the problem that we change the status of the fs
 		 * during the above check and trylock.
 		 */
-		if (need_cleaner_sleep(root)) {
+		if (btrfs_need_cleaner_sleep(root)) {
 			mutex_unlock(&root->fs_info->cleaner_mutex);
 			goto sleep;
 		}
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index fbeb0c0..455117a 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -7378,7 +7378,7 @@ int btrfs_drop_snapshot(struct btrfs_root *root,
 	wc->reada_count = BTRFS_NODEPTRS_PER_BLOCK(root);
 
 	while (1) {
-		if (!for_reloc && btrfs_fs_closing(root->fs_info)) {
+		if (!for_reloc && btrfs_need_cleaner_sleep(root)) {
 			pr_debug("btrfs: drop snapshot early exit\n");
 			err = -EAGAIN;
 			goto out_end_trans;
-- 
1.8.1.4

--
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

2013-May-14 11:31 UTC

head link

Re: [PATCH 1/4] Btrfs: remove unnecessary ->s_umount in cleaner_kthread()

On tue, 14 May 2013 18:20:40 +0800, Miao Xie wrote:> In order to avoid the R/O remount, we acquired ->s_umount lock during
> we deleted the dead snapshots and subvolumes. But it is unnecessary,
> because we have cleaner_mutex.
> 
> We use cleaner_mutex to protect the process of the dead
snapshots/subvolumes
> deletion. And when we remount the fs to be R/O, we also acquire this mutex
to
> do cleanup after we change the status of the fs. That is this lock can
serialize
> the above operations, the cleaner can be aware of the status of the fs, and
if
> the cleaner is deleting the dead snapshots/subvolumes, the remount task
will
> wait for it. So it is safe to remove ->s_umount in cleaner_kthread().
According to my test, this patch can also fix the deadlock problem which is
caused
by the race between autodefragger and freeze(xfstest 068).

Thanks
Miao
> 
> Cc: David Sterba <dsterba@suse.cz>
> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
> ---
>  fs/btrfs/disk-io.c | 40 ++++++++++++++++++++++++++++------------
>  1 file changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index a9df562..cb2bfd1 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1676,24 +1676,40 @@ static void end_workqueue_fn(struct btrfs_work
*work)
>  	bio_endio(bio, error);
>  }
>  
> +/*
> + * If we remount the fs to be R/O, the cleaner needn''t do
anything except
> + * sleeping. This function is used to check the status of the fs.
> + */
> +static inline int need_cleaner_sleep(struct btrfs_root *root)
> +{
> +	return root->fs_info->sb->s_flags & MS_RDONLY;
> +}
> +
>  static int cleaner_kthread(void *arg)
>  {
>  	struct btrfs_root *root = arg;
> +	int again;
>  
>  	do {
> -		int again = 0;
> -
> -		if (!(root->fs_info->sb->s_flags & MS_RDONLY) &&
> -		    down_read_trylock(&root->fs_info->sb->s_umount)) {
> -			if (mutex_trylock(&root->fs_info->cleaner_mutex)) {
> -				btrfs_run_delayed_iputs(root);
> -				again = btrfs_clean_one_deleted_snapshot(root);
> -				mutex_unlock(&root->fs_info->cleaner_mutex);
> -			}
> -			btrfs_run_defrag_inodes(root->fs_info);
> -			up_read(&root->fs_info->sb->s_umount);
> -		}
> +		again = 0;
>  
> +		/* Make the cleaner go to sleep early. */
> +		if (need_cleaner_sleep(root))
> +			goto sleep;
> +
> +		if (!mutex_trylock(&root->fs_info->cleaner_mutex))
> +			goto sleep;
> +
> +		btrfs_run_delayed_iputs(root);
> +		again = btrfs_clean_one_deleted_snapshot(root);
> +		mutex_unlock(&root->fs_info->cleaner_mutex);
> +
> +		/*
> +		 * The defragger has dealt with the R/O remount, needn''t
> +		 * do anything special here.
> +		 */
> +		btrfs_run_defrag_inodes(root->fs_info);
> +sleep:
>  		if (!try_to_freeze() && !again) {
>  			set_current_state(TASK_INTERRUPTIBLE);
>  			if (!kthread_should_stop())
> 
--
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

Btrfs devel - May 2013 - [PATCH 1/4] Btrfs: remove unnecessary ->s_umount in cleaner_kthread()

[PATCH 1/4] Btrfs: remove unnecessary ->s_umount in cleaner_kthread()

[PATCH 2/4] Btrfs: make the cleaner complete early when the fs is going to be umounted

[PATCH 3/4] Btrfs: move the R/O check out of btrfs_clean_one_deleted_snapshot()

[PATCH 4/4] Btrfs: make the snap/subv deletion end more early when the fs is R/O

Re: [PATCH 1/4] Btrfs: remove unnecessary ->s_umount in cleaner_kthread()