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

Eitan Zahavi eitan at mellanox.co.il
Tue Aug 23 23:08:58 PDT 2005


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




More information about the ofw mailing list