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 <dste...@suse.cz>
> Signed-off-by: Miao Xie <mi...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to