Re: [PATCH v3 0/3] Release ethdev shared memory on port cleanup

[David Marchand <david.marchand@redhat.com>]

On Thu, Aug 31, 2023 at 6:14 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 21/08/2023 10:58, David Marchand:
> > This series was triggered after investigating why the
> > eal_flags_file_prefix_autotest unit test was failing in the case of
> > statically built binaries [1]).
> >
> > For now, I went with a simple (naive) approach and put all accesses to the
> > shared data under a single lock: ethdev maintainers, it is your turn to
> > shine and give me reasons why we should keep the locks the way they
> > were ;-).
>
> I think the reasons are:
> - we wanted to call rte_spinlock_init()
> - we didn't want to allocate an ethdev lock in EAL memory config

Hum, it is a choice of implementation, not a list of locking requirements.

As to why we would not want ethdev lock in EAL, I can understand the concern.
This could be enhanced with a new service provided by EAL to register
some space in the shared configuration for use by libraries.
However, seeing how the mempool and timer libraries already had one
lock stored there, I assumed it was ok to do the same.


> How eliminating a lock is making the last patch easier exactly?

Let's say that my goal is to cleanup resources once a DPDK application
exits (no hugepage files left).
Here, it is a first step in that direction, with releasing ethdev
shared mem data.
It is not a complete solution, as other device classes probably have
the same issues of leaving some shared data behind.


The ethdev shared memory hosts ethdev ports, and the ownership notion.
The current code implicitly relies on the assumption that the shared
memory will never go away.
So I had to revisit and protect places accessing this shared memory,
and having one shared lock for protecting access was necessary.
But doing so, the ownership lock would be nested in this global lock
while doing nothing more that the ethdev shared data lock.

I will enhance the patches description.


-- 
David Marchand