Re: [dpdk-dev] [PATCH v2 2/6] ethdev: add port ownership

[Matan Azrad <matan@mellanox.com>]

Hi Neil
From: Neil Horman, Friday, January 19, 2018 3:41 AM
> On Thu, Jan 18, 2018 at 08:21:34PM +0000, Matan Azrad wrote:
> > Hi Neil.
> >
> > From: Neil Horman, Thursday, January 18, 2018 8:42 PM

<snip>
> > 1. What exactly do you want to improve?(in details) 2. Which API
> > specifically do you want to change(\ part of code)?
> > 3. What is the missing in current code(you can answer it in V3 I sent if you
> want) which should be fixed?
> >
> >
> > <snip> sorry for that, I think it is not relevant continue discussion if we are
> not fully understand each other. So let's start from the beginning "with good
> order :)" by answering the above questions.
> 
> 
> Sure, this seems like a reasonable way to level set.
> 
> I mentioned in another thread that perhaps some of my issue here is
> perception regarding what is meant by ownership.  When I think of an
> ownership api I think primarily of mutual exclusion (that is to say,
> enforcement of a single execution context having access to a resource at any
> given time.  In my mind the simplest form of ownership is a spinlock or a
> mutex.  A single execution context either does or does not hold the resource
> at any one time.  Those contexts that attempt to gain excusive access to the
> resource call an api that (depending on
> implementation) either block continued execution of that thread until
> exclusive access to the resource can be granted, or returns immediately with
> a success or error indicator to let the caller know if access is granted.
> 
> If I were to codify this port ownership api in pseudo code it would look
> something like this:
> 
> struct rte_eth_dev {
> 
> 	< eth dev bits >
> 	rte_spinlock_t owner_lock;
> 	bool locked;
> 	pid_t owner_pid;
> }
> 
> 
> bool rte_port_claim_ownership(struct rte_eth_dev *dev) {
> 	bool ret = false;
> 
> 	spin_lock(dev->owner_lock);
> 	if (dev->locked)
> 		goto out;
> 	dev->locked = true;
> 	dev->owner_pid = getpid();
> 	ret = true;
> out:
> 	spin_unlock(dev->lock)
> 	return ret;
> }
> 
> 
> bool rte_port_release_ownership(rte_eth_dev *dev) {
> 
> 	boot ret = false;
> 	spin_lock(dev->owner_lock);
> 	if (!dev->locked)
> 		goto out;
> 	if (dev->owner_pid != getpid())
> 		goto out;
> 	dev->locked = false;
> 	dev_owner_pid = 0;
> 	ret = true;
> out:
> 	spin_unlock(dev->owner_lock)
> 	return ret;
> }
> 
> bool rte_port_is_owned_by(struct rte_eth_dev *dev, pid_t pid) {
> 	bool ret = false;
> 
> 	spin_lock(dev->owner_lock);
> 	if (pid)
> 		ret = (dev->locked && (pid == dev->owner_pid));
> 	else
> 		ret = dev->locked;
> 	spin_unlock(dev->owner_lock);
> 	return ret;
> }
> 
> The idea here is that lock state is isolated from ownership information.  Any
> context has the opportunity to lock the resource (in this case the eth port)
> despite its ownership object.
> 
> In comparison, your api, which is in may ways simmilar, separates the
> creation of ownership objects to a separate api call, and that ownership
> information embodies state that is integral to the ability to get exclusive
> access to the resource.  I.E. if thread A calls your owner_new call, and then
> thread B calls owner_new, thread A will never be able to get access to any
> port unless it calls owner_new again.
> 
> Does that help clarify my position?

Now I fully understand you, thanks for your patience.

So, you are missing here one of the main ideas of my port ownership intention.
There are options for X>1 different uncoordinated owners running in the same thread.

For example:
1. Think about Testpmd control commands that call to failsafe port devop which call to its sub-devices devops, while tespmd is different owner(controlling failsafe-port) and failsafe is a different owner(controlling all its sub-devices ports), There are both run control commands in the same thread and there are uncoordinated!
 2. Interrupt callbacks that anyone can register to them and all will run by the DPDK host thread. 

So, no any optional  owner becomes an owner, it depends in the specific implementation.

So if some "part of code" wants to manage a port exclusively and wants to take ownership of it to prevent other "part of code" to use this port :
1. Take ownership.
2. It should ask itself: Am I run in different threads\processes? If yes, it should synchronize its port management. 
3. Release ownership in the end.

Remember that may be different "part of code"s running in the same thread\threads\process\processes.

Thanks, Matan.
> 
> Regards
> Neil
> 
> }