bitops: Fix big endian compilation
PATCH 1/3 bitops: Fix big endian compilation
Replaced bswap with _ variants. While it's a glibc extension, all of the popular libc implementations (glibc, uClibc, musl, BIONIC) seem to support it.
Added static inline to two functions to match little endian variants. This fixes a linking error experienced when compiling.
--- kernel-lib/bitops.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
kernel-lib/bitops.h +4 -4
@@ -178,9 +178,9 @@ static inline unsigned long find_next_zero_bit(const unsigned long *addr,static inline unsigned long ext2_swab (const unsigned long y) {#if BITS_PER_LONG ==64 -return (unsigned long )bswap64 ((u64 ) y); +return (unsigned long )bswap_64 ((u64 ) y);#elif BITS_PER_LONG ==32 -return (unsigned long )bswap32 ((u32 ) y); +return (unsigned long )bswap_32 ((u32 ) y);#else #error BITS_PER_LONG not defined#endif
@@ -218,14 +218,14 @@ static inline unsigned long _find_next_bit_le(const unsigned long *addr1,return min (start +__ffs (ext2_swab (tmp)), nbits); } -unsigned long find_next_zero_bit_le (const void *addr,unsigned long size, +static inline unsigned long find_next_zero_bit_le (const void *addr,unsigned long size,unsigned long offset) {return _find_next_bit_le (addr,NULL , size, offset, ~0UL ); } -unsigned long find_next_bit_le (const void *addr,unsigned long size, +static inline unsigned long find_next_bit_le (const void *addr,unsigned long size,unsigned long offset) {return _find_next_bit_le (addr,NULL , size, offset,0UL );
Signature · 1 line
2.19.1
PATCH 2/3 task-utils: Fix comparison between pointer and integer
pthread_t is a pointer type, not an integer one. The > 0 makes no sense and throws a warning.
--- task-utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
task-utils.c +1 -1
@@ -67,7 +67,7 @@ void task_stop(struct task_info *info)if (!info)return ; -if (info->id >0 ) { +if (info->id) {pthread_cancel (info->id);pthread_join (info->id,NULL ); info->id =0 ;
Signature · 1 line
2.19.1
Re: [PATCH 2/3] task-utils: Fix comparison between pointer and integer
Quoted 2 lines · Rosen Penev
pthread_tis a pointer type, not an integer one. The > 0 makes no sense and throws a warning.
Code-wise the patch is ok, however, technically pthread_t is an opaque type. I guess David could fix it on the way in so no need to resend.
Quoted 20 lines
Signed-off-by Rosen Penev <[email protected]>--- task-utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)task-utils.c +1
-1@@ -67,7 +67,7 @@ void task_stop(struct task_info *info)if (!info)return ; -if (info->id >0 ) { +if (info->id) {pthread_cancel (info->id);pthread_join (info->id,NULL ); info->id =0 ;
PATCH 3/3 treewide: Fix missing declarations
In reply to Rosen PenevFound using -Wmissing-prototypes in GCC.
This should improve LTO behavior.
Note that set_free_space_tree_thresholds is an unused function. Adding inline seems to remove the unused function warning.
--- btrfs.c | 2 +- check/mode-lowmem.c | 2 +- extent-tree.c | 2 +- free-space-tree.c | 12 ++++++------ libbtrfsutil/stubs.c | 1 + utils-lib.c | 2 ++ utils.h | 1 + 7 files changed, 13 insertions(+), 9 deletions(-)
btrfs.c +1 -1
@@ -210,7 +210,7 @@ static int handle_global_options(int argc, char **argv)return shift; } -void handle_special_globals (int shift,int argc,char **argv) +static void handle_special_globals (int shift,int argc,char **argv) {int has_help =0 ;int has_full =0 ;
check/mode-lowmem.c +1 -1
@@ -953,7 +953,7 @@ out:* returns 0 means success. * returns not 0 means on error; */ -int repair_ternary_lowmem (struct btrfs_root *root,u64 dir_ino,u64 ino, +static int repair_ternary_lowmem (struct btrfs_root *root,u64 dir_ino,u64 ino,u64 index,char *name,int name_len,u8 filetype,int err) {
extent-tree.c +1 -1
@@ -3749,7 +3749,7 @@ static void __get_extent_size(struct btrfs_root *root, struct btrfs_path *path,* Return >0 for not found. * Return <0 for err */ -int btrfs_search_overlap_extent (struct btrfs_root *root, +static int btrfs_search_overlap_extent (struct btrfs_root *root,struct btrfs_path *path,u64 bytenr,u64 len) {struct btrfs_key key;
free-space-tree.c +6 -6
@@ -24,7 +24,7 @@#include "bitops.h" #include "internal.h" -void set_free_space_tree_thresholds (struct btrfs_block_group_cache *cache, +static inline void set_free_space_tree_thresholds (struct btrfs_block_group_cache *cache,u64 sectorsize) {u32 bitmap_range;
@@ -202,7 +202,7 @@ static void le_bitmap_set(unsigned long *map, unsigned int start, int len) } } -int convert_free_space_to_bitmaps (struct btrfs_trans_handle *trans, +static int convert_free_space_to_bitmaps (struct btrfs_trans_handle *trans,struct btrfs_block_group_cache *block_group,struct btrfs_path *path) {
@@ -341,7 +341,7 @@ out:return ret; } -int convert_free_space_to_extents (struct btrfs_trans_handle *trans, +static int convert_free_space_to_extents (struct btrfs_trans_handle *trans,struct btrfs_block_group_cache *block_group,struct btrfs_path *path) {
@@ -780,7 +780,7 @@ out:return ret; } -int __remove_from_free_space_tree (struct btrfs_trans_handle *trans, +static int __remove_from_free_space_tree (struct btrfs_trans_handle *trans,struct btrfs_block_group_cache *block_group,struct btrfs_path *path,u64 start,u64 size) {
@@ -960,7 +960,7 @@ out:return ret; } -int __add_to_free_space_tree (struct btrfs_trans_handle *trans, +static int __add_to_free_space_tree (struct btrfs_trans_handle *trans,struct btrfs_block_group_cache *block_group,struct btrfs_path *path,u64 start,u64 size) {
@@ -1420,7 +1420,7 @@ out:return ret; } -struct btrfs_root *btrfs_create_tree (struct btrfs_trans_handle *trans, +static struct btrfs_root *btrfs_create_tree (struct btrfs_trans_handle *trans,struct btrfs_fs_info *fs_info,u64 objectid) {
libbtrfsutil/stubs.c +1 -0
@@ -19,6 +19,7 @@#include <stdlib.h> #include <errno.h> +#include "stubs.h" void *reallocarray (void *ptr,size_t nmemb,size_t size) {
utils-lib.c +2 -0
@@ -5,6 +5,8 @@#include <sys/ioctl.h> #include <ioctl.h> +#include "utils.h" +#if BTRFS_FLAT_INCLUDES #include "ctree.h" #else
utils.h +1 -0
@@ -29,6 +29,7 @@#include "sizes.h" #include "messages.h" #include "ioctl.h" +#include "fsfeatures.h" #define BTRFS_SCAN_MOUNTED (1ULL << 0)#define BTRFS_SCAN_LBLKID (1ULL << 1)
Signature · 1 line
2.19.1
Re: [PATCH 3/3] treewide: Fix missing declarations
Quoted 8 lines · Rosen Penev
Found using -Wmissing-prototypes in GCC.
This should improve LTO behavior.
Note that
set_free_space_tree_thresholdsis an unused function. Adding inline seems to remove the unused function warning.Signed-off-by Rosen Penev <[email protected]>
I had a series that did exactly this but since you came in first:
Quoted 145 lines
--- btrfs.c | 2 +- check/mode-lowmem.c | 2 +- extent-tree.c | 2 +- free-space-tree.c | 12 ++++++------ libbtrfsutil/stubs.c | 1 + utils-lib.c | 2 ++ utils.h | 1 + 7 files changed, 13 insertions(+), 9 deletions(-)btrfs.c +1
-1@@ -210,7 +210,7 @@ static int handle_global_options(int argc, char **argv)return shift; } -void handle_special_globals (int shift,int argc,char **argv) +static void handle_special_globals (int shift,int argc,char **argv) {int has_help =0 ;int has_full =0 ;check/mode-lowmem.c +1
-1@@ -953,7 +953,7 @@ out:* returns 0 means success. * returns not 0 means on error; */ -int repair_ternary_lowmem (struct btrfs_root *root,u64 dir_ino,u64 ino, +static int repair_ternary_lowmem (struct btrfs_root *root,u64 dir_ino,u64 ino,u64 index,char *name,int name_len,u8 filetype,int err) {extent-tree.c +1
-1@@ -3749,7 +3749,7 @@ static void __get_extent_size(struct btrfs_root *root, struct btrfs_path *path,* Return >0 for not found. * Return <0 for err */ -int btrfs_search_overlap_extent (struct btrfs_root *root, +static int btrfs_search_overlap_extent (struct btrfs_root *root,struct btrfs_path *path,u64 bytenr,u64 len) {struct btrfs_key key;free-space-tree.c +6
-6@@ -24,7 +24,7 @@#include "bitops.h" #include "internal.h" -void set_free_space_tree_thresholds (struct btrfs_block_group_cache *cache, +static inline void set_free_space_tree_thresholds (struct btrfs_block_group_cache *cache,u64 sectorsize) {u32 bitmap_range;@@ -202,7 +202,7 @@ static void le_bitmap_set(unsigned long *map, unsigned int start, int len) } } -int convert_free_space_to_bitmaps (struct btrfs_trans_handle *trans, +static int convert_free_space_to_bitmaps (struct btrfs_trans_handle *trans,struct btrfs_block_group_cache *block_group,struct btrfs_path *path) {@@ -341,7 +341,7 @@ out:return ret; } -int convert_free_space_to_extents (struct btrfs_trans_handle *trans, +static int convert_free_space_to_extents (struct btrfs_trans_handle *trans,struct btrfs_block_group_cache *block_group,struct btrfs_path *path) {@@ -780,7 +780,7 @@ out:return ret; } -int __remove_from_free_space_tree (struct btrfs_trans_handle *trans, +static int __remove_from_free_space_tree (struct btrfs_trans_handle *trans,struct btrfs_block_group_cache *block_group,struct btrfs_path *path,u64 start,u64 size) {@@ -960,7 +960,7 @@ out:return ret; } -int __add_to_free_space_tree (struct btrfs_trans_handle *trans, +static int __add_to_free_space_tree (struct btrfs_trans_handle *trans,struct btrfs_block_group_cache *block_group,struct btrfs_path *path,u64 start,u64 size) {@@ -1420,7 +1420,7 @@ out:return ret; } -struct btrfs_root *btrfs_create_tree (struct btrfs_trans_handle *trans, +static struct btrfs_root *btrfs_create_tree (struct btrfs_trans_handle *trans,struct btrfs_fs_info *fs_info,u64 objectid) {libbtrfsutil/stubs.c +1
-0@@ -19,6 +19,7 @@#include <stdlib.h> #include <errno.h> +#include "stubs.h" void *reallocarray (void *ptr,size_t nmemb,size_t size) {utils-lib.c +2
-0@@ -5,6 +5,8 @@#include <sys/ioctl.h> #include <ioctl.h> +#include "utils.h" +#if BTRFS_FLAT_INCLUDES #include "ctree.h" #else utils.h +1
-0@@ -29,6 +29,7 @@#include "sizes.h" #include "messages.h" #include "ioctl.h" +#include "fsfeatures.h" #define BTRFS_SCAN_MOUNTED (1ULL << 0)#define BTRFS_SCAN_LBLKID (1ULL << 1)
Quoted 6 lines · Rosen Penev
Replaced bswap with _ variants. While it's a glibc extension, all of the popular libc implementations (glibc, uClibc, musl, BIONIC) seem to support it.
Added static inline to two functions to match little endian variants. This fixes a linking error experienced when compiling.
On what platform did you experience the linking error?
Quoted 40 lines
Signed-off-by Rosen Penev <[email protected]>--- kernel-lib/bitops.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)kernel-lib/bitops.h +4
-4@@ -178,9 +178,9 @@ static inline unsigned long find_next_zero_bit(const unsigned long *addr,static inline unsigned long ext2_swab (const unsigned long y) {#if BITS_PER_LONG ==64 -return (unsigned long )bswap64 ((u64 ) y); +return (unsigned long )bswap_64 ((u64 ) y);#elif BITS_PER_LONG ==32 -return (unsigned long )bswap32 ((u32 ) y); +return (unsigned long )bswap_32 ((u32 ) y);#else #error BITS_PER_LONG not defined#endif @@ -218,14 +218,14 @@ static inline unsigned long _find_next_bit_le(const unsigned long *addr1,return min (start +__ffs (ext2_swab (tmp)), nbits); } -unsigned long find_next_zero_bit_le (const void *addr,unsigned long size, +static inline unsigned long find_next_zero_bit_le (const void *addr,unsigned long size,unsigned long offset) {return _find_next_bit_le (addr,NULL , size, offset, ~0UL ); } -unsigned long find_next_bit_le (const void *addr,unsigned long size, +static inline unsigned long find_next_bit_le (const void *addr,unsigned long size,unsigned long offset) {return _find_next_bit_le (addr,NULL , size, offset,0UL );
Quoted 4 lines · Nikolay Borisov
On 5.11.18 г. 21:06 ч., Rosen Penev wrote:
Quoted 8 lines
Quoted 6 lines
Replaced bswap with _ variants. While it's a glibc extension, all of the popular libc implementations (glibc, uClibc, musl, BIONIC) seem to support it.
Added static inline to two functions to match little endian variants. This fixes a linking error experienced when compiling.
On what platform did you experience the linking error?
MIPS 24kc. OpenWrt specifically. Here's a link with the compile log (near the end):
https://circleci.com/gh/openwrt/packages/77?utm_campaign=workflow-failed&utm_medium=email&utm_source=notification
The LTO errors I believe are due to GCC 7.3.0 being broken (fixed in 7.3.1).
This patch fixes both issues. I'm still unsure why bswap32 is not being included but the _ variant works.
Quoted 41 lines
Quoted 41 lines
Signed-off-by Rosen Penev <[email protected]>--- kernel-lib/bitops.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)kernel-lib/bitops.h +4
-4@@ -178,9 +178,9 @@ static inline unsigned long find_next_zero_bit(const unsigned long *addr,static inline unsigned long ext2_swab (const unsigned long y) {#if BITS_PER_LONG ==64 -return (unsigned long )bswap64 ((u64 ) y); +return (unsigned long )bswap_64 ((u64 ) y);#elif BITS_PER_LONG ==32 -return (unsigned long )bswap32 ((u32 ) y); +return (unsigned long )bswap_32 ((u32 ) y);#else #error BITS_PER_LONG not defined#endif @@ -218,14 +218,14 @@ static inline unsigned long _find_next_bit_le(const unsigned long *addr1,return min (start +__ffs (ext2_swab (tmp)), nbits); } -unsigned long find_next_zero_bit_le (const void *addr,unsigned long size, +static inline unsigned long find_next_zero_bit_le (const void *addr,unsigned long size,unsigned long offset) {return _find_next_bit_le (addr,NULL , size, offset, ~0UL ); } -unsigned long find_next_bit_le (const void *addr,unsigned long size, +static inline unsigned long find_next_bit_le (const void *addr,unsigned long size,unsigned long offset) {return _find_next_bit_le (addr,NULL , size, offset,0UL );
Nikolay Borisov
Reviewed-byQuoted 13 lines · Rosen Penev
Quoted 4 lines · Nikolay Borisov
On 5.11.18 г. 21:06 ч., Rosen Penev wrote:
Quoted 8 lines
Quoted 6 lines
Replaced bswap with _ variants. While it's a glibc extension, all of the popular libc implementations (glibc, uClibc, musl, BIONIC) seem to support it.
Added static inline to two functions to match little endian variants. This fixes a linking error experienced when compiling.
On what platform did you experience the linking error?
Quoted 11 lines
MIPS 24kc. OpenWrt specifically. Here's a link with the compile log (near the end):
https://circleci.com/gh/openwrt/packages/77?utm_campaign=workflow-failed&utm_medium=email&utm_source=notificationThe LTO errors I believe are due to GCC 7.3.0 being broken (fixed in 7.3.1).
This patch fixes both issues. I'm still unsure why bswap32 is not being included but the _ variant works.
Quoted 3 lines
Quoted 3 lines
Signed-off-by Rosen Penev <[email protected]>
(Some explanation below but the patch is ok)
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
--- kernel-lib/bitops.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Quoted 13 lines
Quoted 5 lines
Quoted 5 lines
kernel-lib/bitops.h +0
-0@@ -178,9 +178,9 @@ static inline unsigned long find_next_zero_bit(const unsigned long *addr,Quoted 8 lines
kernel-lib/bitops.h static inline unsigned long find_next_zero_bit(const unsigned long *addr, · line 183
Quoted 8 lines
kernel-lib/bitops.h +2
-2static inline unsigned long ext2_swab (const unsigned long y) {#if BITS_PER_LONG ==64 -return (unsigned long )bswap64 ((u64 ) y); +return (unsigned long )bswap_64 ((u64 ) y);#elif BITS_PER_LONG ==32 -return (unsigned long )bswap32 ((u32 ) y); +return (unsigned long )bswap_32 ((u32 ) y);
Alternatively those bswaps* could be replaced by __builtin_bswap32/64 which come directly from the compiler.
Looking at: https://git.musl-libc.org/cgit/musl/tree/include/byteswap.h
It seems musl only defines bswap_* variants. glibc also seems to define only _ variants as per:
https://sourceware.org/git/?p=glibc.git;a=blob;f=string/byteswap.h;h=a45b3e20ed5d0849bc3939b80272bd3d6d43dc31;hb=HEAD
And indeed http://man7.org/linux/man-pages/man3/bswap.3.html documents the functions as having _ suffix so this was a mistake on my part.
Your solution is correct (and now I have no explanation where did bswap64 definition come from so that it didn't fail for me).
Quoted 22 lines
Quoted 21 lines
Quoted 21 lines
kernel-lib/bitops.h +2
-2#else #error BITS_PER_LONG not defined#endif @@ -218,14 +218,14 @@ static inline unsigned long _find_next_bit_le(const unsigned long *addr1,return min (start +__ffs (ext2_swab (tmp)), nbits); } -unsigned long find_next_zero_bit_le (const void *addr,unsigned long size, +static inline unsigned long find_next_zero_bit_le (const void *addr,unsigned long size,unsigned long offset) {return _find_next_bit_le (addr,NULL , size, offset, ~0UL ); } -unsigned long find_next_bit_le (const void *addr,unsigned long size, +static inline unsigned long find_next_bit_le (const void *addr,unsigned long size,unsigned long offset) {return _find_next_bit_le (addr,NULL , size, offset,0UL );
Quoted 8 lines · Rosen Penev
Replaced bswap with _ variants. While it's a glibc extension, all of the popular libc implementations (glibc, uClibc, musl, BIONIC) seem to support it.
Added static inline to two functions to match little endian variants. This fixes a linking error experienced when compiling.
Signed-off-by Rosen Penev <[email protected]>
1-3 applied, thanks.