On wed, 14 Aug 2013 11:41:00 -0400, Josef Bacik wrote:
> I added a patch where we started taking the ordered operations mutex when we
> waited on ordered extents.  We need this because we splice the list and 
> process
> it, so if a flusher came in during this scenario it would think the list was
> empty and we'd usually get an early ENOSPC.  The problem with this is that 
> this
> lock is used in transaction committing.  So we end up with something like this
> 
> Transaction commit
>       -> wait on writers
> 
> Delalloc flusher
>       -> run_ordered_operations (holds mutex)
>               ->wait for filemap-flush to do its thing
> 
> flush task
>       -> cow_file_range
>               ->wait on btrfs_join_transaction because we're commiting
> 
> some other task
>       -> commit_transaction because we notice trans->transaction->flush is set
>               -> run_ordered_operations (hang on mutex)

Sorry, I can not understand this explanation. As far as I know, if the flush 
task
waits on btrfs_join_transaction(), it means the transaction is under commit
(state = TRANS_STATE_COMMIT_DOING), and all the external 
writers(TRANS_START/TRANS_ATTACH/
TRANS_USERSPACE) have quitted the current transaction, so no one would try to 
call
run_ordered_operations().

Could you show us the reproduce steps?

Thanks
Miao

> 
> We need to disentangle the ordered operations flushing from the delalloc
> flushing, since they are separate things.  This solves the deadlock issue I 
> was
> seeing.  Thanks,
> 
> Signed-off-by: Josef Bacik <jba...@fusionio.com>
> ---
>  fs/btrfs/ctree.h        |    7 +++++++
>  fs/btrfs/disk-io.c      |    1 +
>  fs/btrfs/ordered-data.c |    4 ++--
>  3 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index ea4cc16..d79e32c 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1418,6 +1418,13 @@ struct btrfs_fs_info {
>        * before jumping into the main commit.
>        */
>       struct mutex ordered_operations_mutex;
> +
> +     /*
> +      * Same as ordered_operations_mutex except this is for ordered extents
> +      * and not the operations.
> +      */
> +     struct mutex ordered_extent_flush_mutex;
> +
>       struct rw_semaphore extent_commit_sem;
>  
>       struct rw_semaphore cleanup_work_sem;
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index c82025d..880dcde 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2288,6 +2288,7 @@ int open_ctree(struct super_block *sb,
>  
>  
>       mutex_init(&fs_info->ordered_operations_mutex);
> +     mutex_init(&fs_info->ordered_extent_flush_mutex);
>       mutex_init(&fs_info->tree_log_mutex);
>       mutex_init(&fs_info->chunk_mutex);
>       mutex_init(&fs_info->transaction_kthread_mutex);
> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
> index 8136982..b52b2c4 100644
> --- a/fs/btrfs/ordered-data.c
> +++ b/fs/btrfs/ordered-data.c
> @@ -671,7 +671,7 @@ int btrfs_run_ordered_operations(struct 
> btrfs_trans_handle *trans,
>       INIT_LIST_HEAD(&splice);
>       INIT_LIST_HEAD(&works);
>  
> -     mutex_lock(&root->fs_info->ordered_operations_mutex);
> +     mutex_lock(&root->fs_info->ordered_extent_flush_mutex);
>       spin_lock(&root->fs_info->ordered_root_lock);
>       list_splice_init(&cur_trans->ordered_operations, &splice);
>       while (!list_empty(&splice)) {
> @@ -718,7 +718,7 @@ out:
>               list_del_init(&work->list);
>               btrfs_wait_and_free_delalloc_work(work);
>       }
> -     mutex_unlock(&root->fs_info->ordered_operations_mutex);
> +     mutex_unlock(&root->fs_info->ordered_extent_flush_mutex);
>       return ret;
>  }
>  
> 

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