Hi Yuanhan: > -----Original Message----- > From: Yuanhan Liu [mailto:y...@fridaylinux.org] > Sent: Wednesday, July 4, 2018 10:20 AM > To: Zhang, Qi Z <qi.z.zh...@intel.com>; tho...@monjalon.net; Burakov, > Anatoly <anatoly.bura...@intel.com> > Cc: Ananyev, Konstantin <konstantin.anan...@intel.com>; dev@dpdk.org; > Richardson, Bruce <bruce.richard...@intel.com>; Yigit, Ferruh > <ferruh.yi...@intel.com>; Shelton, Benjamin H > <benjamin.h.shel...@intel.com>; Vangati, Narender > <narender.vang...@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.zh...@intel.com> > > Reviewed-by: Anatoly Burakov <anatoly.bura...@intel.com>
