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.
>
> >>
> >> 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");
> >>>
> >>>
> >
