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@xxxxxxx>
> Signed-off-by: Miao Xie <miaox@xxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
