I understand that. Thanks

Thanks
Zaibo


发件人: Bill Fischofer [mailto:[email protected]]
发送时间: 2016年1月25日 10:09
收件人: Xu Zaibo
抄送: Tangchaofei; Liuzhongzhu; chenlizhi; yangfajun; Huwei (Xavier); Cuiaiguo 
(Aiguo Cui, WN); LNG ODP Mailman List
主题: Re: 答复: [lng-odp] [PATCH] pool:odp_buffer_free:check for buffer status

ODP_ASSERT() will cause a runtime check and exception in debug builds and will 
compile out in performance builds. The intent is to find and remove these 
serious bugs during development and then not incur unnecessary overhead in 
production.  One thing you don't want to do is either silently ignore these 
sort of errors or allow them to slip by unnoticed causing strange behavior 
downstream.

On Sun, Jan 24, 2016 at 8:06 PM, Xu Zaibo 
<[email protected]<mailto:[email protected]>> wrote:

Hi,

Anyway, I think “odp_buffer_free” should check this,  and if duplicate free 
happens(all data plane products in our company should avoid this, I think.), 
the bugs will be very hard to figure out.

But I don’t know which is better between using “ODP_ASSERT” and “ if 
(odp_likely())”. Perhaps “ if (odp_likely())” will cost less CPU, Maybe. 
However, if duplicate free happens, “ODP_ASSERT” can

inform us at the happening time.

Thanks
Zaibo


发件人: Bill Fischofer 
[mailto:[email protected]<mailto:[email protected]>]
发送时间: 2016年1月25日 8:57
收件人: Xu Zaibo
抄送: Tangchaofei; Liuzhongzhu; chenlizhi; yangfajun; Huwei (Xavier); Cuiaiguo 
(Aiguo Cui, WN); LNG ODP Mailman List
主题: Re: [lng-odp] [PATCH] pool:odp_buffer_free:check for buffer status

This is a duplicate of patch http://patches.opendataplane.org/patch/3629/ that 
I submitted in November but somehow fell through the cracks.  Perhaps you'd 
like to review that series so that it can be merged?

Thanks.

Bill

On Fri, Jan 22, 2016 at 3:06 AM, Zaibo Xu 
<[email protected]<mailto:[email protected]>> wrote:
if a buffer or packet is freed repeatedly, it will be a disaster.
To avoid this, a checking for buffer status is done before free opertion,
and it will do nothing if buffer allocator is ODP_FREEBUF.

Signed-off-by: Zaibo Xu <[email protected]<mailto:[email protected]>>
---
 platform/linux-generic/odp_pool.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/platform/linux-generic/odp_pool.c 
b/platform/linux-generic/odp_pool.c
index 84d35bf..f26755d 100644
--- a/platform/linux-generic/odp_pool.c
+++ b/platform/linux-generic/odp_pool.c
@@ -562,10 +562,12 @@ void odp_buffer_free(odp_buffer_t buf)
        odp_buffer_hdr_t *buf_hdr = odp_buf_to_hdr(buf);
        pool_entry_t *pool = odp_buf_to_pool(buf_hdr);

-       if (odp_unlikely(pool->s.low_wm_assert))
-               ret_buf(&pool->s, buf_hdr);
-       else
-               ret_local_buf(&pool->s.local_cache[local_id], buf_hdr);
+       if (odp_likely(buf_hdr->allocator != ODP_FREEBUF)) {
+               if (odp_unlikely(pool->s.low_wm_assert))
+                       ret_buf(&pool->s, buf_hdr);
+               else
+                       ret_local_buf(&pool->s.local_cache[local_id], buf_hdr);
+       }
 }

 void odp_buffer_free_multi(const odp_buffer_t buf[], int len)
--
1.9.1

_______________________________________________
lng-odp mailing list
[email protected]<mailto:[email protected]>
https://lists.linaro.org/mailman/listinfo/lng-odp


_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to