Hello,
Thomas Monjalon <tho...@monjalon.net> writes:
> 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.
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.
2. The script is not elegant as before.
Thanks,
Phil
