From: Liu Bo <liub.liubo@gmail.com>
To: Josef Bacik <jbacik@fusionio.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] Btrfs: barrier before waitqueue_active
Date: Thu, 02 Aug 2012 18:46:44 +0800 [thread overview]
Message-ID: <501A5A94.7070307@gmail.com> (raw)
In-Reply-To: <1343852708-24009-1-git-send-email-jbacik@fusionio.com>
On 08/02/2012 04:25 AM, Josef Bacik wrote:
> We need an smb_mb() before waitqueue_active to avoid missing wakeups.
> Before Mitch was hitting a deadlock between the ordered flushers and the
> transaction commit because the ordered flushers were waiting for more refs
> and were never woken up, so those smp_mb()'s are the most important.
> Everything else I added for correctness sake and to avoid getting bitten by
> this again somewhere else. Thanks,
>
Hi Josef,
I'll appreciate a lot if you can add some comments for each memory
barrier, because not everyone knows why it is used here and there. :)
thanks,
liubo
> Signed-off-by: Josef Bacik <jbacik@fusionio.com>
> ---
> fs/btrfs/compression.c | 1 +
> fs/btrfs/delayed-inode.c | 16 ++++++++++------
> fs/btrfs/delayed-ref.c | 18 ++++++++++++------
> fs/btrfs/disk-io.c | 11 ++++++++---
> fs/btrfs/inode.c | 8 +++++---
> fs/btrfs/volumes.c | 8 +++++---
> 6 files changed, 41 insertions(+), 21 deletions(-)
>
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 86eff48..43d1c5a 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -818,6 +818,7 @@ static void free_workspace(int type, struct list_head *workspace)
> btrfs_compress_op[idx]->free_workspace(workspace);
> atomic_dec(alloc_workspace);
> wake:
> + smp_mb();
> if (waitqueue_active(workspace_wait))
> wake_up(workspace_wait);
> }
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index 335605c..8cc9b19 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -513,9 +513,11 @@ static void __btrfs_remove_delayed_item(struct btrfs_delayed_item *delayed_item)
> rb_erase(&delayed_item->rb_node, root);
> delayed_item->delayed_node->count--;
> atomic_dec(&delayed_root->items);
> - if (atomic_read(&delayed_root->items) < BTRFS_DELAYED_BACKGROUND &&
> - waitqueue_active(&delayed_root->wait))
> - wake_up(&delayed_root->wait);
> + if (atomic_read(&delayed_root->items) < BTRFS_DELAYED_BACKGROUND) {
> + smp_mb();
> + if (waitqueue_active(&delayed_root->wait))
> + wake_up(&delayed_root->wait);
> + }
> }
>
> static void btrfs_release_delayed_item(struct btrfs_delayed_item *item)
> @@ -1057,9 +1059,11 @@ static void btrfs_release_delayed_inode(struct btrfs_delayed_node *delayed_node)
> delayed_root = delayed_node->root->fs_info->delayed_root;
> atomic_dec(&delayed_root->items);
> if (atomic_read(&delayed_root->items) <
> - BTRFS_DELAYED_BACKGROUND &&
> - waitqueue_active(&delayed_root->wait))
> - wake_up(&delayed_root->wait);
> + BTRFS_DELAYED_BACKGROUND) {
> + smp_mb();
> + if (waitqueue_active(&delayed_root->wait))
> + wake_up(&delayed_root->wait);
> + }
> }
> }
>
> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
> index da7419e..858ef02 100644
> --- a/fs/btrfs/delayed-ref.c
> +++ b/fs/btrfs/delayed-ref.c
> @@ -662,9 +662,12 @@ int btrfs_add_delayed_tree_ref(struct btrfs_fs_info *fs_info,
> add_delayed_tree_ref(fs_info, trans, &ref->node, bytenr,
> num_bytes, parent, ref_root, level, action,
> for_cow);
> - if (!need_ref_seq(for_cow, ref_root) &&
> - waitqueue_active(&fs_info->tree_mod_seq_wait))
> - wake_up(&fs_info->tree_mod_seq_wait);
> + if (!need_ref_seq(for_cow, ref_root)) {
> + smp_mb();
> + if (waitqueue_active(&fs_info->tree_mod_seq_wait))
> + wake_up(&fs_info->tree_mod_seq_wait);
> + }
> +
> spin_unlock(&delayed_refs->lock);
> if (need_ref_seq(for_cow, ref_root))
> btrfs_qgroup_record_ref(trans, &ref->node, extent_op);
> @@ -713,9 +716,11 @@ int btrfs_add_delayed_data_ref(struct btrfs_fs_info *fs_info,
> add_delayed_data_ref(fs_info, trans, &ref->node, bytenr,
> num_bytes, parent, ref_root, owner, offset,
> action, for_cow);
> - if (!need_ref_seq(for_cow, ref_root) &&
> - waitqueue_active(&fs_info->tree_mod_seq_wait))
> - wake_up(&fs_info->tree_mod_seq_wait);
> + if (!need_ref_seq(for_cow, ref_root)) {
> + smp_mb();
> + if (waitqueue_active(&fs_info->tree_mod_seq_wait))
> + wake_up(&fs_info->tree_mod_seq_wait);
> + }
> spin_unlock(&delayed_refs->lock);
> if (need_ref_seq(for_cow, ref_root))
> btrfs_qgroup_record_ref(trans, &ref->node, extent_op);
> @@ -744,6 +749,7 @@ int btrfs_add_delayed_extent_op(struct btrfs_fs_info *fs_info,
> num_bytes, BTRFS_UPDATE_DELAYED_HEAD,
> extent_op->is_data);
>
> + smp_mb();
> if (waitqueue_active(&fs_info->tree_mod_seq_wait))
> wake_up(&fs_info->tree_mod_seq_wait);
> spin_unlock(&delayed_refs->lock);
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 502b20c..a355c89 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -756,9 +756,11 @@ static void run_one_async_done(struct btrfs_work *work)
>
> atomic_dec(&fs_info->nr_async_submits);
>
> - if (atomic_read(&fs_info->nr_async_submits) < limit &&
> - waitqueue_active(&fs_info->async_submit_wait))
> - wake_up(&fs_info->async_submit_wait);
> + if (atomic_read(&fs_info->nr_async_submits) < limit) {
> + smp_mb();
> + if (waitqueue_active(&fs_info->async_submit_wait))
> + wake_up(&fs_info->async_submit_wait);
> + }
>
> /* If an error occured we just want to clean up the bio and move on */
> if (async->error) {
> @@ -3785,14 +3787,17 @@ int btrfs_cleanup_transaction(struct btrfs_root *root)
> /* FIXME: cleanup wait for commit */
> t->in_commit = 1;
> t->blocked = 1;
> + smp_mb();
> if (waitqueue_active(&root->fs_info->transaction_blocked_wait))
> wake_up(&root->fs_info->transaction_blocked_wait);
>
> t->blocked = 0;
> + smp_mb();
> if (waitqueue_active(&root->fs_info->transaction_wait))
> wake_up(&root->fs_info->transaction_wait);
>
> t->commit_done = 1;
> + smp_mb();
> if (waitqueue_active(&t->commit_wait))
> wake_up(&t->commit_wait);
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 4b82ae2..acea7d9 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1010,9 +1010,11 @@ static noinline void async_cow_submit(struct btrfs_work *work)
> atomic_sub(nr_pages, &root->fs_info->async_delalloc_pages);
>
> if (atomic_read(&root->fs_info->async_delalloc_pages) <
> - 5 * 1024 * 1024 &&
> - waitqueue_active(&root->fs_info->async_submit_wait))
> - wake_up(&root->fs_info->async_submit_wait);
> + 5 * 1024 * 1024) {
> + smp_mb();
> + if (waitqueue_active(&root->fs_info->async_submit_wait))
> + wake_up(&root->fs_info->async_submit_wait);
> + }
>
> if (async_cow->inode)
> submit_compressed_extents(async_cow->inode, async_cow);
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index b8708f9..871f43f 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -229,9 +229,11 @@ loop_lock:
> cur->bi_next = NULL;
> atomic_dec(&fs_info->nr_async_bios);
>
> - if (atomic_read(&fs_info->nr_async_bios) < limit &&
> - waitqueue_active(&fs_info->async_submit_wait))
> - wake_up(&fs_info->async_submit_wait);
> + if (atomic_read(&fs_info->nr_async_bios) < limit) {
> + smp_mb();
> + if (waitqueue_active(&fs_info->async_submit_wait))
> + wake_up(&fs_info->async_submit_wait);
> + }
>
> BUG_ON(atomic_read(&cur->bi_cnt) == 0);
>
>
next prev parent reply other threads:[~2012-08-02 10:46 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-01 20:25 [PATCH] Btrfs: barrier before waitqueue_active Josef Bacik
2012-08-02 0:21 ` Mitch Harder
2012-08-03 14:43 ` Mitch Harder
2012-08-05 9:18 ` Arne Jansen
2012-08-02 10:46 ` Liu Bo [this message]
2012-08-02 12:11 ` Josef Bacik
2012-08-02 13:02 ` David Sterba
2012-08-02 13:01 ` cwillu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=501A5A94.7070307@gmail.com \
--to=liub.liubo@gmail.com \
--cc=jbacik@fusionio.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).