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

[Fab Tillier]

Apparently, your mail server converts all outgoing mail to HTML, even if you try
to send mail in plain text.  While Gilad looks into this bug, here goes.

> -----Original Message-----
> From: Yossi Leybovich [mailto:sleybo at mellanox.co.il]
> Sent: Thursday, August 18, 2005 4:32 AM
> 
> 1. cl_timers:
>         a.
>         we tried to call cl_timer_start from within the timer call back, and
> the thread enter to deadlock . We notice that cl_timer_star call   to
> cl_timer_stop which call to DeleteTimerQueueTimer with INVALI_HANDLE which
> cause it to wait to all the callbacks to finish.
>         Msdn:"...If this parameter is INVALID_HANDLE_VALUE, the function waits
> for the timer callback function to complete before returning.    " So if we
> call it within the timer callback we cause deadlock.
> 
>         The kernel implementation of cl_timer_start does not call
> cl_timer_stop why ?( I am not sure that in the Kernel one can be sure that
> he want get more DPCs)

In the kernel, the timer is implicitly cancelled if it is already in the queue
it when it is re-queued.  See the docs for KeSetTimer.

In user-mode, we want to make sure that repeated calls to cl_timer_start don't
result in multiple callbacks, so we explicitly cancel the timer if it exists.

>         Solution to this problem is to add cl_timer_strat_cb function that
> wont call to timer stop and be able to call
>       from within the callback

Let's try to find a better solution.  Are you always resetting the event from
the callback?  Are you really looking for a periodic timer here, rather than a
one-shot timer?  If so, it would make sense to expose periodic timers, rather
than forcing the user to set a one-shot timer repeatedly.

>         b.
>         I added few function for timers in order to add our perf_main tool To
> OpenIB
>         it include
>         cl_get_time_staamp_usec

cl_get_time_stamp already returns usecs.

>         cl_get_time_staamp_nsec
>         cl_get_frequancy
>         I can send patch to that.

Sure, send a patch for the last two.

> 2. cl_is_debug function - I send you patch for that , currently Linux version
> of OpenSM use it so we need it in our complib as well
>         I don't think its too risky

Runtime checks for linkage errors don't belong, especially in Windows where we
have distinct binary names for debug and release versions of the same component.
If this is needed for Linux, put it in a #ifdef LINUX or something similar.
Long term, I suggest someone fix the Linux side of things to prevent such
linkage errors.

> 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.
 
> 4. Not related to the OpenSM  but I also sent patch for IB_COMPANY_NAME env
> variable for installsp.exe can you apply it and I will also change the
> ics_ver.h file

This is in my pipeline of things to do.  I'm currently working on pushing my
Winsock Direct updates to the SVN repository, and when that's complete will work
on this and the other patches received.  This of course is based on the
assumption that the WSD code is of greater interest than the company name.  If
not, I can reorder things - let me know.

> I think that for most of the things I already sent patches, If not just tell
> me.
> We really appreciate if you can apply the patches so we can move forward to
> add OpenSM
>
> - We intend to merge OpenSM ib_types.h to ibal ib_types.h but it takes time ,
> mean while we create copy of ib_types locally for OpenSM

As long as the OpenSM ib_types doesn't create linkage issues, there shouldn't be
a problem here.  That means that the OpenSM version of ib_types.h needs to
contain only static inline functions.

- Fab