[dpdk-dev] [PATCH v8 2/3] devtools: prevent use of rte atomic APIs in future patches

Stephen Hemminger stephen at networkplumber.org
Thu Jul 16 23:45:52 CEST 2020


On Thu, 16 Jul 2020 16:59:11 +0000
Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com> wrote:

> <snip>
> 
> > >  
> > > > Subject: Re: [dpdk-dev] [PATCH v8 2/3] devtools: prevent use of rte
> > > > atomic APIs in future patches
> > > >
> > > > 16/07/2020 12:48, David Marchand:  
> > > > > On Thu, Jul 16, 2020 at 6:58 AM Phil Yang <phil.yang at arm.com> wrote:  
> > > > > >  check_forbidden_additions() { # <patch>
> > > > > >         res=0
> > > > > > +       c11_atomics_dir="lib/librte_distributor lib/librte_hash  
> > lib/librte_kni  
> > > > > > +                        lib/librte_lpm lib/librte_rcu lib/librte_ring
> > > > > > +                        lib/librte_stack lib/librte_vhost
> > > > > > +                        drivers/event/octeontx drivers/event/octeontx2
> > > > > > +                        drivers/event/opdl drivers/net/bnx2x drivers/net/hinic
> > > > > > +                        drivers/net/hns3 drivers/net/memif  
> > drivers/net/thunderx  
> > > > > > +                        drivers/net/virtio examples/l2fwd-event"  
> > > > >
> > > > > I prefer a form like:
> > > > >
> > > > > +    c11_atomics_dir=""
> > > > > +    c11_atomics_dir="$c11_atomics_dir drivers/event/octeontx"
> > > > > +    c11_atomics_dir="$c11_atomics_dir drivers/event/octeontx2"
> > > > > +    c11_atomics_dir="$c11_atomics_dir drivers/event/opdl"
> > > > > +    c11_atomics_dir="$c11_atomics_dir drivers/net/bnx2x"
> > > > > +    c11_atomics_dir="$c11_atomics_dir drivers/net/hinic"
> > > > > +    c11_atomics_dir="$c11_atomics_dir drivers/net/hns3"
> > > > > +    c11_atomics_dir="$c11_atomics_dir drivers/net/memif"
> > > > > +    c11_atomics_dir="$c11_atomics_dir drivers/net/thunderx"
> > > > > +    c11_atomics_dir="$c11_atomics_dir drivers/net/virtio"
> > > > > +    c11_atomics_dir="$c11_atomics_dir examples/l2fwd-event"
> > > > > +    c11_atomics_dir="$c11_atomics_dir lib/librte_distributor"
> > > > > +    c11_atomics_dir="$c11_atomics_dir lib/librte_hash"
> > > > > +    c11_atomics_dir="$c11_atomics_dir lib/librte_kni"
> > > > > +    c11_atomics_dir="$c11_atomics_dir lib/librte_lpm"
> > > > > +    c11_atomics_dir="$c11_atomics_dir lib/librte_rcu"
> > > > > +    c11_atomics_dir="$c11_atomics_dir lib/librte_ring"
> > > > > +    c11_atomics_dir="$c11_atomics_dir lib/librte_stack"
> > > > > +    c11_atomics_dir="$c11_atomics_dir lib/librte_vhost"
> > > > >
> > > > > Easier to read and update.  
> > > >
> > > > Why do we need this list at all?
> > > > Are we allowed to add new code with old atomics in other
> > > > directories?
> > > > How bad it is to have a warning on non-converted libs?  
> > >
> > > From my perspective, the pros of this list are :
> > > 1. Avoid introducing false warnings in non-converted modules. Otherwise,  
> > the maintainers have to wonder if that module is converted or not.
> > 
> > Don't we want to convert all libs?  
> The goal is to convert all the libs.
> 
> > If we are adding one more rte_atomic in a lib, we should ask the question
> > why not converting to C11, no?  
> Agree, I am fine with this approach. That will kind of distribute the conversion work as well. 
> 
> >   
> > > 2. Keep non-converted modules compatible. C11 atomic builtins cannot be  
> > used directly for rte_atomicXX_t variables.  
> > >
> > > The cons are :
> > > 1. The list needs updating every time we convert a module.  
> This list will go away once all the modules are converted.
> 
> > > 2. The script is not elegant as before.  
> > 
> >   
> 

Can't the conversion just be automated with something coccinelle?



More information about the dev mailing list