Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> writes: <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.y...@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.
Will do in V9. Thanks, Phil > > > > > > 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. > > > > >