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

[Tzachi Dar]

Hi Fab,

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'm not saying that it the ideal solution, but it is very common and
probably "good enough" for almost all implementations. 

Thanks
Tzachi

>-----Original Message-----
>From: Eitan Zahavi [mailto:eitan at mellanox.co.il]
>Sent: Wednesday, August 24, 2005 9:09 AM
>To: Fab Tillier
>Cc: Aviram Gutman; openib-windows at openib.org
>Subject: [Openib-windows] Re: Complib changes to support OpenSM in OpenIB
>
>Fab Tillier wrote:
>>>From: Eitan Zahavi [mailto:eitan at mellanox.co.il]
>>>Sent: Tuesday, August 23, 2005 1:15 AM
>>>
>>>Hi Fab,
>>>
>>>Please the my responses regarding OpenSM changes inline:
>>
>>
>> Sure, see below, and thank you for sending in plain text - it makes it
>much
>> easier for me to reply.
>No problem. I got used to use two mailers...
>Not very productive but arguing with the rest of the LINUX developers is
>not very productive either.
>>
>>
>>>Fab Tillier wrote:
>>>
>> I think we can do something a little smarter in the implementation
>without
>> adding a new API - if we record what thread context we are in before
>invoking
>> the user callback (with GetCurrentThreadId).  We can then check to see if
>we are
>> in the same thread when we invoke start, and not invoke stop at all.  I'm
>at IDF
>> until Thursday, but I'll try to get something coded up so you can try it.
>If
>> you code it yourselves before I get back in the office, please send the
>patch so
>> I don't duplicate the effort.
>This is a nice idea. We can wait for your return to the office. So we
>will wait for your patch.
>>
>>
>>>I agree that we should fix the LINUX side. But as things tend to take
>>>long time to change in the windows area so they take time in the LINUX
>>>area. So for now we simply will make the windows main.c little more
>>>different from the LINUX main.c by removing the section that checks the
>>>    "debug"-ness of the used libraries.
>>
>>
>> Things do move slower than I wish - I wasn't saying fix the Linux side
>now,
>> rather just put a #ifdef for the check to only have it in Linux builds,
>and add
>> an item to the todo list for Linux.
>As I said we are using a different main.c anyway.
>>
>>
>>>>>3. cl_qlock_pool.c - as Eitan said we need it in OpenSM for know and he
>will
>>>>>change the code to work with external locks
>>>>
>>>>As I said before, I'd like to see this file stay in OpenSM.  If OpenSM
>moves
>>>>away from using it, it can then be removed.  I don't want to add it to
>>>>complib
>>>>because I don't want any other ULPs to use it.  Further, the
>implementation
>>>>should be entirely inline functions since there's very little code
>there.
>>>
>>>Moving files around means changing "include" directives.
>>>It is not only a matter of the directory the file is part of.
>>
>>
>> You'll be moving files anyway, as well as updating include directives.  I
>don't
>> think qlockpool has much value, and don't want to encourage its use.  You
>> currently have a complib subdirectory to osm.  Keep that directory and
>put the
>> qlockpool file in there.  The same applies to the cl_event_wheel thing.
>OK.
>>
>>
>>>That is exactly the case. The only exception is ib_copy_ca_attr which is
>>>a prototype that is actually never used and implemented.
>>
>>
>> You mean it isn't used or implemented in OSM, right?
>I mean ib_types.h does not declare any non static inline functions but
>the ib_copy_ca_attr which can be removed.
>>
>> Also, there was an issue about catching signals, and you were going to
>look at
>> using the SetConsoleCtrlHandler function, or even better eliminating that
>> functionality since the underlying access layers should handle the app
>exiting.
>> What is the status of this?
>I think we could do that - I.e. do not catch the kill.
>I need to double check on the LINUX front. In the past adding the signal
>catch helped preventing kernel resources leak and even kernel oops.
>>
>> Thanks,
>>
>> - Fab
>
>_______________________________________________
>openib-windows mailing list
>openib-windows at openib.org
>http://openib.org/mailman/listinfo/openib-windows
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20050826/8d8b34c9/attachment.html>