Re: [PATCH 1/3] bitops: Fix big endian compilation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 5.11.18 г. 23:42 ч., Rosen Penev wrote:
> On Mon, Nov 5, 2018 at 1:31 PM Nikolay Borisov <nborisov@xxxxxxxx> 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@xxxxxxxxx>

(Some explanation below but the patch is ok)

Reviewed-by: Nikolay Borisov <nborisov@xxxxxxxx>

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



[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux