On Tue, Nov 14, 2017 at 04:56:56PM -0500, Josef Bacik wrote:
> From: Josef Bacik <jbacik@xxxxxx>
>
> Now that the only thing that keeps eb's alive is io_pages and it's
> refcount we need to hold the eb ref for the entire end io call so we
> don't get it removed out from underneath us. Also the hooks make no
> sense for us now, so rework this to be cleaner.
>
> Signed-off-by: Josef Bacik <jbacik@xxxxxx>
> ---
> fs/btrfs/disk-io.c | 63 ++++--------------------------------------------
> fs/btrfs/disk-io.h | 1 +
> fs/btrfs/extent_io.c | 67 +++++++++++++++++++++++++++-------------------------
> 3 files changed, 41 insertions(+), 90 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 7ccb6d839126..459491d662a0 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -755,33 +755,13 @@ static int check_node(struct btrfs_root *root, struct extent_buffer *node)
> return ret;
> }
>
> -static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
> - u64 phy_offset, struct page *page,
> - u64 start, u64 end, int mirror)
> +int btrfs_extent_buffer_end_read(struct extent_buffer *eb, int mirror)
> {
> + struct btrfs_fs_info *fs_info = eb->eb_info->fs_info;
> + struct btrfs_root *root = fs_info->tree_root;
> u64 found_start;
> int found_level;
> - struct extent_buffer *eb;
> - struct btrfs_root *root;
> - struct btrfs_fs_info *fs_info;
> int ret = 0;
> - int reads_done;
> -
> - if (!page->private)
> - goto out;
> -
> - eb = (struct extent_buffer *)page->private;
> -
> - /* the pending IO might have been the only thing that kept this buffer
> - * in memory. Make sure we have a ref for all this other checks
> - */
> - extent_buffer_get(eb);
> - fs_info = eb->eb_info->fs_info;
> - root = fs_info->tree_root;
> -
> - reads_done = atomic_dec_and_test(&eb->io_pages);
> - if (!reads_done)
> - goto err;
>
> eb->read_mirror = mirror;
> if (test_bit(EXTENT_BUFFER_READ_ERR, &eb->bflags)) {
> @@ -833,45 +813,14 @@ static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
> if (!ret)
> set_extent_buffer_uptodate(eb);
> err:
> - if (reads_done &&
> - test_and_clear_bit(EXTENT_BUFFER_READAHEAD, &eb->bflags))
> + if (test_and_clear_bit(EXTENT_BUFFER_READAHEAD, &eb->bflags))
> btree_readahead_hook(eb, ret);
>
> - if (ret) {
> - /*
> - * our io error hook is going to dec the io pages
> - * again, we have to make sure it has something
> - * to decrement.
> - *
> - * TODO: Kill this, we've re-arranged how this works now so we
> - * don't need to do this io_pages dance.
> - */
> - atomic_inc(&eb->io_pages);
> + if (ret)
> clear_extent_buffer_uptodate(eb);
> - }
> - if (reads_done) {
> - clear_bit(EXTENT_BUFFER_READING, &eb->bflags);
> - smp_mb__after_atomic();
> - wake_up_bit(&eb->bflags, EXTENT_BUFFER_READING);
> - }
> - free_extent_buffer(eb);
> -out:
> return ret;
> }
>
> -static int btree_io_failed_hook(struct page *page, int failed_mirror)
> -{
> - struct extent_buffer *eb;
> -
> - eb = (struct extent_buffer *)page->private;
> - set_bit(EXTENT_BUFFER_READ_ERR, &eb->bflags);
> - eb->read_mirror = failed_mirror;
> - atomic_dec(&eb->io_pages);
> - if (test_and_clear_bit(EXTENT_BUFFER_READAHEAD, &eb->bflags))
> - btree_readahead_hook(eb, -EIO);
> - return -EIO; /* we fixed nothing */
> -}
> -
> static void end_workqueue_bio(struct bio *bio)
> {
> struct btrfs_end_io_wq *end_io_wq = bio->bi_private;
> @@ -4553,9 +4502,7 @@ static int btree_merge_bio_hook(struct page *page, unsigned long offset,
> static const struct extent_io_ops btree_extent_io_ops = {
> /* mandatory callbacks */
> .submit_bio_hook = btree_submit_bio_hook,
> - .readpage_end_io_hook = btree_readpage_end_io_hook,
> .merge_bio_hook = btree_merge_bio_hook,
> - .readpage_io_failed_hook = btree_io_failed_hook,
> .set_range_writeback = btrfs_set_range_writeback,
> .tree_fs_info = btree_fs_info,
>
> diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
> index 7f7c35d6347a..e1f4fef91547 100644
> --- a/fs/btrfs/disk-io.h
> +++ b/fs/btrfs/disk-io.h
> @@ -152,6 +152,7 @@ int btree_lock_page_hook(struct page *page, void *data,
> int btrfs_get_num_tolerated_disk_barrier_failures(u64 flags);
> int __init btrfs_end_io_wq_init(void);
> void btrfs_end_io_wq_exit(void);
> +int btrfs_extent_buffer_end_read(struct extent_buffer *eb, int mirror);
>
> #ifdef CONFIG_DEBUG_LOCK_ALLOC
> void btrfs_init_lockdep(void);
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 2077bd6ad1b3..1e5affee0f7e 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -20,6 +20,7 @@
> #include "locking.h"
> #include "rcu-string.h"
> #include "backref.h"
> +#include "disk-io.h"
>
> static struct kmem_cache *extent_state_cache;
> static struct kmem_cache *extent_buffer_cache;
> @@ -5375,6 +5376,15 @@ int extent_buffer_uptodate(struct extent_buffer *eb)
> return test_bit(EXTENT_BUFFER_UPTODATE, &eb->bflags);
> }
>
> +static void mark_eb_failed(struct extent_buffer *eb, int failed_mirror)
> +{
> + set_bit(EXTENT_BUFFER_READ_ERR, &eb->bflags);
> + eb->read_mirror = failed_mirror;
> + atomic_dec(&eb->io_pages);
> + if (test_and_clear_bit(EXTENT_BUFFER_READAHEAD, &eb->bflags))
> + btree_readahead_hook(eb, -EIO);
> +}
> +
> static void end_bio_extent_buffer_readpage(struct bio *bio)
> {
> struct btrfs_io_bio *io_bio = btrfs_io_bio(bio);
> @@ -5383,12 +5393,13 @@ static void end_bio_extent_buffer_readpage(struct bio *bio)
> u64 unlock_start = 0, unlock_len = 0;
> int mirror_num = io_bio->mirror_num;
> int uptodate = !bio->bi_status;
> - int i, ret;
> + int i;
>
> bio_for_each_segment_all(bvec, bio, i) {
> struct page *page = bvec->bv_page;
> struct btrfs_eb_info *eb_info;
> struct extent_buffer *eb;
> + int reads_done;
>
> eb = (struct extent_buffer *)page->private;
> if (WARN_ON(!eb))
> @@ -5397,41 +5408,33 @@ static void end_bio_extent_buffer_readpage(struct bio *bio)
> eb_info = eb->eb_info;
> if (!tree)
> tree = &eb_info->io_tree;
> + extent_buffer_get(eb);
> + reads_done = atomic_dec_and_test(&eb->io_pages);
> if (uptodate) {
> - /*
> - * btree_readpage_end_io_hook doesn't care about
> - * start/end so just pass 0. We'll kill this later.
> - */
> - ret = tree->ops->readpage_end_io_hook(io_bio, 0,
> - page, 0, 0,
> - mirror_num);
> - if (ret) {
> - uptodate = 0;
> - } else {
> - u64 start = eb->start;
> - int c, num_pages;
> -
> - num_pages = num_extent_pages(eb->start,
> - eb->len);
> - for (c = 0; c < num_pages; c++) {
> - if (eb->pages[c] == page)
> - break;
> - start += PAGE_SIZE;
> - }
> - clean_io_failure(eb_info->fs_info,
> - &eb_info->io_failure_tree,
> - tree, start, page, 0, 0);
> + u64 start = eb->start;
> + int c, num_pages;
> +
> + num_pages = num_extent_pages(eb->start,
> + eb->len);
> + for (c = 0; c < num_pages; c++) {
> + if (eb->pages[c] == page)
> + break;
> + start += PAGE_SIZE;
> }
> + clean_io_failure(eb_info->fs_info,
> + &eb_info->io_failure_tree,
> + tree, start, page, 0, 0);
We don't fix failures here, so this can be removed.
> }
> - /*
> - * We never fix anything in btree_io_failed_hook.
> - *
> - * TODO: rework the io failed hook to not assume we can fix
> - * anything.
> - */
> + if (reads_done && btrfs_extent_buffer_end_read(eb, mirror_num))
> + uptodate = 0;
> if (!uptodate)
> - tree->ops->readpage_io_failed_hook(page, mirror_num);
> -
> + mark_eb_failed(eb, mirror_num);
Correct me if I'm wrong, if !uptodate, ->io_pages would be dec'd twice
here, eb would be checked without all pages hitting end_io.
Thanks,
-liubo
> + if (reads_done) {
> + clear_bit(EXTENT_BUFFER_READING, &eb->bflags);
> + smp_mb__after_atomic();
> + wake_up_bit(&eb->bflags, EXTENT_BUFFER_READING);
> + }
> + free_extent_buffer(eb);
> if (unlock_start == 0) {
> unlock_start = eb->start;
> unlock_len = PAGE_SIZE;
> --
> 2.7.5
>
