All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] bitops: Fix big endian compilation
@ 2018-11-05 19:06 Rosen Penev
  2018-11-05 19:06 ` [PATCH 2/3] task-utils: Fix comparison between pointer and integer Rosen Penev
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Rosen Penev @ 2018-11-05 19:06 UTC (permalink / raw)
  To: linux-btrfs

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 <rosenp@gmail.com>
---
 kernel-lib/bitops.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel-lib/bitops.h b/kernel-lib/bitops.h
index b1fd6f5..2c51a26 100644
--- a/kernel-lib/bitops.h
+++ b/kernel-lib/bitops.h
@@ -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);
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] task-utils: Fix comparison between pointer and integer
  2018-11-05 19:06 [PATCH 1/3] bitops: Fix big endian compilation Rosen Penev
@ 2018-11-05 19:06 ` Rosen Penev
  2018-11-05 21:35   ` Nikolay Borisov
  2018-11-05 19:06 ` [PATCH 3/3] treewide: Fix missing declarations Rosen Penev
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Rosen Penev @ 2018-11-05 19:06 UTC (permalink / raw)
  To: linux-btrfs

pthread_t is a pointer type, not an integer one. The > 0 makes no sense
and throws a warning.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 task-utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/task-utils.c b/task-utils.c
index a9bee8f..e4dcd36 100644
--- a/task-utils.c
+++ b/task-utils.c
@@ -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;
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] treewide: Fix missing declarations
  2018-11-05 19:06 [PATCH 1/3] bitops: Fix big endian compilation Rosen Penev
  2018-11-05 19:06 ` [PATCH 2/3] task-utils: Fix comparison between pointer and integer Rosen Penev
@ 2018-11-05 19:06 ` Rosen Penev
  2018-11-05 21:29   ` Nikolay Borisov
  2018-11-05 21:31 ` [PATCH 1/3] bitops: Fix big endian compilation Nikolay Borisov
  2018-11-13 12:34 ` David Sterba
  3 siblings, 1 reply; 9+ messages in thread
From: Rosen Penev @ 2018-11-05 19:06 UTC (permalink / raw)
  To: linux-btrfs

Found 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.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 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(-)

diff --git a/btrfs.c b/btrfs.c
index 2d39f2c..78c468d 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -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;
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 14bbc9e..94123c1 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -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)
 {
diff --git a/extent-tree.c b/extent-tree.c
index cd98633..8c9cdef 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -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;
diff --git a/free-space-tree.c b/free-space-tree.c
index 6641cdf..6ef5792 100644
--- a/free-space-tree.c
+++ b/free-space-tree.c
@@ -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)
 {
diff --git a/libbtrfsutil/stubs.c b/libbtrfsutil/stubs.c
index 9b9e037..c530e40 100644
--- a/libbtrfsutil/stubs.c
+++ b/libbtrfsutil/stubs.c
@@ -19,6 +19,7 @@
 
 #include <stdlib.h>
 #include <errno.h>
+#include "stubs.h"
 
 void *reallocarray(void *ptr, size_t nmemb, size_t size)
 {
diff --git a/utils-lib.c b/utils-lib.c
index 044f93f..2ac421b 100644
--- a/utils-lib.c
+++ b/utils-lib.c
@@ -5,6 +5,8 @@
 #include <sys/ioctl.h>
 #include <ioctl.h>
 
+#include "utils.h"
+
 #if BTRFS_FLAT_INCLUDES
 #include "ctree.h"
 #else
diff --git a/utils.h b/utils.h
index b6c00cf..7c5eb79 100644
--- a/utils.h
+++ b/utils.h
@@ -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)
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] treewide: Fix missing declarations
  2018-11-05 19:06 ` [PATCH 3/3] treewide: Fix missing declarations Rosen Penev
@ 2018-11-05 21:29   ` Nikolay Borisov
  0 siblings, 0 replies; 9+ messages in thread
From: Nikolay Borisov @ 2018-11-05 21:29 UTC (permalink / raw)
  To: Rosen Penev, linux-btrfs



On 5.11.18 г. 21:06 ч., Rosen Penev wrote:
> Found 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.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>


I had a series that did exactly this but since you came in first:

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  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(-)
> 
> diff --git a/btrfs.c b/btrfs.c
> index 2d39f2c..78c468d 100644
> --- a/btrfs.c
> +++ b/btrfs.c
> @@ -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;
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 14bbc9e..94123c1 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -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)
>  {
> diff --git a/extent-tree.c b/extent-tree.c
> index cd98633..8c9cdef 100644
> --- a/extent-tree.c
> +++ b/extent-tree.c
> @@ -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;
> diff --git a/free-space-tree.c b/free-space-tree.c
> index 6641cdf..6ef5792 100644
> --- a/free-space-tree.c
> +++ b/free-space-tree.c
> @@ -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)
>  {
> diff --git a/libbtrfsutil/stubs.c b/libbtrfsutil/stubs.c
> index 9b9e037..c530e40 100644
> --- a/libbtrfsutil/stubs.c
> +++ b/libbtrfsutil/stubs.c
> @@ -19,6 +19,7 @@
>  
>  #include <stdlib.h>
>  #include <errno.h>
> +#include "stubs.h"
>  
>  void *reallocarray(void *ptr, size_t nmemb, size_t size)
>  {
> diff --git a/utils-lib.c b/utils-lib.c
> index 044f93f..2ac421b 100644
> --- a/utils-lib.c
> +++ b/utils-lib.c
> @@ -5,6 +5,8 @@
>  #include <sys/ioctl.h>
>  #include <ioctl.h>
>  
> +#include "utils.h"
> +
>  #if BTRFS_FLAT_INCLUDES
>  #include "ctree.h"
>  #else
> diff --git a/utils.h b/utils.h
> index b6c00cf..7c5eb79 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -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)
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] bitops: Fix big endian compilation
  2018-11-05 19:06 [PATCH 1/3] bitops: Fix big endian compilation Rosen Penev
  2018-11-05 19:06 ` [PATCH 2/3] task-utils: Fix comparison between pointer and integer Rosen Penev
  2018-11-05 19:06 ` [PATCH 3/3] treewide: Fix missing declarations Rosen Penev
@ 2018-11-05 21:31 ` Nikolay Borisov
  2018-11-05 21:42   ` Rosen Penev
  2018-11-13 12:34 ` David Sterba
  3 siblings, 1 reply; 9+ messages in thread
