[ofa-general] [PATCH] opensm/osm_ucast_mgr: code consolidation and cleanup

[Sasha Khapyorsky]

Hi Yevgeny,

On 10:53 Wed 25 Jun     , Yevgeny Kliteynik wrote:
> OpenSM crashed in cl_qlist_insert_tail() on the following assert:
>
>    CL_ASSERT(p_list_item->p_list != p_list);

Yes, I see why it does. Thanks for finding this.

Something like this:

diff --git a/opensm/opensm/osm_ucast_mgr.c b/opensm/opensm/osm_ucast_mgr.c
index b9e484e..39ce2bd 100644
--- a/opensm/opensm/osm_ucast_mgr.c
+++ b/opensm/opensm/osm_ucast_mgr.c
@@ -743,6 +743,9 @@ static void ucast_mgr_build_lfts(osm_ucast_mgr_t *p_mgr)
 
 	cl_qmap_apply_func(&p_mgr->p_subn->sw_guid_tbl,
 			   __osm_ucast_mgr_process_tbl, p_mgr);
+
+	while(cl_is_qlist_empty(&p_mgr->port_order_list))
+		cl_qlist_remove_head(&p_mgr->port_order_list);
 }
 
 /**********************************************************************

, solves it "conventionally".

OTOH I am not sure that this CL_ACCERT() is optimal. As far as I can
understand it check that list element is not entered list twice (and it
is ok), but as side effect it also doesn't permit to reinitialize and
reuse the qlist without explicit removing all members (as was done in
this patch and I guess in some other places too) and this is bad.

Personally I would vote for removing this assert at all - a bugs which
it tries to prevent are fatal in runtime anyway. Thoughts?

Sasha