On 11/2/2018 9:37 AM, Honggang LI wrote:
> On Fri, Nov 02, 2018 at 09:17:43AM -0400, Hal Rosenstock wrote:
>> On 11/2/2018 9:11 AM, Honggang LI wrote:
>>> On Fri, Nov 02, 2018 at 08:40:40AM -0400, Hal Rosenstock wrote:
>>>> On 11/1/2018 10:53 PM, Honggang LI wrote:
>>>>> From: Honggang Li <honli@xxxxxxxxxx>
>>>>>
>>>>> Signed-off-by: Honggang Li <honli@xxxxxxxxxx>
>>>>> ---
>>>>> complib/cl_event_wheel.c | 9 +++------
>>>>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/complib/cl_event_wheel.c b/complib/cl_event_wheel.c
>>>>> index 27443f66..21e1f4ee 100644
>>>>> --- a/complib/cl_event_wheel.c
>>>>> +++ b/complib/cl_event_wheel.c
>>>>> @@ -457,6 +457,7 @@ uint32_t cl_event_wheel_num_regs(IN cl_event_wheel_t * const p_event_wheel,
>>>>> }
>>>>>
>>>>> #ifdef __CL_EVENT_WHEEL_TEST__
>>>>> +#include <unistd.h> /* sleep() */
>>>>>
>>>>> /* Dump out the complete state of the event wheel */
>>>>> void __cl_event_wheel_dump(IN cl_event_wheel_t * const p_event_wheel)
>>>>> @@ -511,14 +512,10 @@ static uint64_t __test_event_aging(uint64_t key, uint32_t num_regs, void *contex
>>>>> int main()
>>>>> {
>>>>> cl_event_wheel_t event_wheel;
>>>>> - /* uint64_t key; */
>>>>>
>>>>> /* init complib *
>>>>> complib_init();
>>>>>
>>>>> - /* construct */
>>>>> - cl_event_wheel_construct(&event_wheel);
>>>>> -
>>>>
>>>> While this is redundant, it breaks the general design pattern. Should it
>>>> be changed ? Does it make some difference ?
>>>
>>> The cl_event_wheel_init duplicates the works for cl_event_wheel_construct.
>>
>> That's what I meant by it being redundant.
>>
>>> If you want keep the general design pattern, you need update
>>> cl_event_wheel_init to call cl_event_wheel_construct.
>>
>> Why ? It may have just been "optimized" to incorporate what was needed
>> from construct in the init function. I don't see why this needs to change.
>
> Just scanned opensm code, all usage of cl_event_wheel_init_ex stick with cl_event_wheel_construct.
>
> OK. It seems your comments make sense.
Just pushed the remaining parts of these 2 patches.
>>
>>>>
>>>> Note that event wheel is used for SM trap handling and also to support
>>>> now "ancient" vendor driver (for generation 1 prior to OpenIB/OpenFabrics).
>>
>> These use cases follow the design pattern of calling construct and then
>> init.
>>
>>>>
>>>>> /* init */
>>>>> cl_event_wheel_init(&event_wheel);
>>>>>
>>>>> @@ -534,7 +531,7 @@ int main()
>>>>> "The Second Aging Event");
>>>>>
>>>>> cl_event_wheel_reg(&event_wheel, 3, /* key */
>>>>> - cl_get_time_stamp() + 3500000, /* 3 sec lifetime */
>>>>> + cl_get_time_stamp() + 3500000, /* 3.5 sec lifetime */
>>>>> __test_event_aging, /* cb */
>>>>> "The Third Aging Event");
>>>>>
>>>>> @@ -542,7 +539,7 @@ int main()
>>>>>
>>>>> sleep(2);
>>>>> cl_event_wheel_reg(&event_wheel, 2, /* key */
>>>>> - cl_get_time_stamp() + 8000000, /* 3 sec lifetime */
>>>>> + cl_get_time_stamp() + 8000000, /* 8 sec lifetime */
>>>>> __test_event_aging, /* cb */
>>>>> "The Second Aging Event Moved");
>>>>>
>>>>>
>>>
>
