btrfs: do not double complete bio on errors during compressed reads

I hit some weird panics while fixing up the error handling from
btrfs_lookup_bio_sums().  Turns out the compression path will complete
the bio we use if we set up any of the compression bios and then return
an error, and then btrfs_submit_data_bio() will also call bio_endio() on
the bio.

Fix this by making btrfs_submit_compressed_read() responsible for
calling bio_endio() on the bio if there are any errors.  Currently it
was only doing it if we created the compression bios, otherwise it was
depending on btrfs_submit_data_bio() to do the right thing.  This
creates the above problem, so fix up btrfs_submit_compressed_read() to
always call bio_endio() in case of an error, and then simply return from
btrfs_submit_data_bio() if we had to call
btrfs_submit_compressed_read().

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index f459e1832cad92f6aaef64175f4d7360ecc0dbdb..be476f094300ac8e53e3309963cd469664a6b31e 100644 (file)
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -811,7 +811,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
        u64 em_len;
        u64 em_start;
        struct extent_map *em;
-       blk_status_t ret = BLK_STS_RESOURCE;
+       blk_status_t ret;
        int faili = 0;
        u8 *sums;
 
@@ -824,14 +824,18 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
        read_lock(&em_tree->lock);
        em = lookup_extent_mapping(em_tree, file_offset, fs_info->sectorsize);
        read_unlock(&em_tree->lock);
-       if (!em)
-               return BLK_STS_IOERR;
+       if (!em) {
+               ret = BLK_STS_IOERR;
+               goto out;
+       }
 
        ASSERT(em->compress_type != BTRFS_COMPRESS_NONE);
        compressed_len = em->block_len;
        cb = kmalloc(compressed_bio_size(fs_info, compressed_len), GFP_NOFS);
-       if (!cb)
+       if (!cb) {
+               ret = BLK_STS_RESOURCE;
                goto out;
+       }
 
        refcount_set(&cb->pending_sectors, compressed_len >> fs_info->sectorsize_bits);
        cb->status = BLK_STS_OK;
@@ -854,8 +858,10 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
        nr_pages = DIV_ROUND_UP(compressed_len, PAGE_SIZE);
        cb->compressed_pages = kcalloc(nr_pages, sizeof(struct page *),
                                       GFP_NOFS);
-       if (!cb->compressed_pages)
+       if (!cb->compressed_pages) {
+               ret = BLK_STS_RESOURCE;
                goto fail1;
+       }
 
        for (pg_index = 0; pg_index < nr_pages; pg_index++) {
                cb->compressed_pages[pg_index] = alloc_page(GFP_NOFS);
@@ -941,7 +947,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
                        comp_bio = NULL;
                }
        }
-       return 0;
+       return BLK_STS_OK;
 
 fail2:
        while (faili >= 0) {
@@ -954,6 +960,8 @@ fail1:
        kfree(cb);
 out:
        free_extent_map(em);
+       bio->bi_status = ret;
+       bio_endio(bio);
        return ret;
 finish_cb:
        if (comp_bio) {

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 964eab60742fc1c519cfd9d33b70ef5e0fd95a13..53f46d5c78c77ad1ebcdf8897bde118883a53a7d 100644 (file)
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2543,10 +2543,15 @@ blk_status_t btrfs_submit_data_bio(struct inode *inode, struct bio *bio,
                        goto out;
 
                if (bio_flags & EXTENT_BIO_COMPRESSED) {
+                       /*
+                        * btrfs_submit_compressed_read will handle completing
+                        * the bio if there were any errors, so just return
+                        * here.
+                        */
                        ret = btrfs_submit_compressed_read(inode, bio,
                                                           mirror_num,
                                                           bio_flags);
-                       goto out;
+                       goto out_no_endio;
                } else {
                        /*
                         * Lookup bio sums does extra checks around whether we
@@ -2580,6 +2585,7 @@ out:
                bio->bi_status = ret;
                bio_endio(bio);
        }
+out_no_endio:
        return ret;
 }