Petri Savolainen(psavol) replied on github web page:

platform/linux-generic/odp_packet.c
line 51
@@ -1259,7 +1260,15 @@ int odp_packet_input_index(odp_packet_t pkt)
 
 void odp_packet_user_ptr_set(odp_packet_t pkt, const void *ptr)
 {
-       packet_hdr(pkt)->buf_hdr.user_ptr = ptr;
+       odp_packet_hdr_t *pkt_hdr = packet_hdr(pkt);
+
+       if (odp_unlikely(ptr == NULL)) {
+               pkt_hdr->p.flags.user_ptr_set = 0;
+               return;
+       }


Comment:
This makes the get side work consistently - code path and performance is the 
same for NULL values. The flag tells if NULL is returned, or pointer value 
needs to be read. Reading the pointer value may cause cache miss. The flag 
needs to be read in any case. If flag is zero, we can return NULL right away 
(no need to wait for cache to bring NULL value from memory).

     if (flags.user_ptr_set == 0)
         return NULL;

All NULL cases avoid going to the memory:
pkt = alloc()
user_ptr(); <-- NULL
user_ptr_set(pkt, &foo);
user_ptr(); <-- != NULL
user_ptr_set(pkt, NULL);
user_ptr(); <-- NULL
user_ptr(); <-- NULL
user_ptr(); <-- NULL
user_ptr(); <-- NULL


> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
> This "optimization" is unnecessary. If the user wants to set the user ptr to 
> `NULL` that's as legitimate a value as any and will be retrieved normally. 
> All we care about is whether `odp_packet_user_ptr_set()` has been called. The 
> flag is reset by `odp_packet_reset()`.


>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>> I like using this feature, but `_ODP_HIDE_FROM_DOXYGEN_` seems awkward here. 
>> Should we just use `_ODP_INTERNAL_` as a standard means of denoting internal 
>> items that should not appear in doxygen?


https://github.com/Linaro/odp/pull/392#discussion_r161976128
updated_at 2018-01-17 07:51:16

Reply via email to