While testing the new macros for working with 48 bit containers,
I faced a weird problem:
32 + 16: 0x2ef6e8da 0x79e60000
48: 0xffffe8da + 0x79e60000
All the bits starting from the 32nd were getting 1d in 9/10 cases.
The debug showed:
p[0]: 0x00002e0000000000
p[1]: 0x00002ef600000000
p[2]: 0xffffffffe8000000
p[3]: 0xffffffffe8da0000
p[4]: 0xffffffffe8da7900
p[5]: 0xffffffffe8da79e6
that the value becomes a garbage after the third OR, i.e. on
`p[2] << 24`.
When the 31st bit is 1 and there's no explicit cast to an unsigned,
it's being considered as a signed int and getting sign-extended on
OR, so `e8000000` becomes `ffffffffe8000000` and messes up the
result.
Cast the @p[2] to u64 as well to avoid this. Now:
32 + 16: 0x7ef6a490 0xddc10000
48: 0x7ef6a490 + 0xddc10000
p[0]: 0x00007e0000000000
p[1]: 0x00007ef600000000
p[2]: 0x00007ef6a4000000
p[3]: 0x00007ef6a4900000
p[4]: 0x00007ef6a490dd00
p[5]: 0x00007ef6a490ddc1
Fixes: c2ea5fcf53d5 ("asm-generic: introduce be48 unaligned accessors")
Signed-off-by: Alexander Lobakin <[email protected]>
---
include/asm-generic/unaligned.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h
index 8fc637379899..df30f11b4a46 100644
--- a/include/asm-generic/unaligned.h
+++ b/include/asm-generic/unaligned.h
@@ -143,7 +143,7 @@ static inline void put_unaligned_be48(const u64 val, void *p)
static inline u64 __get_unaligned_be48(const u8 *p)
{
- return (u64)p[0] << 40 | (u64)p[1] << 32 | p[2] << 24 |
+ return (u64)p[0] << 40 | (u64)p[1] << 32 | (u64)p[2] << 24 |
p[3] << 16 | p[4] << 8 | p[5];
}
--
2.35.1
From: Alexander Lobakin <[email protected]>
Date: Wed, 06 Apr 2022 23:46:04 +0000
> While testing the new macros for working with 48 bit containers,
> I faced a weird problem:
>
> 32 + 16: 0x2ef6e8da 0x79e60000
> 48: 0xffffe8da + 0x79e60000
>
> All the bits starting from the 32nd were getting 1d in 9/10 cases.
> The debug showed:
>
> p[0]: 0x00002e0000000000
> p[1]: 0x00002ef600000000
> p[2]: 0xffffffffe8000000
> p[3]: 0xffffffffe8da0000
> p[4]: 0xffffffffe8da7900
> p[5]: 0xffffffffe8da79e6
>
> that the value becomes a garbage after the third OR, i.e. on
> `p[2] << 24`.
> When the 31st bit is 1 and there's no explicit cast to an unsigned,
> it's being considered as a signed int and getting sign-extended on
> OR, so `e8000000` becomes `ffffffffe8000000` and messes up the
> result.
> Cast the @p[2] to u64 as well to avoid this. Now:
>
> 32 + 16: 0x7ef6a490 0xddc10000
> 48: 0x7ef6a490 + 0xddc10000
>
> p[0]: 0x00007e0000000000
> p[1]: 0x00007ef600000000
> p[2]: 0x00007ef6a4000000
> p[3]: 0x00007ef6a4900000
> p[4]: 0x00007ef6a490dd00
> p[5]: 0x00007ef6a490ddc1
>
> Fixes: c2ea5fcf53d5 ("asm-generic: introduce be48 unaligned accessors")
> Signed-off-by: Alexander Lobakin <[email protected]>
Uhm, ping?
> ---
> include/asm-generic/unaligned.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unalign=
> ed.h
> index 8fc637379899..df30f11b4a46 100644
> --- a/include/asm-generic/unaligned.h
> +++ b/include/asm-generic/unaligned.h
> @@ -143,7 +143,7 @@ static inline void put_unaligned_be48(const u64 val, v=
> oid *p)
>
> static inline u64 __get_unaligned_be48(const u8 *p)
> {
> - return (u64)p[0] << 40 | (u64)p[1] << 32 | p[2] << 24 |
> + return (u64)p[0] << 40 | (u64)p[1] << 32 | (u64)p[2] << 24 |
> p[3] << 16 | p[4] << 8 | p[5];
> }
>
> --
> 2.35.1
Thanks,
Al
On 4/11/22 13:00, Alexander Lobakin wrote:
> Uhm, ping?
What happened with the plan to move this function into the block layer?
I'm asking this because if that function is moved your patch will conflict
with the patch that moves that function.
Thanks,
Bart.
On Mon, Apr 11, 2022 at 02:20:47PM -0700, Bart Van Assche wrote:
> On 4/11/22 13:00, Alexander Lobakin wrote:
> > Uhm, ping?
>
> What happened with the plan to move this function into the block layer?
> I'm asking this because if that function is moved your patch will conflict
> with the patch that moves that function.
I believe you're thinking of lower_48_bits() instead of this unaligned
accessor. It appears that patch hasn't been picked up yet though, and I am
assuming it should go through block.