From 2fe1d55134fce05c17ea118a2e37a4af771887bc Mon Sep 17 00:00:00 2001 From: Omar Sandoval Date: Thu, 22 Sep 2016 17:24:20 -0700 Subject: Btrfs: fix free space tree bitmaps on big-endian systems MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In convert_free_space_to_{bitmaps,extents}(), we buffer the free space bitmaps in memory and copy them directly to/from the extent buffers with {read,write}_extent_buffer(). The extent buffer bitmap helpers use byte granularity, which is equivalent to a little-endian bitmap. This means that on big-endian systems, the in-memory bitmaps will be written to disk byte-swapped. To fix this, use byte-granularity for the bitmaps in memory. Fixes: a5ed91828518 ("Btrfs: implement the free space B-tree") Cc: stable@vger.kernel.org # 4.5+ Tested-by: Holger Hoffstätte Tested-by: Chandan Rajendra Signed-off-by: Omar Sandoval Signed-off-by: David Sterba --- fs/btrfs/extent_io.c | 64 +++++++++++++++++++++++++++++++++------------- fs/btrfs/extent_io.h | 22 ++++++++++++++++ fs/btrfs/free-space-tree.c | 17 ++++++------ 3 files changed, 76 insertions(+), 27 deletions(-) diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 44fe66b53c8b..c3ec30dea9a5 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -5524,17 +5524,45 @@ void copy_extent_buffer(struct extent_buffer *dst, struct extent_buffer *src, } } -/* - * The extent buffer bitmap operations are done with byte granularity because - * bitmap items are not guaranteed to be aligned to a word and therefore a - * single word in a bitmap may straddle two pages in the extent buffer. - */ -#define BIT_BYTE(nr) ((nr) / BITS_PER_BYTE) -#define BYTE_MASK ((1 << BITS_PER_BYTE) - 1) -#define BITMAP_FIRST_BYTE_MASK(start) \ - ((BYTE_MASK << ((start) & (BITS_PER_BYTE - 1))) & BYTE_MASK) -#define BITMAP_LAST_BYTE_MASK(nbits) \ - (BYTE_MASK >> (-(nbits) & (BITS_PER_BYTE - 1))) +void le_bitmap_set(u8 *map, unsigned int start, int len) +{ + u8 *p = map + BIT_BYTE(start); + const unsigned int size = start + len; + int bits_to_set = BITS_PER_BYTE - (start % BITS_PER_BYTE); + u8 mask_to_set = BITMAP_FIRST_BYTE_MASK(start); + + while (len - bits_to_set >= 0) { + *p |= mask_to_set; + len -= bits_to_set; + bits_to_set = BITS_PER_BYTE; + mask_to_set = ~(u8)0; + p++; + } + if (len) { + mask_to_set &= BITMAP_LAST_BYTE_MASK(size); + *p |= mask_to_set; + } +} + +void le_bitmap_clear(u8 *map, unsigned int start, int len) +{ + u8 *p = map + BIT_BYTE(start); + const unsigned int size = start + len; + int bits_to_clear = BITS_PER_BYTE - (start % BITS_PER_BYTE); + u8 mask_to_clear = BITMAP_FIRST_BYTE_MASK(start); + + while (len - bits_to_clear >= 0) { + *p &= ~mask_to_clear; + len -= bits_to_clear; + bits_to_clear = BITS_PER_BYTE; + mask_to_clear = ~(u8)0; + p++; + } + if (len) { + mask_to_clear &= BITMAP_LAST_BYTE_MASK(size); + *p &= ~mask_to_clear; + } +} /* * eb_bitmap_offset() - calculate the page and offset of the byte containing the @@ -5578,7 +5606,7 @@ static inline void eb_bitmap_offset(struct extent_buffer *eb, int extent_buffer_test_bit(struct extent_buffer *eb, unsigned long start, unsigned long nr) { - char *kaddr; + u8 *kaddr; struct page *page; unsigned long i; size_t offset; @@ -5600,13 +5628,13 @@ int extent_buffer_test_bit(struct extent_buffer *eb, unsigned long start, void extent_buffer_bitmap_set(struct extent_buffer *eb, unsigned long start, unsigned long pos, unsigned long len) { - char *kaddr; + u8 *kaddr; struct page *page; unsigned long i; size_t offset; const unsigned int size = pos + len; int bits_to_set = BITS_PER_BYTE - (pos % BITS_PER_BYTE); - unsigned int mask_to_set = BITMAP_FIRST_BYTE_MASK(pos); + u8 mask_to_set = BITMAP_FIRST_BYTE_MASK(pos); eb_bitmap_offset(eb, start, pos, &i, &offset); page = eb->pages[i]; @@ -5617,7 +5645,7 @@ void extent_buffer_bitmap_set(struct extent_buffer *eb, unsigned long start, kaddr[offset] |= mask_to_set; len -= bits_to_set; bits_to_set = BITS_PER_BYTE; - mask_to_set = ~0U; + mask_to_set = ~(u8)0; if (++offset >= PAGE_SIZE && len > 0) { offset = 0; page = eb->pages[++i]; @@ -5642,13 +5670,13 @@ void extent_buffer_bitmap_set(struct extent_buffer *eb, unsigned long start, void extent_buffer_bitmap_clear(struct extent_buffer *eb, unsigned long start, unsigned long pos, unsigned long len) { - char *kaddr; + u8 *kaddr; struct page *page; unsigned long i; size_t offset; const unsigned int size = pos + len; int bits_to_clear = BITS_PER_BYTE - (pos % BITS_PER_BYTE); - unsigned int mask_to_clear = BITMAP_FIRST_BYTE_MASK(pos); + u8 mask_to_clear = BITMAP_FIRST_BYTE_MASK(pos); eb_bitmap_offset(eb, start, pos, &i, &offset); page = eb->pages[i]; @@ -5659,7 +5687,7 @@ void extent_buffer_bitmap_clear(struct extent_buffer *eb, unsigned long start, kaddr[offset] &= ~mask_to_clear; len -= bits_to_clear; bits_to_clear = BITS_PER_BYTE; - mask_to_clear = ~0U; + mask_to_clear = ~(u8)0; if (++offset >= PAGE_SIZE && len > 0) { offset = 0; page = eb->pages[++i]; diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h index 28cd88fccc7e..1cf4e4226fc8 100644 --- a/fs/btrfs/extent_io.h +++ b/fs/btrfs/extent_io.h @@ -59,6 +59,28 @@ */ #define EXTENT_PAGE_PRIVATE 1 +/* + * The extent buffer bitmap operations are done with byte granularity instead of + * word granularity for two reasons: + * 1. The bitmaps must be little-endian on disk. + * 2. Bitmap items are not guaranteed to be aligned to a word and therefore a + * single word in a bitmap may straddle two pages in the extent buffer. + */ +#define BIT_BYTE(nr) ((nr) / BITS_PER_BYTE) +#define BYTE_MASK ((1 << BITS_PER_BYTE) - 1) +#define BITMAP_FIRST_BYTE_MASK(start) \ + ((BYTE_MASK << ((start) & (BITS_PER_BYTE - 1))) & BYTE_MASK) +#define BITMAP_LAST_BYTE_MASK(nbits) \ + (BYTE_MASK >> (-(nbits) & (BITS_PER_BYTE - 1))) + +static inline int le_test_bit(int nr, const u8 *addr) +{ + return 1U & (addr[BIT_BYTE(nr)] >> (nr & (BITS_PER_BYTE-1))); +} + +extern void le_bitmap_set(u8 *map, unsigned int start, int len); +extern void le_bitmap_clear(u8 *map, unsigned int start, int len); + struct extent_state; struct btrfs_root; struct btrfs_io_bio; diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c index 87e7e3d3e676..8fd85bfbe2da 100644 --- a/fs/btrfs/free-space-tree.c +++ b/fs/btrfs/free-space-tree.c @@ -151,7 +151,7 @@ static inline u32 free_space_bitmap_size(u64 size, u32 sectorsize) return DIV_ROUND_UP((u32)div_u64(size, sectorsize), BITS_PER_BYTE); } -static unsigned long *alloc_bitmap(u32 bitmap_size) +static u8 *alloc_bitmap(u32 bitmap_size) { void *mem; @@ -180,8 +180,7 @@ int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans, struct btrfs_free_space_info *info; struct btrfs_key key, found_key; struct extent_buffer *leaf; - unsigned long *bitmap; - char *bitmap_cursor; + u8 *bitmap, *bitmap_cursor; u64 start, end; u64 bitmap_range, i; u32 bitmap_size, flags, expected_extent_count; @@ -231,7 +230,7 @@ int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans, block_group->sectorsize); last = div_u64(found_key.objectid + found_key.offset - start, block_group->sectorsize); - bitmap_set(bitmap, first, last - first); + le_bitmap_set(bitmap, first, last - first); extent_count++; nr++; @@ -269,7 +268,7 @@ int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans, goto out; } - bitmap_cursor = (char *)bitmap; + bitmap_cursor = bitmap; bitmap_range = block_group->sectorsize * BTRFS_FREE_SPACE_BITMAP_BITS; i = start; while (i < end) { @@ -318,7 +317,7 @@ int convert_free_space_to_extents(struct btrfs_trans_handle *trans, struct btrfs_free_space_info *info; struct btrfs_key key, found_key; struct extent_buffer *leaf; - unsigned long *bitmap; + u8 *bitmap; u64 start, end; /* Initialize to silence GCC. */ u64 extent_start = 0; @@ -362,7 +361,7 @@ int convert_free_space_to_extents(struct btrfs_trans_handle *trans, break; } else if (found_key.type == BTRFS_FREE_SPACE_BITMAP_KEY) { unsigned long ptr; - char *bitmap_cursor; + u8 *bitmap_cursor; u32 bitmap_pos, data_size; ASSERT(found_key.objectid >= start); @@ -372,7 +371,7 @@ int convert_free_space_to_extents(struct btrfs_trans_handle *trans, bitmap_pos = div_u64(found_key.objectid - start, block_group->sectorsize * BITS_PER_BYTE); - bitmap_cursor = ((char *)bitmap) + bitmap_pos; + bitmap_cursor = bitmap + bitmap_pos; data_size = free_space_bitmap_size(found_key.offset, block_group->sectorsize); @@ -409,7 +408,7 @@ int convert_free_space_to_extents(struct btrfs_trans_handle *trans, offset = start; bitnr = 0; while (offset < end) { - bit = !!test_bit(bitnr, bitmap); + bit = !!le_test_bit(bitnr, bitmap); if (prev_bit == 0 && bit == 1) { extent_start = offset; } else if (prev_bit == 1 && bit == 0) { -- cgit From f8d468a15c22b954b379aa0c74914d5068448fb1 Mon Sep 17 00:00:00 2001 From: Omar Sandoval Date: Thu, 22 Sep 2016 17:24:21 -0700 Subject: Btrfs: fix mount -o clear_cache,space_cache=v2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We moved the code for creating the free space tree the first time that it's enabled, but didn't move the clearing code along with it. This breaks my (undocumented) intention that `mount -o clear_cache,space_cache=v2` would clear the free space tree and then recreate it. Fixes: 511711af91f2 ("btrfs: don't run delayed references while we are creating the free space tree") Cc: stable@vger.kernel.org # 4.5+ Tested-by: Holger Hoffstätte Tested-by: Chandan Rajendra Signed-off-by: Omar Sandoval Signed-off-by: David Sterba --- fs/btrfs/disk-io.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 54bc8c7c6bcd..c0bfc6ce5f06 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -3129,6 +3129,18 @@ retry_root_backup: if (sb->s_flags & MS_RDONLY) return 0; + if (btrfs_test_opt(fs_info, CLEAR_CACHE) && + btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE)) { + btrfs_info(fs_info, "clearing free space tree"); + ret = btrfs_clear_free_space_tree(fs_info); + if (ret) { + btrfs_warn(fs_info, + "failed to clear free space tree: %d", ret); + close_ctree(tree_root); + return ret; + } + } + if (btrfs_test_opt(tree_root->fs_info, FREE_SPACE_TREE) && !btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE)) { btrfs_info(fs_info, "creating free space tree"); @@ -3166,18 +3178,6 @@ retry_root_backup: btrfs_qgroup_rescan_resume(fs_info); - if (btrfs_test_opt(tree_root->fs_info, CLEAR_CACHE) && - btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE)) { - btrfs_info(fs_info, "clearing free space tree"); - ret = btrfs_clear_free_space_tree(fs_info); - if (ret) { - btrfs_warn(fs_info, - "failed to clear free space tree: %d", ret); - close_ctree(tree_root); - return ret; - } - } - if (!fs_info->uuid_root) { btrfs_info(fs_info, "creating UUID tree"); ret = btrfs_create_uuid_tree(fs_info); -- cgit From 6675df311db87aa2107a04ef97e19420953cbace Mon Sep 17 00:00:00 2001 From: Omar Sandoval Date: Thu, 22 Sep 2016 17:24:22 -0700 Subject: Btrfs: catch invalid free space trees MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are two separate issues that can lead to corrupted free space trees. 1. The free space tree bitmaps had an endianness issue on big-endian systems which is fixed by an earlier patch in this series. 2. btrfs-progs before v4.7.3 modified filesystems without updating the free space tree. To catch both of these issues at once, we need to force the free space tree to be rebuilt. To do so, add a FREE_SPACE_TREE_VALID compat_ro bit. If the bit isn't set, we know that it was either produced by a broken big-endian kernel or may have been corrupted by btrfs-progs. This also provides us with a way to add rudimentary read-write support for the free space tree to btrfs-progs: it can just clear this bit and have the kernel rebuild the free space tree. Cc: stable@vger.kernel.org # 4.5+ Tested-by: Holger Hoffstätte Tested-by: Chandan Rajendra Signed-off-by: Omar Sandoval Signed-off-by: David Sterba --- fs/btrfs/ctree.h | 3 ++- fs/btrfs/disk-io.c | 9 +++++++++ fs/btrfs/free-space-tree.c | 2 ++ include/uapi/linux/btrfs.h | 12 +++++++++++- 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 33fe03551105..791e47ce9d27 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -251,7 +251,8 @@ struct btrfs_super_block { #define BTRFS_FEATURE_COMPAT_SAFE_CLEAR 0ULL #define BTRFS_FEATURE_COMPAT_RO_SUPP \ - (BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE) + (BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE | \ + BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID) #define BTRFS_FEATURE_COMPAT_RO_SAFE_SET 0ULL #define BTRFS_FEATURE_COMPAT_RO_SAFE_CLEAR 0ULL diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index c0bfc6ce5f06..3dede6d53bad 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -2566,6 +2566,7 @@ int open_ctree(struct super_block *sb, int num_backups_tried = 0; int backup_index = 0; int max_active; + int clear_free_space_tree = 0; tree_root = fs_info->tree_root = btrfs_alloc_root(fs_info, GFP_KERNEL); chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info, GFP_KERNEL); @@ -3131,6 +3132,14 @@ retry_root_backup: if (btrfs_test_opt(fs_info, CLEAR_CACHE) && btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE)) { + clear_free_space_tree = 1; + } else if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE) && + !btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE_VALID)) { + btrfs_warn(fs_info, "free space tree is invalid"); + clear_free_space_tree = 1; + } + + if (clear_free_space_tree) { btrfs_info(fs_info, "clearing free space tree"); ret = btrfs_clear_free_space_tree(fs_info); if (ret) { diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c index 8fd85bfbe2da..ea605ffd0e03 100644 --- a/fs/btrfs/free-space-tree.c +++ b/fs/btrfs/free-space-tree.c @@ -1182,6 +1182,7 @@ int btrfs_create_free_space_tree(struct btrfs_fs_info *fs_info) } btrfs_set_fs_compat_ro(fs_info, FREE_SPACE_TREE); + btrfs_set_fs_compat_ro(fs_info, FREE_SPACE_TREE_VALID); fs_info->creating_free_space_tree = 0; ret = btrfs_commit_transaction(trans, tree_root); @@ -1250,6 +1251,7 @@ int btrfs_clear_free_space_tree(struct btrfs_fs_info *fs_info) return PTR_ERR(trans); btrfs_clear_fs_compat_ro(fs_info, FREE_SPACE_TREE); + btrfs_clear_fs_compat_ro(fs_info, FREE_SPACE_TREE_VALID); fs_info->free_space_root = NULL; ret = clear_free_space_tree(trans, free_space_root); diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h index ac5eacd3055b..db4c253f8011 100644 --- a/include/uapi/linux/btrfs.h +++ b/include/uapi/linux/btrfs.h @@ -239,7 +239,17 @@ struct btrfs_ioctl_fs_info_args { * Used by: * struct btrfs_ioctl_feature_flags */ -#define BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE (1ULL << 0) +#define BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE (1ULL << 0) +/* + * Older kernels (< 4.9) on big-endian systems produced broken free space tree + * bitmaps, and btrfs-progs also used to corrupt the free space tree (versions + * < 4.7.3). If this bit is clear, then the free space tree cannot be trusted. + * btrfs-progs can also intentionally clear this bit to ask the kernel to + * rebuild the free space tree, however this might not work on older kernels + * that do not know about this bit. If not sure, clear the cache manually on + * first mount when booting older kernel versions. + */ +#define BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID (1ULL << 1) #define BTRFS_FEATURE_INCOMPAT_MIXED_BACKREF (1ULL << 0) #define BTRFS_FEATURE_INCOMPAT_DEFAULT_SUBVOL (1ULL << 1) -- cgit From 9426ce754fab0f02a45b61402119c57de446ffa3 Mon Sep 17 00:00:00 2001 From: Omar Sandoval Date: Thu, 22 Sep 2016 17:24:23 -0700 Subject: Btrfs: fix extent buffer bitmap tests on big-endian systems The in-memory bitmap code manipulates words and is therefore sensitive to endianness, while the extent buffer bitmap code addresses bytes and is byte-order agnostic. Because the byte addressing of the extent buffer bitmaps is equivalent to a little-endian in-memory bitmap, the extent buffer bitmap tests fail on big-endian systems. 34b3e6c92af1 ("Btrfs: self-tests: Fix extent buffer bitmap test fail on BE system") worked around another endianness bug in the tests but missed this one because ed9e4afdb055 ("Btrfs: self-tests: Execute page straddling test only when nodesize < PAGE_SIZE") disables this part of the test on ppc64. That change lost the original meaning of the test, however. We really want to test that an equivalent series of operations using the in-memory bitmap API and the extent buffer bitmap API produces equivalent results. To fix this, don't use memcmp_extent_buffer() or write_extent_buffer(); do everything bit-by-bit. Reported-by: Anatoly Pugachev Tested-by: Anatoly Pugachev Tested-by: Feifei Xu Tested-by: Chandan Rajendra Signed-off-by: Omar Sandoval Signed-off-by: David Sterba --- fs/btrfs/tests/extent-io-tests.c | 87 +++++++++++++++++++++++----------------- 1 file changed, 51 insertions(+), 36 deletions(-) diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c index d19ab0317283..caad80bb9bd0 100644 --- a/fs/btrfs/tests/extent-io-tests.c +++ b/fs/btrfs/tests/extent-io-tests.c @@ -273,20 +273,37 @@ out: return ret; } -/** - * test_bit_in_byte - Determine whether a bit is set in a byte - * @nr: bit number to test - * @addr: Address to start counting from - */ -static inline int test_bit_in_byte(int nr, const u8 *addr) +static int check_eb_bitmap(unsigned long *bitmap, struct extent_buffer *eb, + unsigned long len) { - return 1UL & (addr[nr / BITS_PER_BYTE] >> (nr & (BITS_PER_BYTE - 1))); + unsigned long i; + + for (i = 0; i < len * BITS_PER_BYTE; i++) { + int bit, bit1; + + bit = !!test_bit(i, bitmap); + bit1 = !!extent_buffer_test_bit(eb, 0, i); + if (bit1 != bit) { + test_msg("Bits do not match\n"); + return -EINVAL; + } + + bit1 = !!extent_buffer_test_bit(eb, i / BITS_PER_BYTE, + i % BITS_PER_BYTE); + if (bit1 != bit) { + test_msg("Offset bits do not match\n"); + return -EINVAL; + } + } + return 0; } static int __test_eb_bitmaps(unsigned long *bitmap, struct extent_buffer *eb, unsigned long len) { - unsigned long i, x; + unsigned long i, j; + u32 x; + int ret; memset(bitmap, 0, len); memset_extent_buffer(eb, 0, 0, len); @@ -297,16 +314,18 @@ static int __test_eb_bitmaps(unsigned long *bitmap, struct extent_buffer *eb, bitmap_set(bitmap, 0, len * BITS_PER_BYTE); extent_buffer_bitmap_set(eb, 0, 0, len * BITS_PER_BYTE); - if (memcmp_extent_buffer(eb, bitmap, 0, len) != 0) { + ret = check_eb_bitmap(bitmap, eb, len); + if (ret) { test_msg("Setting all bits failed\n"); - return -EINVAL; + return ret; } bitmap_clear(bitmap, 0, len * BITS_PER_BYTE); extent_buffer_bitmap_clear(eb, 0, 0, len * BITS_PER_BYTE); - if (memcmp_extent_buffer(eb, bitmap, 0, len) != 0) { + ret = check_eb_bitmap(bitmap, eb, len); + if (ret) { test_msg("Clearing all bits failed\n"); - return -EINVAL; + return ret; } /* Straddling pages test */ @@ -316,9 +335,10 @@ static int __test_eb_bitmaps(unsigned long *bitmap, struct extent_buffer *eb, sizeof(long) * BITS_PER_BYTE); extent_buffer_bitmap_set(eb, PAGE_SIZE - sizeof(long) / 2, 0, sizeof(long) * BITS_PER_BYTE); - if (memcmp_extent_buffer(eb, bitmap, 0, len) != 0) { + ret = check_eb_bitmap(bitmap, eb, len); + if (ret) { test_msg("Setting straddling pages failed\n"); - return -EINVAL; + return ret; } bitmap_set(bitmap, 0, len * BITS_PER_BYTE); @@ -328,9 +348,10 @@ static int __test_eb_bitmaps(unsigned long *bitmap, struct extent_buffer *eb, extent_buffer_bitmap_set(eb, 0, 0, len * BITS_PER_BYTE); extent_buffer_bitmap_clear(eb, PAGE_SIZE - sizeof(long) / 2, 0, sizeof(long) * BITS_PER_BYTE); - if (memcmp_extent_buffer(eb, bitmap, 0, len) != 0) { + ret = check_eb_bitmap(bitmap, eb, len); + if (ret) { test_msg("Clearing straddling pages failed\n"); - return -EINVAL; + return ret; } } @@ -339,28 +360,22 @@ static int __test_eb_bitmaps(unsigned long *bitmap, struct extent_buffer *eb, * something repetitive that could miss some hypothetical off-by-n bug. */ x = 0; - for (i = 0; i < len / sizeof(long); i++) { - x = (0x19660dULL * (u64)x + 0x3c6ef35fULL) & 0xffffffffUL; - bitmap[i] = x; - } - write_extent_buffer(eb, bitmap, 0, len); - - for (i = 0; i < len * BITS_PER_BYTE; i++) { - int bit, bit1; - - bit = !!test_bit_in_byte(i, (u8 *)bitmap); - bit1 = !!extent_buffer_test_bit(eb, 0, i); - if (bit1 != bit) { - test_msg("Testing bit pattern failed\n"); - return -EINVAL; + bitmap_clear(bitmap, 0, len * BITS_PER_BYTE); + extent_buffer_bitmap_clear(eb, 0, 0, len * BITS_PER_BYTE); + for (i = 0; i < len * BITS_PER_BYTE / 32; i++) { + x = (0x19660dULL * (u64)x + 0x3c6ef35fULL) & 0xffffffffU; + for (j = 0; j < 32; j++) { + if (x & (1U << j)) { + bitmap_set(bitmap, i * 32 + j, 1); + extent_buffer_bitmap_set(eb, 0, i * 32 + j, 1); + } } + } - bit1 = !!extent_buffer_test_bit(eb, i / BITS_PER_BYTE, - i % BITS_PER_BYTE); - if (bit1 != bit) { - test_msg("Testing bit pattern with offset failed\n"); - return -EINVAL; - } + ret = check_eb_bitmap(bitmap, eb, len); + if (ret) { + test_msg("Random bit pattern failed\n"); + return ret; } return 0; -- cgit From 781e3bcf0e7632736d7562b52451a2d4fdfa231c Mon Sep 17 00:00:00 2001 From: Omar Sandoval Date: Thu, 22 Sep 2016 17:24:24 -0700 Subject: Btrfs: expand free space tree sanity tests to catch endianness bug The free space tree format conversion functions were broken on big-endian systems, but the sanity tests didn't catch it because all of the operations were aligned to multiple words. This was meant to catch any bugs in the extent buffer code's handling of high memory, but it ended up hiding the endianness bug. Expand the tests to do both sector-aligned and page-aligned operations. Tested-by: Chandan Rajendra Signed-off-by: Omar Sandoval Signed-off-by: David Sterba --- fs/btrfs/tests/free-space-tree-tests.c | 164 +++++++++++++++++++-------------- 1 file changed, 96 insertions(+), 68 deletions(-) diff --git a/fs/btrfs/tests/free-space-tree-tests.c b/fs/btrfs/tests/free-space-tree-tests.c index 7508d3b42780..c449da4d77b9 100644 --- a/fs/btrfs/tests/free-space-tree-tests.c +++ b/fs/btrfs/tests/free-space-tree-tests.c @@ -27,12 +27,6 @@ struct free_space_extent { u64 start, length; }; -/* - * The test cases align their operations to this in order to hit some of the - * edge cases in the bitmap code. - */ -#define BITMAP_RANGE (BTRFS_FREE_SPACE_BITMAP_BITS * PAGE_SIZE) - static int __check_free_space_extents(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, struct btrfs_block_group_cache *cache, @@ -168,7 +162,8 @@ static int check_free_space_extents(struct btrfs_trans_handle *trans, static int test_empty_block_group(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, struct btrfs_block_group_cache *cache, - struct btrfs_path *path) + struct btrfs_path *path, + u32 alignment) { struct free_space_extent extents[] = { {cache->key.objectid, cache->key.offset}, @@ -181,7 +176,8 @@ static int test_empty_block_group(struct btrfs_trans_handle *trans, static int test_remove_all(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, struct btrfs_block_group_cache *cache, - struct btrfs_path *path) + struct btrfs_path *path, + u32 alignment) { struct free_space_extent extents[] = {}; int ret; @@ -201,16 +197,17 @@ static int test_remove_all(struct btrfs_trans_handle *trans, static int test_remove_beginning(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, struct btrfs_block_group_cache *cache, - struct btrfs_path *path) + struct btrfs_path *path, + u32 alignment) { struct free_space_extent extents[] = { - {cache->key.objectid + BITMAP_RANGE, - cache->key.offset - BITMAP_RANGE}, + {cache->key.objectid + alignment, + cache->key.offset - alignment}, }; int ret; ret = __remove_from_free_space_tree(trans, fs_info, cache, path, - cache->key.objectid, BITMAP_RANGE); + cache->key.objectid, alignment); if (ret) { test_msg("Could not remove free space\n"); return ret; @@ -224,17 +221,18 @@ static int test_remove_beginning(struct btrfs_trans_handle *trans, static int test_remove_end(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, struct btrfs_block_group_cache *cache, - struct btrfs_path *path) + struct btrfs_path *path, + u32 alignment) { struct free_space_extent extents[] = { - {cache->key.objectid, cache->key.offset - BITMAP_RANGE}, + {cache->key.objectid, cache->key.offset - alignment}, }; int ret; ret = __remove_from_free_space_tree(trans, fs_info, cache, path, cache->key.objectid + - cache->key.offset - BITMAP_RANGE, - BITMAP_RANGE); + cache->key.offset - alignment, + alignment); if (ret) { test_msg("Could not remove free space\n"); return ret; @@ -247,18 +245,19 @@ static int test_remove_end(struct btrfs_trans_handle *trans, static int test_remove_middle(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, struct btrfs_block_group_cache *cache, - struct btrfs_path *path) + struct btrfs_path *path, + u32 alignment) { struct free_space_extent extents[] = { - {cache->key.objectid, BITMAP_RANGE}, - {cache->key.objectid + 2 * BITMAP_RANGE, - cache->key.offset - 2 * BITMAP_RANGE}, + {cache->key.objectid, alignment}, + {cache->key.objectid + 2 * alignment, + cache->key.offset - 2 * alignment}, }; int ret; ret = __remove_from_free_space_tree(trans, fs_info, cache, path, - cache->key.objectid + BITMAP_RANGE, - BITMAP_RANGE); + cache->key.objectid + alignment, + alignment); if (ret) { test_msg("Could not remove free space\n"); return ret; @@ -271,10 +270,11 @@ static int test_remove_middle(struct btrfs_trans_handle *trans, static int test_merge_left(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, struct btrfs_block_group_cache *cache, - struct btrfs_path *path) + struct btrfs_path *path, + u32 alignment) { struct free_space_extent extents[] = { - {cache->key.objectid, 2 * BITMAP_RANGE}, + {cache->key.objectid, 2 * alignment}, }; int ret; @@ -287,15 +287,15 @@ static int test_merge_left(struct btrfs_trans_handle *trans, } ret = __add_to_free_space_tree(trans, fs_info, cache, path, - cache->key.objectid, BITMAP_RANGE); + cache->key.objectid, alignment); if (ret) { test_msg("Could not add free space\n"); return ret; } ret = __add_to_free_space_tree(trans, fs_info, cache, path, - cache->key.objectid + BITMAP_RANGE, - BITMAP_RANGE); + cache->key.objectid + alignment, + alignment); if (ret) { test_msg("Could not add free space\n"); return ret; @@ -308,10 +308,11 @@ static int test_merge_left(struct btrfs_trans_handle *trans, static int test_merge_right(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, struct btrfs_block_group_cache *cache, - struct btrfs_path *path) + struct btrfs_path *path, + u32 alignment) { struct free_space_extent extents[] = { - {cache->key.objectid + BITMAP_RANGE, 2 * BITMAP_RANGE}, + {cache->key.objectid + alignment, 2 * alignment}, }; int ret; @@ -324,16 +325,16 @@ static int test_merge_right(struct btrfs_trans_handle *trans, } ret = __add_to_free_space_tree(trans, fs_info, cache, path, - cache->key.objectid + 2 * BITMAP_RANGE, - BITMAP_RANGE); + cache->key.objectid + 2 * alignment, + alignment); if (ret) { test_msg("Could not add free space\n"); return ret; } ret = __add_to_free_space_tree(trans, fs_info, cache, path, - cache->key.objectid + BITMAP_RANGE, - BITMAP_RANGE); + cache->key.objectid + alignment, + alignment); if (ret) { test_msg("Could not add free space\n"); return ret; @@ -346,10 +347,11 @@ static int test_merge_right(struct btrfs_trans_handle *trans, static int test_merge_both(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, struct btrfs_block_group_cache *cache, - struct btrfs_path *path) + struct btrfs_path *path, + u32 alignment) { struct free_space_extent extents[] = { - {cache->key.objectid, 3 * BITMAP_RANGE}, + {cache->key.objectid, 3 * alignment}, }; int ret; @@ -362,23 +364,23 @@ static int test_merge_both(struct btrfs_trans_handle *trans, } ret = __add_to_free_space_tree(trans, fs_info, cache, path, - cache->key.objectid, BITMAP_RANGE); + cache->key.objectid, alignment); if (ret) { test_msg("Could not add free space\n"); return ret; } ret = __add_to_free_space_tree(trans, fs_info, cache, path, - cache->key.objectid + 2 * BITMAP_RANGE, - BITMAP_RANGE); + cache->key.objectid + 2 * alignment, + alignment); if (ret) { test_msg("Could not add free space\n"); return ret; } ret = __add_to_free_space_tree(trans, fs_info, cache, path, - cache->key.objectid + BITMAP_RANGE, - BITMAP_RANGE); + cache->key.objectid + alignment, + alignment); if (ret) { test_msg("Could not add free space\n"); return ret; @@ -391,12 +393,13 @@ static int test_merge_both(struct btrfs_trans_handle *trans, static int test_merge_none(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, struct btrfs_block_group_cache *cache, - struct btrfs_path *path) + struct btrfs_path *path, + u32 alignment) { struct free_space_extent extents[] = { - {cache->key.objectid, BITMAP_RANGE}, - {cache->key.objectid + 2 * BITMAP_RANGE, BITMAP_RANGE}, - {cache->key.objectid + 4 * BITMAP_RANGE, BITMAP_RANGE}, + {cache->key.objectid, alignment}, + {cache->key.objectid + 2 * alignment, alignment}, + {cache->key.objectid + 4 * alignment, alignment}, }; int ret; @@ -409,23 +412,23 @@ static int test_merge_none(struct btrfs_trans_handle *trans, } ret = __add_to_free_space_tree(trans, fs_info, cache, path, - cache->key.objectid, BITMAP_RANGE); + cache->key.objectid, alignment); if (ret) { test_msg("Could not add free space\n"); return ret; } ret = __add_to_free_space_tree(trans, fs_info, cache, path, - cache->key.objectid + 4 * BITMAP_RANGE, - BITMAP_RANGE); + cache->key.objectid + 4 * alignment, + alignment); if (ret) { test_msg("Could not add free space\n"); return ret; } ret = __add_to_free_space_tree(trans, fs_info, cache, path, - cache->key.objectid + 2 * BITMAP_RANGE, - BITMAP_RANGE); + cache->key.objectid + 2 * alignment, + alignment); if (ret) { test_msg("Could not add free space\n"); return ret; @@ -438,10 +441,11 @@ static int test_merge_none(struct btrfs_trans_handle *trans, typedef int (*test_func_t)(struct btrfs_trans_handle *, struct btrfs_fs_info *, struct btrfs_block_group_cache *, - struct btrfs_path *); + struct btrfs_path *, + u32 alignment); -static int run_test(test_func_t test_func, int bitmaps, - u32 sectorsize, u32 nodesize) +static int run_test(test_func_t test_func, int bitmaps, u32 sectorsize, + u32 nodesize, u32 alignment) { struct btrfs_fs_info *fs_info; struct btrfs_root *root = NULL; @@ -480,7 +484,7 @@ static int run_test(test_func_t test_func, int bitmaps, btrfs_set_header_nritems(root->node, 0); root->alloc_bytenr += 2 * nodesize; - cache = btrfs_alloc_dummy_block_group(8 * BITMAP_RANGE, sectorsize); + cache = btrfs_alloc_dummy_block_group(8 * alignment, sectorsize); if (!cache) { test_msg("Couldn't allocate dummy block group cache\n"); ret = -ENOMEM; @@ -514,7 +518,7 @@ static int run_test(test_func_t test_func, int bitmaps, } } - ret = test_func(&trans, root->fs_info, cache, path); + ret = test_func(&trans, root->fs_info, cache, path, alignment); if (ret) goto out; @@ -539,15 +543,27 @@ out: return ret; } -static int run_test_both_formats(test_func_t test_func, - u32 sectorsize, u32 nodesize) +static int run_test_both_formats(test_func_t test_func, u32 sectorsize, + u32 nodesize, u32 alignment) { + int test_ret = 0; int ret; - ret = run_test(test_func, 0, sectorsize, nodesize); - if (ret) - return ret; - return run_test(test_func, 1, sectorsize, nodesize); + ret = run_test(test_func, 0, sectorsize, nodesize, alignment); + if (ret) { + test_msg("%pf failed with extents, sectorsize=%u, nodesize=%u, alignment=%u\n", + test_func, sectorsize, nodesize, alignment); + test_ret = ret; + } + + ret = run_test(test_func, 1, sectorsize, nodesize, alignment); + if (ret) { + test_msg("%pf failed with bitmaps, sectorsize=%u, nodesize=%u, alignment=%u\n", + test_func, sectorsize, nodesize, alignment); + test_ret = ret; + } + + return test_ret; } int btrfs_test_free_space_tree(u32 sectorsize, u32 nodesize) @@ -563,18 +579,30 @@ int btrfs_test_free_space_tree(u32 sectorsize, u32 nodesize) test_merge_both, test_merge_none, }; + u32 bitmap_alignment; + int test_ret = 0; int i; + /* + * Align some operations to a page to flush out bugs in the extent + * buffer bitmap handling of highmem. + */ + bitmap_alignment = BTRFS_FREE_SPACE_BITMAP_BITS * PAGE_SIZE; + test_msg("Running free space tree tests\n"); for (i = 0; i < ARRAY_SIZE(tests); i++) { - int ret = run_test_both_formats(tests[i], sectorsize, - nodesize); - if (ret) { - test_msg("%pf : sectorsize %u failed\n", - tests[i], sectorsize); - return ret; - } + int ret; + + ret = run_test_both_formats(tests[i], sectorsize, nodesize, + sectorsize); + if (ret) + test_ret = ret; + + ret = run_test_both_formats(tests[i], sectorsize, nodesize, + bitmap_alignment); + if (ret) + test_ret = ret; } - return 0; + return test_ret; } -- cgit From d2d9ac6aae1b743d729b2e4027d5666b2bc93003 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Fri, 23 Sep 2016 13:54:09 +0200 Subject: btrfs: tests: constify free space extent specs We don't change the given extent ranges, mark them const to catch accidental changes. Signed-off-by: David Sterba --- fs/btrfs/tests/free-space-tree-tests.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/fs/btrfs/tests/free-space-tree-tests.c b/fs/btrfs/tests/free-space-tree-tests.c index c449da4d77b9..061b4d5315c4 100644 --- a/fs/btrfs/tests/free-space-tree-tests.c +++ b/fs/btrfs/tests/free-space-tree-tests.c @@ -31,7 +31,7 @@ static int __check_free_space_extents(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, struct btrfs_block_group_cache *cache, struct btrfs_path *path, - struct free_space_extent *extents, + const struct free_space_extent * const extents, unsigned int num_extents) { struct btrfs_free_space_info *info; @@ -120,7 +120,7 @@ static int check_free_space_extents(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, struct btrfs_block_group_cache *cache, struct btrfs_path *path, - struct free_space_extent *extents, + const struct free_space_extent * const extents, unsigned int num_extents) { struct btrfs_free_space_info *info; @@ -165,7 +165,7 @@ static int test_empty_block_group(struct btrfs_trans_handle *trans, struct btrfs_path *path, u32 alignment) { - struct free_space_extent extents[] = { + const struct free_space_extent extents[] = { {cache->key.objectid, cache->key.offset}, }; @@ -179,7 +179,7 @@ static int test_remove_all(struct btrfs_trans_handle *trans, struct btrfs_path *path, u32 alignment) { - struct free_space_extent extents[] = {}; + const struct free_space_extent extents[] = {}; int ret; ret = __remove_from_free_space_tree(trans, fs_info, cache, path, @@ -200,7 +200,7 @@ static int test_remove_beginning(struct btrfs_trans_handle *trans, struct btrfs_path *path, u32 alignment) { - struct free_space_extent extents[] = { + const struct free_space_extent extents[] = { {cache->key.objectid + alignment, cache->key.offset - alignment}, }; @@ -224,7 +224,7 @@ static int test_remove_end(struct btrfs_trans_handle *trans, struct btrfs_path *path, u32 alignment) { - struct free_space_extent extents[] = { + const struct free_space_extent extents[] = { {cache->key.objectid, cache->key.offset - alignment}, }; int ret; @@ -248,7 +248,7 @@ static int test_remove_middle(struct btrfs_trans_handle *trans, struct btrfs_path *path, u32 alignment) { - struct free_space_extent extents[] = { + const struct free_space_extent extents[] = { {cache->key.objectid, alignment}, {cache->key.objectid + 2 * alignment, cache->key.offset - 2 * alignment}, @@ -273,7 +273,7 @@ static int test_merge_left(struct btrfs_trans_handle *trans, struct btrfs_path *path, u32 alignment) { - struct free_space_extent extents[] = { + const struct free_space_extent extents[] = { {cache->key.objectid, 2 * alignment}, }; int ret; @@ -311,7 +311,7 @@ static int test_merge_right(struct btrfs_trans_handle *trans, struct btrfs_path *path, u32 alignment) { - struct free_space_extent extents[] = { + const struct free_space_extent extents[] = { {cache->key.objectid + alignment, 2 * alignment}, }; int ret; @@ -350,7 +350,7 @@ static int test_merge_both(struct btrfs_trans_handle *trans, struct btrfs_path *path, u32 alignment) { - struct free_space_extent extents[] = { + const struct free_space_extent extents[] = { {cache->key.objectid, 3 * alignment}, }; int ret; @@ -396,7 +396,7 @@ static int test_merge_none(struct btrfs_trans_handle *trans, struct btrfs_path *path, u32 alignment) { - struct free_space_extent extents[] = { + const struct free_space_extent extents[] = { {cache->key.objectid, alignment}, {cache->key.objectid + 2 * alignment, alignment}, {cache->key.objectid + 4 * alignment, alignment}, -- cgit From 0e6757859efea6ed919fc37e4ee468634220b2d2 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Fri, 23 Sep 2016 13:57:06 +0200 Subject: btrfs: tests: uninline member definitions in free_space_extent The recommended way is to put all members on separate lines. Signed-off-by: David Sterba --- fs/btrfs/tests/free-space-tree-tests.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/btrfs/tests/free-space-tree-tests.c b/fs/btrfs/tests/free-space-tree-tests.c index 061b4d5315c4..6e144048a72e 100644 --- a/fs/btrfs/tests/free-space-tree-tests.c +++ b/fs/btrfs/tests/free-space-tree-tests.c @@ -24,7 +24,8 @@ #include "../transaction.h" struct free_space_extent { - u64 start, length; + u64 start; + u64 length; }; static int __check_free_space_extents(struct btrfs_trans_handle *trans, -- cgit