[dpdk-dev] [PATCH v3 1/3] ring: read tail using atomic load

Justin He Justin.He at arm.com
Wed Sep 26 12:09:44 CEST 2018



> -----Original Message-----
> From: Gavin Hu (Arm Technology China)
> Sent: 2018年9月26日 17:29
> To: Gavin Hu (Arm Technology China) <Gavin.Hu at arm.com>; dev at dpdk.org
> Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com>; Steve Capper
> <Steve.Capper at arm.com>; Ola Liljedahl <Ola.Liljedahl at arm.com>;
> jerin.jacob at caviumnetworks.com; nd <nd at arm.com>; stable at dpdk.org; Justin
> He <Justin.He at arm.com>
> Subject: RE: [PATCH v3 1/3] ring: read tail using atomic load
>
> +Justin He
>
> > -----Original Message-----
> > From: Gavin Hu <gavin.hu at arm.com>
> > Sent: Monday, September 17, 2018 4:17 PM
> > To: dev at dpdk.org
> > Cc: Gavin Hu (Arm Technology China) <Gavin.Hu at arm.com>; Honnappa
> > Nagarahalli <Honnappa.Nagarahalli at arm.com>; Steve Capper
> > <Steve.Capper at arm.com>; Ola Liljedahl <Ola.Liljedahl at arm.com>;
> > jerin.jacob at caviumnetworks.com; nd <nd at arm.com>; stable at dpdk.org
> > Subject: [PATCH v3 1/3] ring: read tail using atomic load
> >
> > In update_tail, read ht->tail using __atomic_load.Although the
> > compiler currently seems to be doing the right thing even without
> > _atomic_load, we don't want to give the compiler freedom to optimise
> > what should be an atomic load, it should not be arbitarily moved around.
> >
> > Fixes: 39368ebfc6 ("ring: introduce C11 memory model barrier option")
> > Cc: stable at dpdk.org
> >
> > Signed-off-by: Gavin Hu <gavin.hu at arm.com>
> > Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com>
> > Reviewed-by: Steve Capper <steve.capper at arm.com>
> > Reviewed-by: Ola Liljedahl <Ola.Liljedahl at arm.com>
> > ---
> >  lib/librte_ring/rte_ring_c11_mem.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_ring/rte_ring_c11_mem.h
> > b/lib/librte_ring/rte_ring_c11_mem.h
> > index 94df3c4..234fea0 100644
> > --- a/lib/librte_ring/rte_ring_c11_mem.h
> > +++ b/lib/librte_ring/rte_ring_c11_mem.h
> > @@ -21,7 +21,8 @@ update_tail(struct rte_ring_headtail *ht, uint32_t
> > old_val, uint32_t new_val,
> >   * we need to wait for them to complete
> >   */
> >  if (!single)
> > -while (unlikely(ht->tail != old_val))
> > +while (unlikely(old_val != __atomic_load_n(&ht->tail,
> > +__ATOMIC_RELAXED)))

I still wonder why an atomic load is needed here?
Cheers,
Justin (Jia He)
> >  rte_pause();
> >
> >  __atomic_store_n(&ht->tail, new_val, __ATOMIC_RELEASE);
> > --
> > 2.7.4

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


More information about the dev mailing list