On Mon, 30 Apr 2012 18:41:39 +0200, David Sterba wrote:
> On Fri, Apr 27, 2012 at 06:55:13PM +0800, Miao Xie wrote:
>> But I found you add a trylock for ->s_umount in cleaner_kthread(), this method
>> can fix the deadlock problem, I think. It may be introduced by the other patch,
>> could you send that patch to me? I found if we fail to trylock ->cleaner_mutex,
>> we will forget to unlock ->s_umount.
>
> the unlock was not visible within the diff context, the full patch is:
>
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1578,11 +1578,13 @@ static int cleaner_kthread(void *arg)
> vfs_check_frozen(root->fs_info->sb, SB_FREEZE_WRITE);
>
> if (!(root->fs_info->sb->s_flags & MS_RDONLY) &&
> + down_read_trylock(&root->fs_info->sb->s_umount) &&
> mutex_trylock(&root->fs_info->cleaner_mutex)) {
> btrfs_run_delayed_iputs(root);
> btrfs_clean_old_snapshots(root);
> mutex_unlock(&root->fs_info->cleaner_mutex);
> btrfs_run_defrag_inodes(root->fs_info);
> + up_read(&root->fs_info->sb->s_umount);
> }
>
> if (freezing(current)) {
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3413,7 +3413,9 @@ static int shrink_delalloc(struct btrfs_
> max_reclaim >> PAGE_CACHE_SHIFT);
> while (loops < 1024) {
> /* have the flusher threads jump in and do some IO */
> - smp_mb();
> + if (btrfs_fs_closing(root->fs_info))
> + return -EAGAIN;
> +
> nr_pages = min_t(unsigned long, nr_pages,
> root->fs_info->delalloc_bytes >> PAGE_CACHE_SHIFT);
> writeback_inodes_sb_nr_if_idle(root->fs_info->sb, nr_pages);
> ---
>
> after close_tree starts the "fs_closing" check will prevent further
> calls to writeback. I don't remember from who the patch acutally comes
> from (via irc), it was noted as a workaround for the cleaner deadlock
> until it gets fully solved (re your patches
> http://www.mail-archive.com/linux-btrfs@xxxxxxxxxxxxxxx/msg13897.html
> and reference to balance and scrub:
> http://www.spinics.net/lists/linux-btrfs/msg14056.html
> )
Sorry, I forget to reply this mail.
I think this method can not fix the problem safely because if the other
background threads(not the cleaner) call shrink_delalloc(), the problem
can still occur.
Though trylock for the cleaner can not fix this problem, I think the cleaner
still need trylock ->s_umount, in this way, we can stop the cleaner making
lots of dirty pages when we do readonly remount or umount.
Thanks
Miao
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
