<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.
> >
> > >
> > > > 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?
I do not know what 'coccinelle' means, not much help on the internet as well.
I really have not thought about automation. But, working on these conversions,
I have realized that understanding of the synchronizations used is required
(basically, understanding the entire module) to ensure the correct barriers are
used. We could use the most strongest barrier and functionality would be fine,
but the performance would take a hit.
