[dpdk-dev] [PATCH v8 04/19] ethdev: introduce device lock

[Zhang, Qi Z]

Hi Yuanhan:

> -----Original Message-----
> From: Yuanhan Liu [mailto:yliu at fridaylinux.org]
> Sent: Wednesday, July 4, 2018 10:20 AM
> To: Zhang, Qi Z <qi.z.zhang at intel.com>; thomas at monjalon.net; Burakov,
> Anatoly <anatoly.burakov at intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev at intel.com>; dev at dpdk.org;
> Richardson, Bruce <bruce.richardson at intel.com>; Yigit, Ferruh
> <ferruh.yigit at intel.com>; Shelton, Benjamin H
> <benjamin.h.shelton at intel.com>; Vangati, Narender
> <narender.vangati at intel.com>
> Subject: Re: [dpdk-dev] [PATCH v8 04/19] ethdev: introduce device lock
> 
> 
> 
> On Mon, Jul 2, 2018, at 1:44 PM, Qi Zhang wrote:
> > Introduce API rte_eth_dev_lock and rte_eth_dev_unlock to let
> > application lock or unlock on specific ethdev, a locked device can't
> > be detached, this help applicaiton to prevent unexpected device
> > detaching, especially in multi-process envrionment.
> 
> I'm just wondering why the need of introducing such 2 (very specific) APIs,
> judging that it  looks like a ref count could solve this kind of issues perfectly.

Yes, if we separate the callback stuff, ref_count can solve the issue.
Actually this is my original implementation, but since we also need a callback for runtime check, 
so I just reuse it by registering a default callback.

But if we decide to replace lock_with_callback by adding a new PRE_DETACH event in rte_eth_dev_register_callback, 
We can take the ref_count way for simple device pin.

> 
> Also, the API name (_dev_lock/unlock) looks like an over killing, judging that
> you just want to pin a device (if I understand this commit log correctly).
> What firstly comes to my mind when I see "lock" was "no one can *access* a
> device after another has grabbed the lock", which doesn't quite match what
> you described here.

Yes, device is shared, any processes can access it, we are not going to introduce exclusive access.
But probably we should rename the API's name as rte_eth_dev_get/put? or rte_eth_dev_pin/unpin?

Thanks!
Qi

> 
> That said, if I were you, I would go with introducing few generic refcount APIs.
> 
>         --yliu
> 
> 
> >
> > Aslo introduce the new API rte_eth_dev_lock_with_callback and
> > rte_eth_dev_unlock_with callback to let application to register a
> > callback function which will be invoked before a device is going to be
> > detached, the return value of the function will decide if device will
> > continue be detached or not, this support application to do condition
> > check at runtime.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang at intel.com>
> > Reviewed-by: Anatoly Burakov <anatoly.burakov at intel.com>