From: Nikolay Borisov @ 2018-11-05 21:31 UTC (permalink / raw)
  To: Rosen Penev, linux-btrfs



On 5.11.18 г. 21:06 ч., Rosen Penev wrote:
> 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?

> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  kernel-lib/bitops.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel-lib/bitops.h b/kernel-lib/bitops.h
> index b1fd6f5..2c51a26 100644
> --- a/kernel-lib/bitops.h
> +++ b/kernel-lib/bitops.h
> @@ -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);
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] task-utils: Fix comparison between pointer and integer
  2018-11-05 19:06 ` [PATCH 2/3] task-utils: Fix comparison between pointer and integer Rosen Penev
@ 2018-11-05 21:35   ` Nikolay Borisov
  0 siblings, 0 replies; 9+ messages in thread
From: Nikolay Borisov @ 2018-11-05 21:35 UTC (permalink / raw)
  To: Rosen Penev, linux-btrfs



On 5.11.18 г. 21:06 ч., Rosen Penev wrote:
> pthread_t is 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.

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  task-utils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/task-utils.c b/task-utils.c
> index a9bee8f..e4dcd36 100644
> --- a/task-utils.c
> +++ b/task-utils.c
> @@ -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;
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] bitops: Fix big endian compilation
  2018-11-05 21:31 ` [PATCH 1/3] bitops: Fix big endian compilation Nikolay Borisov
@ 2018-11-05 21:42   ` Rosen Penev
  2018-11-06  6:27     ` Nikolay Borisov
  0 siblings, 1 reply; 9+ messages in thread
From: Rosen Penev @ 2018-11-05 21:42 UTC (permalink / raw)
  To: nborisov; +Cc: linux-btrfs

On Mon, Nov 5, 2018 at 1:31 PM Nikolay Borisov <nborisov@suse.com> wrote:
>
>
>
> On 5.11.18 г. 21:06 ч., Rosen Penev wrote:
> > 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.
>
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  kernel-lib/bitops.h | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel-lib/bitops.h b/kernel-lib/bitops.h
> > index b1fd6f5..2c51a26 100644
> > --- a/kernel-lib/bitops.h
> > +++ b/kernel-lib/bitops.h
> > @@ -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);
> >

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] bitops: Fix big endian compilation
  2018-11-05 21:42   ` Rosen Penev
@ 2018-11-06  6:27     ` Nikolay Borisov
  0 siblings, 0 replies; 9+ messages in thread
From: Nikolay Borisov @ 2018-11-06  6:27 UTC (permalink / raw)
  To: Rosen Penev; +Cc: linux-btrfs



On 5.11.18 г. 23:42 ч., Rosen Penev wrote:
> On Mon, Nov 5, 2018 at 1:31 PM Nikolay Borisov <nborisov@suse.com> wrote:
>>
>>
>>
>> On 5.11.18 г. 21:06 ч., Rosen Penev wrote:
>>> 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.
>>
>>>
>>> Signed-off-by: Rosen Penev <rosenp@gmail.com>

(Some explanation below but the patch is ok)

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

>>> ---
>>>  kernel-lib/bitops.h | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/kernel-lib/bitops.h b/kernel-lib/bitops.h
>>> index b1fd6f5..2c51a26 100644
>>> --- a/kernel-lib/bitops.h
>>> +++ b/kernel-lib/bitops.h
>>> @@ -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);

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).


>>>  #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);
>>>
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] bitops: Fix big endian compilation
  2018-11-05 19:06 [PATCH 1/3] bitops: Fix big endian compilation Rosen Penev
                   ` (2 preceding siblings ...)
  2018-11-05 21:31 ` [PATCH 1/3] bitops: Fix big endian compilation Nikolay Borisov
@ 2018-11-13 12:34 ` David Sterba
  3 siblings, 0 replies; 9+ messages in thread
From: David Sterba @ 2018-11-13 12:34 UTC (permalink / raw)
  To: Rosen Penev; +Cc: linux-btrfs

On Mon, Nov 05, 2018 at 11:06:41AM -0800, Rosen Penev wrote:
> 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 <rosenp@gmail.com>

1-3 applied, thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-11-13 12:34 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-05 19:06 [PATCH 1/3] bitops: Fix big endian compilation Rosen Penev
2018-11-05 19:06 ` [PATCH 2/3] task-utils: Fix comparison between pointer and integer Rosen Penev
2018-11-05 21:35   ` Nikolay Borisov
2018-11-05 19:06 ` [PATCH 3/3] treewide: Fix missing declarations Rosen Penev
2018-11-05 21:29   ` Nikolay Borisov
2018-11-05 21:31 ` [PATCH 1/3] bitops: Fix big endian compilation Nikolay Borisov
2018-11-05 21:42   ` Rosen Penev
2018-11-06  6:27     ` Nikolay Borisov
2018-11-13 12:34 ` David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.