[dpdk-dev] [PATCH v2 05/22] ethdev: introduce device lock

[Zhang, Qi Z]

> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Thursday, June 21, 2018 4:51 PM
> To: Zhang, Qi Z <qi.z.zhang at intel.com>; thomas at monjalon.net
> 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: [PATCH v2 05/22] ethdev: introduce device lock
> 
> On 21-Jun-18 3:00 AM, 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 application to prevent unexpected device
> > detaching, especially in multi-process environment.
> >
> > Also 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>
> > ---
> 
> <snip>
> 
> > +
> > +static int clean_lock_callback_one(uint16_t port_id) {
> > +	struct lock_entry *le;
> > +	int ret = 0;
> > +
> > +	rte_spinlock_lock(&lock_entry_lock);
> > +
> > +	TAILQ_FOREACH(le, &lock_entry_list, next) {
> > +		if (le->port_id == port_id)
> > +			break;
> > +	}
> > +
> > +	if (le != NULL) {
> > +		le->ref_count--;
> > +		if (le->ref_count == 0) {
> > +			TAILQ_REMOVE(&lock_entry_list, le, next);
> > +			free(le);
> > +		}
> > +	} else {
> > +		ret = -ENOENT;
> > +	}
> > +
> > +	rte_spinlock_unlock(&lock_entry_lock);
> > +	return ret;
> > +
> > +}
> > +
> > +void clean_lock_callback(uint16_t port_id) {
> > +	int ret;
> > +
> > +	for (;;) {
> > +		ret = clean_lock_callback_one(port_id);
> > +		if (ret == -ENOENT)
> > +			break;
> > +	}
> > +}
> 
> Why not lock/unlock the list in clean_lock_callback() and proceed to cleaning
> callbacks one by one, instead of locking-and-unlocking the list over and over
> again?

Definitely!

> 
> --
> Thanks,
> Anatoly