From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH v2] ring: guarantee ordering of cons/prod loading when doing Date: Tue, 7 Nov 2017 10:06:57 +0530 Message-ID: <20171107043655.GA3244@jerin> References: <1509612210-5499-1-git-send-email-hejianet@gmail.com> <20171102172337.GB1478@jerin> <25192429-8369-ac3d-44b0-c1b1d7182ef0@gmail.com> <20171103125616.GB20326@jerin> <7b7f3677-8313-9a2f-868f-b3a6231548d6@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org, olivier.matz@6wind.com, konstantin.ananyev@intel.com, bruce.richardson@intel.com, jianbo.liu@arm.com, hemant.agrawal@nxp.com, jie2.liu@hxt-semitech.com, bing.zhao@hxt-semitech.com, jia.he@hxt-semitech.com To: Jia He Return-path: Received: from NAM01-BN3-obe.outbound.protection.outlook.com (mail-bn3nam01on0060.outbound.protection.outlook.com [104.47.33.60]) by dpdk.org (Postfix) with ESMTP id 2F7791B341 for ; Tue, 7 Nov 2017 05:37:18 +0100 (CET) Content-Disposition: inline In-Reply-To: <7b7f3677-8313-9a2f-868f-b3a6231548d6@gmail.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" -----Original Message----- > Date: Mon, 6 Nov 2017 15:25:12 +0800 > From: Jia He > To: Jerin Jacob > Cc: dev@dpdk.org, olivier.matz@6wind.com, konstantin.ananyev@intel.com, > bruce.richardson@intel.com, jianbo.liu@arm.com, hemant.agrawal@nxp.com, > jie2.liu@hxt-semitech.com, bing.zhao@hxt-semitech.com, > jia.he@hxt-semitech.com > Subject: Re: [PATCH v2] ring: guarantee ordering of cons/prod loading when > doing > User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 > Thunderbird/52.4.0 > > Hi Jerin > > > On 11/3/2017 8:56 PM, Jerin Jacob Wrote: > > -----Original Message----- > > > > [...] > > > g like that. > > > Ok, but how to distinguish following 2 options? > > No clearly understood this question. For arm64 case, you can add > > CONFIG_RTE_RING_USE_C11_MEM_MODEL=y in config/defconfig_arm64-armv8a-* > Sorry for my unclear expressions. > I mean there should be one additional config macro besides > CONFIG_RTE_RING_USE_C11_MEM_MODEL > for users to choose? > > i.e. >  - On X86:CONFIG_RTE_RING_USE_C11_MEM_MODEL=n > include rte_ring_generic.h, no changes > - On arm64，CONFIG_RTE_RING_USE_C11_MEM_MODEL=y > include rte_ring_c11_mem.h by default. > In rte_ring_c11_mem.h, implement new version of > __rte_ring_move_prod_head/__rte_ring_move_cons_head/update_tail > > Then, how to distinguish the option of using rte_smp_rmb() or > __atomic_load/store_n()? On option could be to change the prototype of update_tail() and make compiler accommodate it for zero cost for arm64(Which I think, it it the case. But you can check the generated instructions) If not, move, __rte_ring_do_dequeue() and __rte_ring_do_enqueue() instead of __rte_ring_move_prod_head/__rte_ring_move_cons_head/update_tail() ➜ [master][dpdk.org] $ git diff diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h index 5e9b3b7b4..b32648825 100644 --- a/lib/librte_ring/rte_ring.h +++ b/lib/librte_ring/rte_ring.h @@ -358,8 +358,12 @@ void rte_ring_dump(FILE *f, const struct rte_ring *r); static __rte_always_inline void update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val, - uint32_t single) + uint32_t single, const uint32_t enqueue) { + if (enqueue) + rte_smp_wmb(); + else + rte_smp_rmb(); /* * If there are other enqueues/dequeues in progress that * preceded us, * we need to wait for them to complete @@ -470,9 +474,8 @@ __rte_ring_do_enqueue(struct rte_ring *r, void * const *obj_table, goto end; ENQUEUE_PTRS(r, &r[1], prod_head, obj_table, n, void *); - rte_smp_wmb(); - update_tail(&r->prod, prod_head, prod_next, is_sp); + update_tail(&r->prod, prod_head, prod_next, is_sp, 1); end: if (free_space != NULL) *free_space = free_entries - n; @@ -575,9 +578,8 @@ __rte_ring_do_dequeue(struct rte_ring *r, void **obj_table, goto end; DEQUEUE_PTRS(r, &r[1], cons_head, obj_table, n, void *); - rte_smp_rmb(); - update_tail(&r->cons, cons_head, cons_next, is_sc); + update_tail(&r->cons, cons_head, cons_next, is_sc, 0); end: if (available != NULL) > > Thanks for the clarification. > > -- > Cheers, > Jia >