[Openib-windows] Re: Complib changes to support OpenSM in OpenIB

[Eitan Zahavi]

Fab Tillier wrote:
> What is the problem with keeping qlockpool in opensm?
To have qlockpool be part of OpenSM we will need to:
1. Modify its implementation to be all static inline
2. Change all files including it to use <opensm/cl_qlock_pool.h> and not <complib/cl_qlock_pool.h>

Making qlockpool be part of complib requires:
1. Add qlocpool.h to the SOURCES file in the complib dir. One file change!

We can do either or.
The argument was about not USING qlockpool in OpenSM which is a much bigger issue.

EZ
> 
>  
> 
> -----Original Message-----
> From: Tzachi Dar [mailto:tzachid at mellanox.co.il] 
> Sent: Sunday, August 28, 2005 2:15 AM
> To: Tillier, Fabian; Tzachi Dar; Eitan Zahavi
> Cc: Aviram Gutman; openib-windows at openib.org
> Subject: RE: [Openib-windows] Re: Complib changes to support OpenSM in
> OpenIB
> 
>  
> 
> Hi fab, 
> 
> I read the answer that you have sent me and I still disagree. 
> Locking of objects and maintaining their life cycle is a very
> complicated task no matter what approach is used. I hope that we can
> agree at least on this.
> 
> As a result there are also many "valid" approaches on how to solve these
> problems. Deciding which approach is better has to take the following
> considerations: 1) simplicity of the code. 2) Performance of the code.
> 3) Time that is available for developing the code. 4) Existing code
> already. 5) Does the code has to be portable or not (and to where), and
> probably many other issues. Please also note, that when trying to decide
> on one of this issues, it is very hard to tell what are the
> "sub-considerations" that should be argued for example let's take
> performance: Are we measuring performance on a single processor or on a
> multiprocessor system? Are we measuring the performance when the system
> is loaded or not. Are we measuring the worst time complicity or the
> average complexity?
> 
> So far, many questions and not a single answer. 
> I have wrote all this questions just to show that since the question is
> very complicated, it is hard to believe that it will have a simple
> answer. (I guess that this is the nice thing about computer since:
> although this "science" exists for more than 50 years, there is not one
> implementation of an array that is agreed on everyone).
> 
> The answer that you gave me bellow, if I understand it correctly, tells
> that it is possible to do better than always taking the lock (at some
> situations). Although I agree that it is some times true, I would like
> to suggest three other approaches to solving a programming problem in
> which this is not the case. What I mean by that is that using these
> approaches, one can write a "good" (what is really good, was not yet
> defined) program. Under this approaches one need a "multi-thread safe"
> container.
> 
> These approaches are: 
> 1) Use reference counting to mange the life time of the container, and
> use it from many threads. Personally I believe that when the project is
> big enough, this is the only thing that works, but this is just my
> opinion.
> 
> 2) Use reference counting to manage the life time of more than one
> container and use them all. 
> 3) Have the container(s) accessed by more than one thread simultaneously
> and destroy it only when that thread is dead. 
> 
> Again, I'm not saying that any of this approaches are "always better",
> but I do say that they might be used. As a result, I suggest that we
> will add the locked-pool to the complib implementation.
> 
> Thanks 
> Tzachi 
> 
> One other small point that I want to mention is this: weather to use
> inline or not inline is an implementation detail - although we might
> have to close this detail one day, I'm not sure that this is the time. 
> 
> 
>>-----Original Message----- 
>>From: Fab Tillier [ mailto:ftillier at silverstorm.com
> 
> <mailto:ftillier at silverstorm.com> ] 
> 
>>Sent: Friday, August 26, 2005 8:56 PM 
>>To: 'Tzachi Dar'; Eitan Zahavi 
>>Cc: Aviram Gutman; openib-windows at openib.org 
>>Subject: RE: [Openib-windows] Re: Complib changes to support OpenSM in 
>>OpenIB 
>>
>>
>>>From: Tzachi Dar [ mailto:tzachid at mellanox.co.il
> 
> <mailto:tzachid at mellanox.co.il> ] 
> 
>>>Sent: Friday, August 26, 2005 6:37 AM 
>>>
>>>Can you please explain in more detail what is wrong with the 
>>
>>cl_qlockpool.c ? 
>>
>>>It seems to me like a very common programming way that there is a 
>>
>>container 
>>
>>>that is single threaded, and in order to make it "multi-threaded
> 
> save" 
> 
>>one 
>>
>>>adds a lock around it. 
>>
>>I have generally found that locking is required at a higher level than
> 
> just 
> 
>>the 
>>pool or list operations.  There must be extra logic to prevent the pool
> 
> 
>>from 
> 
>>being destroyed while items are out, and locking at such a fine level 
>>doesn't 
>>help at all. 
>>
>>Even looking in the OSM code base, the few cases that I tried to figure
> 
> out 
> 
>>held 
>>other locks while accessing the pools.  If there are other locks held,
> 
> then 
> 
>>locking at the pool level is a waste of resources as the lock will
> 
> never 
> 
>>see 
>>contention.  Having the lock hidden in the pool operations makes it
> 
> much 
> 
>>less 
>>obvious when the lock is unnecessary.  If you see explicit locking,
> 
> it's 
> 
>>much 
>>easier to find out if the lock is needed or not, and if not,
> 
> eliminated. 
> 
>>Lastly, if an operation does something like: 
>>
>>get item from pool 
>>try something 
>>if failed, put item in pool 
>>
>>If "try something" is a relatively quick operation, you'll get better 
>>performance taking the lock up front and releasing it after the failure
> 
> 
>>test. 
>>
>>Also, as I mentioned before, having a function call just to mask a
> 
> couple 
> 
>>function calls doesn't seem efficient.  The qlockpool functions should
> 
> be 
> 
>>inline. 
>>
>>- Fab 
> 
> 
>