btrfs: return errno instead of -1 from compression
PATCH v2 1/3 btrfs: return errno instead of -1 from compression
The compression layer seems to have been built to return -1 and have callers make up errors that make sense. This isn't great because there are different errors that originate down in the compression layer.
Let's return real negative errnos from the compression layer so that callers can pass on the error without having to guess what happened. ENOMEM for allocation failure, E2BIG when compression exceeds the uncompressed input, and EIO for everything else.
This helps a future path return errors from btrfs_decompress().
--- fs/btrfs/lzo.c | 14 +++++++------- fs/btrfs/zlib.c | 26 +++++++++++++------------- 2 files changed, 20 insertions(+), 20 deletions(-)
fs/btrfs/lzo.c +7 -7
@@ -143,7 +143,7 @@ static int lzo_compress_pages(struct list_head *ws,if (ret !=LZO_E_OK ) {printk (KERN_DEBUG "BTRFS: deflate in loop returned %d\n" , ret); - ret =-1 ; + ret = -EIO ; goto out; }
@@ -189,7 +189,7 @@ static int lzo_compress_pages(struct list_head *ws,kunmap (out_page);if (nr_pages == nr_dest_pages) { out_page =NULL ; - ret =-1 ; + ret = -E2BIG ; goto out; }
@@ -208,7 +208,7 @@ static int lzo_compress_pages(struct list_head *ws,/* we're making it bigger, give up */ if (tot_in >8192 && tot_in < tot_out) { - ret =-1 ; + ret = -E2BIG ; goto out; }
@@ -335,7 +335,7 @@ cont:break ;if (page_in_index +1 >= total_pages_in) { - ret =-1 ; + ret = -EIO ; goto done; }
@@ -358,7 +358,7 @@ cont:kunmap (pages_in[page_in_index -1 ]);if (ret !=LZO_E_OK ) {printk (KERN_WARNING "BTRFS: decompress failed\n" ); - ret =-1 ; + ret = -EIO ;break ; }
@@ -402,12 +402,12 @@ static int lzo_decompress(struct list_head *ws, unsigned char *data_in, ret =lzo1x_decompress_safe (data_in, in_len, workspace->buf, &out_len);if (ret !=LZO_E_OK ) {printk (KERN_WARNING "BTRFS: decompress failed!\n" ); - ret =-1 ; + ret = -EIO ; goto out; }if (out_len < start_byte) { - ret =-1 ; + ret = -EIO ; goto out; }
fs/btrfs/zlib.c +13 -13
@@ -98,7 +98,7 @@ static int zlib_compress_pages(struct list_head *ws,if (Z_OK !=zlib_deflateInit (&workspace->def_strm,3 )) {printk (KERN_WARNING "BTRFS: deflateInit failed\n" ); - ret =-1 ; + ret = -EIO ; goto out; }
@@ -110,7 +110,7 @@ static int zlib_compress_pages(struct list_head *ws, out_page =alloc_page (GFP_NOFS | __GFP_HIGHMEM);if (out_page ==NULL ) { - ret =-1 ; + ret = -ENOMEM ; goto out; } cpage_out =kmap (out_page);
@@ -128,7 +128,7 @@ static int zlib_compress_pages(struct list_head *ws,printk (KERN_DEBUG "BTRFS: deflate in loop returned %d\n" , ret);zlib_deflateEnd (&workspace->def_strm); - ret =-1 ; + ret = -EIO ; goto out; }
@@ -136,7 +136,7 @@ static int zlib_compress_pages(struct list_head *ws,if (workspace->def_strm.total_in >8192 && workspace->def_strm.total_in < workspace->def_strm.total_out) { - ret =-1 ; + ret = -EIO ; goto out; }/* we need another page for writing out. Test this
@@ -147,12 +147,12 @@ static int zlib_compress_pages(struct list_head *ws,kunmap (out_page);if (nr_pages == nr_dest_pages) { out_page =NULL ; - ret =-1 ; + ret = -E2BIG ; goto out; } out_page =alloc_page (GFP_NOFS | __GFP_HIGHMEM);if (out_page ==NULL ) { - ret =-1 ; + ret = -ENOMEM ; goto out; } cpage_out =kmap (out_page);
@@ -188,12 +188,12 @@ static int zlib_compress_pages(struct list_head *ws,zlib_deflateEnd (&workspace->def_strm);if (ret !=Z_STREAM_END ) { - ret =-1 ; + ret = -EIO ; goto out; }if (workspace->def_strm.total_out >= workspace->def_strm.total_in) { - ret =-1 ; + ret = -E2BIG ; goto out; }
@@ -253,7 +253,7 @@ static int zlib_decompress_biovec(struct list_head *ws, struct page **pages_in,if (Z_OK !=zlib_inflateInit2 (&workspace->inf_strm, wbits)) {printk (KERN_WARNING "BTRFS: inflateInit failed\n" ); -return -1 ; +return -EIO ; }while (workspace->inf_strm.total_in < srclen) { ret =zlib_inflate (&workspace->inf_strm,Z_NO_FLUSH );
@@ -295,7 +295,7 @@ static int zlib_decompress_biovec(struct list_head *ws, struct page **pages_in, } }if (ret !=Z_STREAM_END ) - ret =-1 ; + ret = -EIO ;else ret =0 ; done:
@@ -337,7 +337,7 @@ static int zlib_decompress(struct list_head *ws, unsigned char *data_in,if (Z_OK !=zlib_inflateInit2 (&workspace->inf_strm, wbits)) {printk (KERN_WARNING "BTRFS: inflateInit failed\n" ); -return -1 ; +return -EIO ; }while (bytes_left >0 ) {
@@ -354,7 +354,7 @@ static int zlib_decompress(struct list_head *ws, unsigned char *data_in, total_out = workspace->inf_strm.total_out;if (total_out == buf_start) { - ret =-1 ; + ret = -EIO ;break ; }
@@ -382,7 +382,7 @@ next: }if (ret !=Z_STREAM_END && bytes_left !=0 ) - ret =-1 ; + ret = -EIO ;else ret =0 ;
Signature · 1 line
1.8.1.4
PATCH v2 2/3 btrfs: return ptr error from compression workspace
The btrfs compression wrappers translated errors from workspace allocation to either -ENOMEM or -1. The compression type workspace allocators are already returning a ERR_PTR(-ENOMEM). Just return that and get rid of the magical -1.
This helps a future patch return errors from the compression wrappers.
--- fs/btrfs/compression.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
fs/btrfs/compression.c +3 -3
@@ -888,7 +888,7 @@ int btrfs_compress_pages(int type, struct address_space *mapping, workspace =find_workspace (type);if (IS_ERR (workspace)) -return -1 ; +return PTR_ERR (workspace); ret = btrfs_compress_op[type-1]->compress_pages(workspace, mapping, start, len, pages,
@@ -924,7 +924,7 @@ static int btrfs_decompress_biovec(int type, struct page **pages_in, workspace =find_workspace (type);if (IS_ERR (workspace)) -return -ENOMEM ; +return PTR_ERR (workspace); ret = btrfs_compress_op[type-1]->decompress_biovec(workspace, pages_in, disk_start,
@@ -946,7 +946,7 @@ int btrfs_decompress(int type, unsigned char *data_in, struct page *dest_page, workspace =find_workspace (type);if (IS_ERR (workspace)) -return -ENOMEM ; +return PTR_ERR (workspace); ret = btrfs_compress_op[type-1]->decompress(workspace, data_in, dest_page, start_byte,
Signature · 1 line
1.8.1.4
PATCH v2 3/3 btrfs: fix inline compressed read err corruption
In reply to Zach Brownuncompress_inline() is dropping the error from btrfs_decompress() after testing it and zeroing the page that was supposed to hold decompressed data. This can silently turn compressed inline data in to zeros if decompression fails due to corrupt compressed data or memory allocation failure.
I verified this by manually forcing the error from btrfs_decompress() for a silly named copy of od:
if (!strcmp(current->comm, "failod")) ret = -ENOMEM;
# od -x /mnt/btrfs/dir/80 | head -1 0000000 3031 3038 310a 2d30 6f70 6e69 0a74 3031 # echo 3 > /proc/sys/vm/drop_caches # cp $(which od) /tmp/failod # /tmp/failod -x /mnt/btrfs/dir/80 | head -1 0000000 0000 0000 0000 0000 0000 0000 0000 0000
The fix is to pass the error to its caller. Which still has a BUG_ON(). So we fix that too.
There seems to be no reason for the zeroing of the page on the error from btrfs_decompress() but not from the allocation error a few lines above. So the page zeroing is removed.
--- fs/btrfs/inode.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
fs/btrfs/inode.c +5 -10
@@ -6082,16 +6082,8 @@ static noinline int uncompress_inline(struct btrfs_path *path, max_size = min_t(unsigned long, PAGE_CACHE_SIZE, max_size); ret =btrfs_decompress (compress_type, tmp, page, extent_offset, inline_size, max_size); -if (ret) { -char *kaddr =kmap_atomic (page); -unsigned long copy_size =min_t (u64, -PAGE_CACHE_SIZE - pg_offset, - max_size - extent_offset); -memset (kaddr + pg_offset,0 , copy_size); -kunmap_atomic (kaddr); - }kfree (tmp); -return 0 ; +return ret; }/*
@@ -6292,7 +6284,10 @@ next: ret =uncompress_inline (path, inode, page, pg_offset, extent_offset, item); -BUG_ON (ret);/* -ENOMEM */ +if (ret) { + err = ret; + goto out; + } }else { map =kmap (page); read_extent_buffer(leaf, map + pg_offset, ptr,
Signature · 1 line
1.8.1.4
fs/btrfs/zlib.c static int zlib_compress_pages(struct list_head *ws, · line 139
Quoted 8 lines · Zach Brown
fs/btrfs/zlib.c +1
-1@@ -136,7 +136,7 @@ static int zlib_compress_pages(struct list_head *ws,if (workspace->def_strm.total_in >8192 && workspace->def_strm.total_in < workspace->def_strm.total_out) { - ret =-1 ; + ret = -EIO ;
E2BIG, the comment above the code says
/* we're making it bigger, give up */
and the error code is consistent with lzo.c
The rest is ok.