[API-NEXT,PATCHv2,2/3] linix-generic: _odp_pri account 64 bits

Message ID 1441975993-4687-3-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Sept. 11, 2015, 12:53 p.m.
_odp_pri returns uint64_t value but actually accounts only 32 bit.
In my case that lead to return the same value for printed packets
with additional handle bits.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 platform/linux-generic/include/odp/plat/strong_types.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Bill Fischofer Sept. 11, 2015, 10:50 p.m. | #1
In linux-generic most handles are in fact 32-bit values that are embedded
in pseudo-pointers that may be either 32 bit or 64 bit, depending on the
architecture.  _odp_pri() returns a uint64_t because the
odp_typename_to_u64() APIs are defined to return a uint64_t value.

Are you saying you are trying to use 64-bit values as handles for some new
ODP type?  I'd be careful about trying to make this change without careful
testing on both 32-bit and 64-bit systems since the internal
odp_buffer_bits_t type, for example, assumes that handles can convert to
32-bit tokens.

On Fri, Sep 11, 2015 at 7:53 AM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:

> _odp_pri returns uint64_t value but actually accounts only 32 bit.
> In my case that lead to return the same value for printed packets
> with additional handle bits.
>
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  platform/linux-generic/include/odp/plat/strong_types.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/platform/linux-generic/include/odp/plat/strong_types.h
> b/platform/linux-generic/include/odp/plat/strong_types.h
> index a53d763..843e2d2 100644
> --- a/platform/linux-generic/include/odp/plat/strong_types.h
> +++ b/platform/linux-generic/include/odp/plat/strong_types.h
> @@ -25,7 +25,7 @@
>  #endif
>
>  /** Internal macro to get value of an ODP handle */
> -#define _odp_typeval(handle) ((uint32_t)(uintptr_t)(handle))
> +#define _odp_typeval(handle) ((uint64_t)(uintptr_t)(handle))
>
>  /** Internal macro to get printable value of an ODP handle */
>  #define _odp_pri(handle) ((uint64_t)_odp_typeval(handle))
> --
> 1.9.1
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
Maxim Uvarov Sept. 14, 2015, 8:57 a.m. | #2
On 09/12/15 01:50, Bill Fischofer wrote:
> In linux-generic most handles are in fact 32-bit values that are 
> embedded in pseudo-pointers that may be either 32 bit or 64 bit, 
> depending on the architecture.  _odp_pri() returns a uint64_t because 
> the odp_typename_to_u64() APIs are defined to return a uint64_t value.
>
> Are you saying you are trying to use 64-bit values as handles for some 
> new ODP type?  I'd be careful about trying to make this change without 
> careful testing on both 32-bit and 64-bit systems since the internal 
> odp_buffer_bits_t type, for example, assumes that handles can convert 
> to 32-bit tokens.
>

Bill, I need that change for next 3/3 patch. There are lines:

+	pkt_ref = odp_packet_create_ref(pkt);
+	/* Handles should be different */
+	CU_ASSERT(pkt != pkt_ref);
+	/* Debug print also should have refcount bits */
+	CU_ASSERT(odp_packet_to_u64(pkt) !=
+		  odp_packet_to_u64(pkt_ref));


Without that change first asset passes and second fails. I.e. 
odp_packet_to_u64 returns same value for packet and ref but pkt != pkt_ref.
To fix that I changed pri to 64 so no pkt is something about 1000000 and 
pkt_ref is 1100000.

Of course I did not check all prints in everywhere but for my case it 
worked and validation script pass.

Other solution might be do _odp_pri64() and make odp_packet_to_u64() use 
_odp_pri64() instead of _odp_pri(). But I suspect in other places we
can also hide bits on printing.

Maxim.

> On Fri, Sep 11, 2015 at 7:53 AM, Maxim Uvarov <maxim.uvarov@linaro.org 
> <mailto:maxim.uvarov@linaro.org>> wrote:
>
>     _odp_pri returns uint64_t value but actually accounts only 32 bit.
>     In my case that lead to return the same value for printed packets
>     with additional handle bits.
>
>     Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org
>     <mailto:maxim.uvarov@linaro.org>>
>     ---
>      platform/linux-generic/include/odp/plat/strong_types.h | 2 +-
>      1 file changed, 1 insertion(+), 1 deletion(-)
>
>     diff --git
>     a/platform/linux-generic/include/odp/plat/strong_types.h
>     b/platform/linux-generic/include/odp/plat/strong_types.h
>     index a53d763..843e2d2 100644
>     --- a/platform/linux-generic/include/odp/plat/strong_types.h
>     +++ b/platform/linux-generic/include/odp/plat/strong_types.h
>     @@ -25,7 +25,7 @@
>      #endif
>
>      /** Internal macro to get value of an ODP handle */
>     -#define _odp_typeval(handle) ((uint32_t)(uintptr_t)(handle))
>     +#define _odp_typeval(handle) ((uint64_t)(uintptr_t)(handle))
>
>      /** Internal macro to get printable value of an ODP handle */
>      #define _odp_pri(handle) ((uint64_t)_odp_typeval(handle))
>     --
>     1.9.1
>
>     _______________________________________________
>     lng-odp mailing list
>     lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>     https://lists.linaro.org/mailman/listinfo/lng-odp
>
>

Patch

diff --git a/platform/linux-generic/include/odp/plat/strong_types.h b/platform/linux-generic/include/odp/plat/strong_types.h
index a53d763..843e2d2 100644
--- a/platform/linux-generic/include/odp/plat/strong_types.h
+++ b/platform/linux-generic/include/odp/plat/strong_types.h
@@ -25,7 +25,7 @@ 
 #endif
 
 /** Internal macro to get value of an ODP handle */
-#define _odp_typeval(handle) ((uint32_t)(uintptr_t)(handle))
+#define _odp_typeval(handle) ((uint64_t)(uintptr_t)(handle))
 
 /** Internal macro to get printable value of an ODP handle */
 #define _odp_pri(handle) ((uint64_t)_odp_typeval(handle))