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 <jbacik@xxxxxxxxxxxx>
> ---
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
