* [PATCH 2/4] Btrfs: make the cleaner complete early when the fs is going to be umounted
2013-05-14 10:20 [PATCH 1/4] Btrfs: remove unnecessary ->s_umount in cleaner_kthread() Miao Xie
@ 2013-05-14 10:20 ` Miao Xie
2013-05-14 10:20 ` [PATCH 3/4] Btrfs: move the R/O check out of btrfs_clean_one_deleted_snapshot() Miao Xie
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Miao Xie @ 2013-05-14 10:20 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/4] Btrfs: move the R/O check out of btrfs_clean_one_deleted_snapshot()
2013-05-14 10:20 [PATCH 1/4] Btrfs: remove unnecessary ->s_umount in cleaner_kthread() Miao Xie
2013-05-14 10:20 ` [PATCH 2/4] Btrfs: make the cleaner complete early when the fs is going to be umounted Miao Xie
@ 2013-05-14 10:20 ` Miao Xie
2013-05-14 10:20 ` [PATCH 4/4] Btrfs: make the snap/subv deletion end more early when the fs is R/O Miao Xie
2013-05-14 11:31 ` [PATCH 1/4] Btrfs: remove unnecessary ->s_umount in cleaner_kthread() Miao Xie
3 siblings, 0 replies; 5+ messages in thread
From: Miao Xie @ 2013-05-14 10:20 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 4/4] Btrfs: make the snap/subv deletion end more early when the fs is R/O
2013-05-14 10:20 [PATCH 1/4] Btrfs: remove unnecessary ->s_umount in cleaner_kthread() Miao Xie
2013-05-14 10:20 ` [PATCH 2/4] Btrfs: make the cleaner complete early when the fs is going to be umounted Miao Xie
2013-05-14 10:20 ` [PATCH 3/4] Btrfs: move the R/O check out of btrfs_clean_one_deleted_snapshot() Miao Xie
@ 2013-05-14 10:20 ` Miao Xie
2013-05-14 11:31 ` [PATCH 1/4] Btrfs: remove unnecessary ->s_umount in cleaner_kthread() Miao Xie
3 siblings, 0 replies; 5+ messages in thread
From: Miao Xie @ 2013-05-14 10:20 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/4] Btrfs: remove unnecessary ->s_umount in cleaner_kthread()
2013-05-14 10:20 [PATCH 1/4] Btrfs: remove unnecessary ->s_umount in cleaner_kthread() Miao Xie
` (2 preceding siblings ...)
2013-05-14 10:20 ` [PATCH 4/4] Btrfs: make the snap/subv deletion end more early when the fs is R/O Miao Xie
@ 2013-05-14 11:31 ` Miao Xie
3 siblings, 0 replies; 5+ messages in thread
From: Miao Xie @ 2013-05-14 11:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
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())
>
^ permalink raw reply [flat|nested] 5+ messages in thread