On 14.05.2018 15:23, David Sterba wrote:
> Use the wrappers and reduce the amount of low-level details about the
> waitqueue management.
>
> Signed-off-by: David Sterba <dsterba@xxxxxxxx>
Reviewed-by: Nikolay Borisov <nborisov@xxxxxxxx>
> ---
> fs/btrfs/compression.c | 7 +------
> fs/btrfs/delayed-inode.c | 9 +++------
> fs/btrfs/dev-replace.c | 10 ++++------
> fs/btrfs/extent-tree.c | 7 +------
> fs/btrfs/inode.c | 9 +++------
> fs/btrfs/locking.c | 34 +++++++++++-----------------------
> fs/btrfs/ordered-data.c | 14 ++++----------
> fs/btrfs/transaction.c | 7 +------
> fs/btrfs/tree-log.c | 34 ++++++++++++----------------------
> 9 files changed, 40 insertions(+), 91 deletions(-)
>
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 5268c9f85ca7..dcebb91e4e0c 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -990,12 +990,7 @@ static void __free_workspace(int type, struct list_head *workspace,
> btrfs_compress_op[idx]->free_workspace(workspace);
> atomic_dec(total_ws);
> wake:
> - /*
> - * Make sure counter is updated before we wake up waiters.
> - */
> - smp_mb();
> - if (waitqueue_active(ws_wait))
> - wake_up(ws_wait);
> + cond_wake_up(ws_wait);
> }
>
> static void free_workspace(int type, struct list_head *ws)
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index a8d492dbd3e7..fe6caa7e698b 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -460,13 +460,10 @@ static void finish_one_item(struct btrfs_delayed_root *delayed_root)
> {
> int seq = atomic_inc_return(&delayed_root->items_seq);
>
> - /*
> - * atomic_dec_return implies a barrier for waitqueue_active
> - */
> + /* atomic_dec_return implies a barrier */
> if ((atomic_dec_return(&delayed_root->items) <
> - BTRFS_DELAYED_BACKGROUND || seq % BTRFS_DELAYED_BATCH == 0) &&
> - waitqueue_active(&delayed_root->wait))
> - wake_up(&delayed_root->wait);
> + BTRFS_DELAYED_BACKGROUND || seq % BTRFS_DELAYED_BATCH == 0))
> + cond_wake_up_nomb(&delayed_root->wait);
> }
>
> static void __btrfs_remove_delayed_item(struct btrfs_delayed_item *delayed_item)
> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> index f82be266ba4b..d33ff5b23302 100644
> --- a/fs/btrfs/dev-replace.c
> +++ b/fs/btrfs/dev-replace.c
> @@ -916,9 +916,9 @@ void btrfs_dev_replace_clear_lock_blocking(
> ASSERT(atomic_read(&dev_replace->read_locks) > 0);
> ASSERT(atomic_read(&dev_replace->blocking_readers) > 0);
> read_lock(&dev_replace->lock);
> - if (atomic_dec_and_test(&dev_replace->blocking_readers) &&
> - waitqueue_active(&dev_replace->read_lock_wq))
> - wake_up(&dev_replace->read_lock_wq);
> + /* Barrier implied by atomic_dec_and_test */
> + if (atomic_dec_and_test(&dev_replace->blocking_readers))
> + cond_wake_up_nomb(&dev_replace->read_lock_wq);
> }
>
> void btrfs_bio_counter_inc_noblocked(struct btrfs_fs_info *fs_info)
> @@ -929,9 +929,7 @@ void btrfs_bio_counter_inc_noblocked(struct btrfs_fs_info *fs_info)
> void btrfs_bio_counter_sub(struct btrfs_fs_info *fs_info, s64 amount)
> {
> percpu_counter_sub(&fs_info->bio_counter, amount);
> -
> - if (waitqueue_active(&fs_info->replace_wait))
> - wake_up(&fs_info->replace_wait);
> + cond_wake_up_nomb(&fs_info->replace_wait);
> }
>
> void btrfs_bio_counter_inc_blocked(struct btrfs_fs_info *fs_info)
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 3871658b6ab1..94fc825a126d 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -11094,12 +11094,7 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
> void btrfs_end_write_no_snapshotting(struct btrfs_root *root)
> {
> percpu_counter_dec(&root->subv_writers->counter);
> - /*
> - * Make sure counter is updated before we wake up waiters.
> - */
> - smp_mb();
> - if (waitqueue_active(&root->subv_writers->wait))
> - wake_up(&root->subv_writers->wait);
> + cond_wake_up(&root->subv_writers->wait);
> }
>
> int btrfs_start_write_no_snapshotting(struct btrfs_root *root)
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index d241285a0d2a..cf6f2815ffee 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1156,13 +1156,10 @@ static noinline void async_cow_submit(struct btrfs_work *work)
> nr_pages = (async_cow->end - async_cow->start + PAGE_SIZE) >>
> PAGE_SHIFT;
>
> - /*
> - * atomic_sub_return implies a barrier for waitqueue_active
> - */
> + /* atomic_sub_return implies a barrier */
> if (atomic_sub_return(nr_pages, &fs_info->async_delalloc_pages) <
> - 5 * SZ_1M &&
> - waitqueue_active(&fs_info->async_submit_wait))
> - wake_up(&fs_info->async_submit_wait);
> + 5 * SZ_1M)
> + cond_wake_up_nomb(&fs_info->async_submit_wait);
>
> if (async_cow->inode)
> submit_compressed_extents(async_cow->inode, async_cow);
> diff --git a/fs/btrfs/locking.c b/fs/btrfs/locking.c
> index e4faefac9d16..1da768e5ef75 100644
> --- a/fs/btrfs/locking.c
> +++ b/fs/btrfs/locking.c
> @@ -66,22 +66,16 @@ void btrfs_clear_lock_blocking_rw(struct extent_buffer *eb, int rw)
> write_lock(&eb->lock);
> WARN_ON(atomic_read(&eb->spinning_writers));
> atomic_inc(&eb->spinning_writers);
> - /*
> - * atomic_dec_and_test implies a barrier for waitqueue_active
> - */
> - if (atomic_dec_and_test(&eb->blocking_writers) &&
> - waitqueue_active(&eb->write_lock_wq))
> - wake_up(&eb->write_lock_wq);
> + /* atomic_dec_and_test implies a barrier */
> + if (atomic_dec_and_test(&eb->blocking_writers))
> + cond_wake_up_nomb(&eb->write_lock_wq);
> } else if (rw == BTRFS_READ_LOCK_BLOCKING) {
> BUG_ON(atomic_read(&eb->blocking_readers) == 0);
> read_lock(&eb->lock);
> atomic_inc(&eb->spinning_readers);
> - /*
> - * atomic_dec_and_test implies a barrier for waitqueue_active
> - */
> - if (atomic_dec_and_test(&eb->blocking_readers) &&
> - waitqueue_active(&eb->read_lock_wq))
> - wake_up(&eb->read_lock_wq);
> + /* atomic_dec_and_test implies a barrier */
> + if (atomic_dec_and_test(&eb->blocking_readers))
> + cond_wake_up_nomb(&eb->read_lock_wq);
> }
> }
>
> @@ -221,12 +215,9 @@ void btrfs_tree_read_unlock_blocking(struct extent_buffer *eb)
> }
> btrfs_assert_tree_read_locked(eb);
> WARN_ON(atomic_read(&eb->blocking_readers) == 0);
> - /*
> - * atomic_dec_and_test implies a barrier for waitqueue_active
> - */
> - if (atomic_dec_and_test(&eb->blocking_readers) &&
> - waitqueue_active(&eb->read_lock_wq))
> - wake_up(&eb->read_lock_wq);
> + /* atomic_dec_and_test implies a barrier */
> + if (atomic_dec_and_test(&eb->blocking_readers))
> + cond_wake_up_nomb(&eb->read_lock_wq);
> atomic_dec(&eb->read_locks);
> }
>
> @@ -275,12 +266,9 @@ void btrfs_tree_unlock(struct extent_buffer *eb)
> if (blockers) {
> WARN_ON(atomic_read(&eb->spinning_writers));
> atomic_dec(&eb->blocking_writers);
> - /*
> - * Make sure counter is updated before we wake up waiters.
> - */
> + /* Use the lighter barrier after atomic */
> smp_mb__after_atomic();
> - if (waitqueue_active(&eb->write_lock_wq))
> - wake_up(&eb->write_lock_wq);
> + cond_wake_up_nomb(&eb->write_lock_wq);
> } else {
> WARN_ON(atomic_read(&eb->spinning_writers) != 1);
> atomic_dec(&eb->spinning_writers);
> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
> index 6db8bb2f2c28..2e1a1694a33d 100644
> --- a/fs/btrfs/ordered-data.c
> +++ b/fs/btrfs/ordered-data.c
> @@ -343,11 +343,8 @@ int btrfs_dec_test_first_ordered_pending(struct inode *inode,
>
> if (entry->bytes_left == 0) {
> ret = test_and_set_bit(BTRFS_ORDERED_IO_DONE, &entry->flags);
> - /*
> - * Implicit memory barrier after test_and_set_bit
> - */
> - if (waitqueue_active(&entry->wait))
> - wake_up(&entry->wait);
> + /* test_and_set_bit implies a barrier */
> + cond_wake_up_nomb(&entry->wait);
> } else {
> ret = 1;
> }
> @@ -410,11 +407,8 @@ int btrfs_dec_test_ordered_pending(struct inode *inode,
>
> if (entry->bytes_left == 0) {
> ret = test_and_set_bit(BTRFS_ORDERED_IO_DONE, &entry->flags);
> - /*
> - * Implicit memory barrier after test_and_set_bit
> - */
> - if (waitqueue_active(&entry->wait))
> - wake_up(&entry->wait);
> + /* test_and_set_bit implies a barrier */
> + cond_wake_up_nomb(&entry->wait);
> } else {
> ret = 1;
> }
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index c944b4769e3c..ff841abb756e 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -877,12 +877,7 @@ static int __btrfs_end_transaction(struct btrfs_trans_handle *trans,
> atomic_dec(&cur_trans->num_writers);
> extwriter_counter_dec(cur_trans, trans->type);
>
> - /*
> - * Make sure counter is updated before we wake up waiters.
> - */
> - smp_mb();
> - if (waitqueue_active(&cur_trans->writer_wait))
> - wake_up(&cur_trans->writer_wait);
> + cond_wake_up(&cur_trans->writer_wait);
> btrfs_put_transaction(cur_trans);
>
> if (current->journal_info == trans)
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index fa5b3dc5f4d5..a3ebda24ae12 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -222,11 +222,8 @@ int btrfs_pin_log_trans(struct btrfs_root *root)
> void btrfs_end_log_trans(struct btrfs_root *root)
> {
> if (atomic_dec_and_test(&root->log_writers)) {
> - /*
> - * Implicit memory barrier after atomic_dec_and_test
> - */
> - if (waitqueue_active(&root->log_writer_wait))
> - wake_up(&root->log_writer_wait);
> + /* atomic_dec_and_test implies a barrier */
> + cond_wake_up_nomb(&root->log_writer_wait);
> }
> }
>
> @@ -2988,11 +2985,8 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
>
> mutex_lock(&log_root_tree->log_mutex);
> if (atomic_dec_and_test(&log_root_tree->log_writers)) {
> - /*
> - * Implicit memory barrier after atomic_dec_and_test
> - */
> - if (waitqueue_active(&log_root_tree->log_writer_wait))
> - wake_up(&log_root_tree->log_writer_wait);
> + /* atomic_dec_and_test implies a barrier */
> + cond_wake_up_nomb(&log_root_tree->log_writer_wait);
> }
>
> if (ret) {
> @@ -3116,13 +3110,11 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
> mutex_unlock(&log_root_tree->log_mutex);
>
> /*
> - * The barrier before waitqueue_active is needed so all the updates
> - * above are seen by the woken threads. It might not be necessary, but
> - * proving that seems to be hard.
> + * The barrier before waitqueue_active (in cond_wake_up) is needed so
> + * all the updates above are seen by the woken threads. It might not be
> + * necessary, but proving that seems to be hard.
> */
> - smp_mb();
> - if (waitqueue_active(&log_root_tree->log_commit_wait[index2]))
> - wake_up(&log_root_tree->log_commit_wait[index2]);
> + cond_wake_up(&log_root_tree->log_commit_wait[index2]);
> out:
> mutex_lock(&root->log_mutex);
> btrfs_remove_all_log_ctxs(root, index1, ret);
> @@ -3131,13 +3123,11 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
> mutex_unlock(&root->log_mutex);
>
> /*
> - * The barrier before waitqueue_active is needed so all the updates
> - * above are seen by the woken threads. It might not be necessary, but
> - * proving that seems to be hard.
> + * The barrier before waitqueue_active (in cond_wake_up) is needed so
> + * all the updates above are seen by the woken threads. It might not be
> + * necessary, but proving that seems to be hard.
> */
> - smp_mb();
> - if (waitqueue_active(&root->log_commit_wait[index1]))
> - wake_up(&root->log_commit_wait[index1]);
> + cond_wake_up(&root->log_commit_wait[index1]);
> 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