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

[Fab Tillier]

> From: Tzachi Dar [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