From: Omar Sandoval <osandov@osandov.com>
To: David Sterba <dsterba@suse.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 06/11] btrfs: dev-replace: move replace members out of fs_info
Date: Fri, 7 Sep 2018 14:09:48 -0700 [thread overview]
Message-ID: <20180907210948.GI29245@vader> (raw)
In-Reply-To: <b990783799ffa2cdcccd262cacdf4ce23ba36502.1536331604.git.dsterba@suse.com>
On Fri, Sep 07, 2018 at 04:55:13PM +0200, David Sterba wrote:
> The replace_wait and bio_counter were mistakenly added to fs_info in
> commit c404e0dc2c843b154f ("Btrfs: fix use-after-free in the finishing
> procedure of the device replace"), but they logically belong to
> fs_info::dev_replace. Besides, bio_counter is a very generic name and is
> confusing in bare fs_info context.
Makes much more sense there.
Reviewed-by: Omar Sandoval <osandov@fb.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
> fs/btrfs/ctree.h | 6 +++---
> fs/btrfs/dev-replace.c | 16 ++++++++--------
> fs/btrfs/disk-io.c | 9 +++++----
> 3 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 31f7a58add7e..89fc90eafb0a 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -371,6 +371,9 @@ struct btrfs_dev_replace {
> wait_queue_head_t read_lock_wq;
>
> struct btrfs_scrub_progress scrub_progress;
> +
> + struct percpu_counter bio_counter;
> + wait_queue_head_t replace_wait;
> };
>
> /* For raid type sysfs entries */
> @@ -1093,9 +1096,6 @@ struct btrfs_fs_info {
> /* device replace state */
> struct btrfs_dev_replace dev_replace;
>
> - struct percpu_counter bio_counter;
> - wait_queue_head_t replace_wait;
> -
> struct semaphore uuid_tree_rescan_sem;
>
> /* Used to reclaim the metadata space in the background. */
> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> index 0a49843b2ee6..c0e9b2c229d5 100644
> --- a/fs/btrfs/dev-replace.c
> +++ b/fs/btrfs/dev-replace.c
> @@ -540,8 +540,8 @@ int btrfs_dev_replace_by_ioctl(struct btrfs_fs_info *fs_info,
> static void btrfs_rm_dev_replace_blocked(struct btrfs_fs_info *fs_info)
> {
> set_bit(BTRFS_FS_STATE_DEV_REPLACING, &fs_info->fs_state);
> - wait_event(fs_info->replace_wait, !percpu_counter_sum(
> - &fs_info->bio_counter));
> + wait_event(fs_info->dev_replace.replace_wait, !percpu_counter_sum(
> + &fs_info->dev_replace.bio_counter));
> }
>
> /*
> @@ -550,7 +550,7 @@ static void btrfs_rm_dev_replace_blocked(struct btrfs_fs_info *fs_info)
> static void btrfs_rm_dev_replace_unblocked(struct btrfs_fs_info *fs_info)
> {
> clear_bit(BTRFS_FS_STATE_DEV_REPLACING, &fs_info->fs_state);
> - wake_up(&fs_info->replace_wait);
> + wake_up(&fs_info->dev_replace.replace_wait);
> }
>
> static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
> @@ -992,25 +992,25 @@ void btrfs_dev_replace_set_lock_blocking(
>
> void btrfs_bio_counter_inc_noblocked(struct btrfs_fs_info *fs_info)
> {
> - percpu_counter_inc(&fs_info->bio_counter);
> + percpu_counter_inc(&fs_info->dev_replace.bio_counter);
> }
>
> void btrfs_bio_counter_sub(struct btrfs_fs_info *fs_info, s64 amount)
> {
> - percpu_counter_sub(&fs_info->bio_counter, amount);
> - cond_wake_up_nomb(&fs_info->replace_wait);
> + percpu_counter_sub(&fs_info->dev_replace.bio_counter, amount);
> + cond_wake_up_nomb(&fs_info->dev_replace.replace_wait);
> }
>
> void btrfs_bio_counter_inc_blocked(struct btrfs_fs_info *fs_info)
> {
> while (1) {
> - percpu_counter_inc(&fs_info->bio_counter);
> + percpu_counter_inc(&fs_info->dev_replace.bio_counter);
> if (likely(!test_bit(BTRFS_FS_STATE_DEV_REPLACING,
> &fs_info->fs_state)))
> break;
>
> btrfs_bio_counter_dec(fs_info);
> - wait_event(fs_info->replace_wait,
> + wait_event(fs_info->dev_replace.replace_wait,
> !test_bit(BTRFS_FS_STATE_DEV_REPLACING,
> &fs_info->fs_state));
> }
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index acd0cbc5a6b9..3b3906589d12 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2156,7 +2156,7 @@ static void btrfs_init_dev_replace_locks(struct btrfs_fs_info *fs_info)
> mutex_init(&fs_info->dev_replace.lock_finishing_cancel_unmount);
> rwlock_init(&fs_info->dev_replace.lock);
> atomic_set(&fs_info->dev_replace.blocking_readers, 0);
> - init_waitqueue_head(&fs_info->replace_wait);
> + init_waitqueue_head(&fs_info->dev_replace.replace_wait);
> init_waitqueue_head(&fs_info->dev_replace.read_lock_wq);
> }
>
> @@ -2646,7 +2646,8 @@ int open_ctree(struct super_block *sb,
> goto fail_dirty_metadata_bytes;
> }
>
> - ret = percpu_counter_init(&fs_info->bio_counter, 0, GFP_KERNEL);
> + ret = percpu_counter_init(&fs_info->dev_replace.bio_counter, 0,
> + GFP_KERNEL);
> if (ret) {
> err = ret;
> goto fail_delalloc_bytes;
> @@ -3307,7 +3308,7 @@ int open_ctree(struct super_block *sb,
>
> iput(fs_info->btree_inode);
> fail_bio_counter:
> - percpu_counter_destroy(&fs_info->bio_counter);
> + percpu_counter_destroy(&fs_info->dev_replace.bio_counter);
> fail_delalloc_bytes:
> percpu_counter_destroy(&fs_info->delalloc_bytes);
> fail_dirty_metadata_bytes:
> @@ -4016,7 +4017,7 @@ void close_ctree(struct btrfs_fs_info *fs_info)
>
> percpu_counter_destroy(&fs_info->dirty_metadata_bytes);
> percpu_counter_destroy(&fs_info->delalloc_bytes);
> - percpu_counter_destroy(&fs_info->bio_counter);
> + percpu_counter_destroy(&fs_info->dev_replace.bio_counter);
> cleanup_srcu_struct(&fs_info->subvol_srcu);
>
> btrfs_free_stripe_hash_table(fs_info);
> --
> 2.18.0
>
next prev parent reply other threads:[~2018-09-08 1:52 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-07 14:54 [PATCH 00/11] Cleanup dev-replace locking David Sterba
2018-09-07 14:55 ` [PATCH 01/11] btrfs: remove btrfs_dev_replace::read_locks David Sterba
2018-09-07 14:55 ` [PATCH 02/11] btrfs: open code btrfs_dev_replace_clear_lock_blocking David Sterba
2018-09-07 14:55 ` [PATCH 03/11] btrfs: open code btrfs_dev_replace_stats_inc David Sterba
2018-09-07 21:02 ` Omar Sandoval
2018-09-07 14:55 ` [PATCH 04/11] btrfs: open code btrfs_after_dev_replace_commit David Sterba
2018-09-07 21:03 ` Omar Sandoval
2018-09-07 14:55 ` [PATCH 05/11] btrfs: dev-replace: avoid useless lock on error handling path David Sterba
2018-09-07 21:06 ` Omar Sandoval
2018-09-14 16:53 ` David Sterba
2018-09-07 14:55 ` [PATCH 06/11] btrfs: dev-replace: move replace members out of fs_info David Sterba
2018-09-07 21:09 ` Omar Sandoval [this message]
2018-09-07 14:55 ` [PATCH 07/11] btrfs: dev-replace: remove pointless assert in write unlock David Sterba
2018-09-07 14:55 ` [PATCH 08/11] btrfs: reada: reorder dev-replace locks before radix tree preload David Sterba
2018-09-07 14:55 ` [PATCH 09/11] btrfs: dev-replace: swich locking to rw semaphore David Sterba
2018-09-07 14:55 ` [PATCH 10/11] btrfs: dev-replace: remove custom read/write blocking scheme David Sterba
2018-09-07 14:55 ` [PATCH 11/11] btrfs: dev-replace: open code trivial locking helpers David Sterba
2018-09-27 11:06 ` [PATCH 00/11] Cleanup dev-replace locking David Sterba
2018-10-04 1:06 ` Anand Jain
2018-10-04 10:13 ` David Sterba
2018-10-05 8:58 ` Anand Jain
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=20180907210948.GI29245@vader \
--to=osandov@osandov.com \
--cc=dsterba@suse.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.