Hal Rosenstock wrote: > On Thu, May 20, 2010 at 3:43 PM, Smith, Stan <[email protected]> > wrote: >> >> Reduce stack consumption by using a union of structs. >> Borrowed from osm_vendor_umad_sa.c >> >> signed-off-by: stan smith <[email protected]> >> >> --- a/ulp/opensm/user/libvendor/osm_vendor_mlx_sa.c Thu May 20 >> 11:51:00 2010 +++ b/ulp/opensm/user/libvendor/osm_vendor_mlx_sa.c >> Thu May 20 11:45:32 2010 @@ -593,14 +593,23 @@ osmv_query_sa(IN >> osm_bind_handle_t h_bind, >> IN const osmv_query_req_t * const p_query_req) >> { >> - osmv_sa_bind_info_t *p_bind = (osmv_sa_bind_info_t *) >> h_bind; + union { + ib_service_record_t svc_rec; >> + ib_node_record_t node_rec; >> + ib_portinfo_record_t port_info; >> + ib_path_rec_t path_rec; >> +#ifdef DUAL_SIDED_RMPP >> + ib_multipath_rec_t multipath_rec; >> +#endif >> + ib_class_port_info_t class_port_info; + } u; >> osmv_sa_mad_data_t sa_mad_data; >> + osmv_sa_bind_info_t *p_bind = (osmv_sa_bind_info_t *) h_bind; >> osmv_user_query_t *p_user_query; >> - ib_service_record_t svc_rec; >> - ib_node_record_t node_rec; >> - ib_portinfo_record_t port_info; >> - ib_path_rec_t path_rec; >> - ib_class_port_info_t class_port_info; >> +#ifdef DUAL_SIDED_RMPP >> + osmv_multipath_req_t *p_mpr_req; >> + int i, j; >> +#endif >> osm_log_t *p_log = p_bind->p_log; >> ib_api_status_t status; >> >> @@ -610,6 +619,8 @@ >> sa_mad_data.method = IB_MAD_METHOD_GETTABLE; >> sa_mad_data.attr_mod = 0; >> >> + memset((void*)&u, 0, sizeof(u)); >> + >> /* Set the MAD attributes and component mask correctly. */ >> switch (p_query_req->query_type) { >> >> @@ -634,7 +645,7 @@ >> sa_mad_data.attr_offset = >> ib_get_attr_offset(sizeof(ib_service_record_t)); >> sa_mad_data.comp_mask = 0; >> - sa_mad_data.p_attr = &svc_rec; >> + sa_mad_data.p_attr = &u.svc_rec; >> break; >> >> case OSMV_QUERY_SVC_REC_BY_NAME: >> @@ -645,8 +656,8 @@ >> sa_mad_data.comp_mask = IB_SR_COMPMASK_SNAME; >> sa_mad_data.attr_offset = >> ib_get_attr_offset(sizeof(ib_service_record_t)); >> - sa_mad_data.p_attr = &svc_rec; >> - memcpy(svc_rec.service_name, >> p_query_req->p_query_input, + sa_mad_data.p_attr = >> &u.svc_rec; + memcpy(u.svc_rec.service_name, >> p_query_req->p_query_input, >> sizeof(ib_svc_name_t)); >> break; >> >> @@ -657,8 +668,8 @@ >> sa_mad_data.comp_mask = IB_SR_COMPMASK_SID; >> sa_mad_data.attr_offset = >> ib_get_attr_offset(sizeof(ib_service_record_t)); >> - sa_mad_data.p_attr = &svc_rec; >> - svc_rec.service_id = >> + sa_mad_data.p_attr = &u.svc_rec; >> + u.svc_rec.service_id = >> *(ib_net64_t *) (p_query_req->p_query_input); >> break; >> >> @@ -670,7 +681,7 @@ >> sa_mad_data.attr_offset = >> ib_get_attr_offset(sizeof(ib_class_port_info_t)); >> sa_mad_data.comp_mask = 0; >> - sa_mad_data.p_attr = &class_port_info; >> + sa_mad_data.p_attr = &u.class_port_info; >> >> break; >> >> @@ -682,8 +693,8 @@ >> sa_mad_data.attr_offset = >> ib_get_attr_offset(sizeof(ib_node_record_t)); >> sa_mad_data.comp_mask = IB_NR_COMPMASK_NODEGUID; >> - sa_mad_data.p_attr = &node_rec; >> - node_rec.node_info.node_guid = >> + sa_mad_data.p_attr = &u.node_rec; >> + u.node_rec.node_info.node_guid = >> *(ib_net64_t *) (p_query_req->p_query_input); >> >> break; >> @@ -695,8 +706,8 @@ >> sa_mad_data.attr_offset = >> ib_get_attr_offset(sizeof(ib_portinfo_record_t)); >> sa_mad_data.comp_mask = IB_PIR_COMPMASK_LID; >> - sa_mad_data.p_attr = &port_info; >> - port_info.lid = *(ib_net16_t *) >> (p_query_req->p_query_input); + sa_mad_data.p_attr = >> &u.port_info; + u.port_info.lid = *(ib_net16_t *) >> (p_query_req->p_query_input); >> break; >> >> case OSMV_QUERY_PORT_REC_BY_LID_AND_NUM: >> @@ -746,39 +757,37 @@ >> case OSMV_QUERY_PATH_REC_BY_PORT_GUIDS: >> osm_log(p_log, OSM_LOG_DEBUG, >> "osmv_query_sa DBG:001 %s", >> "PATH_REC_BY_PORT_GUIDS\n"); - memset(&path_rec, 0, >> sizeof(ib_path_rec_t)); + memset(&u.path_rec, 0, >> sizeof(ib_path_rec_t)); >> sa_mad_data.attr_id = IB_MAD_ATTR_PATH_RECORD; >> sa_mad_data.attr_offset = >> ib_get_attr_offset(sizeof(ib_path_rec_t)); >> sa_mad_data.comp_mask = >> (IB_PR_COMPMASK_DGID | IB_PR_COMPMASK_SGID | >> IB_PR_COMPMASK_NUMBPATH); >> - path_rec.num_path = 0x7f; >> - sa_mad_data.p_attr = &path_rec; >> - ib_gid_set_default(&path_rec.dgid, >> + u.path_rec.num_path = 0x7f; >> + sa_mad_data.p_attr = &u.path_rec; >> + ib_gid_set_default(&u.path_rec.dgid, >> ((osmv_guid_pair_t *) >> (p_query_req-> >> - >> p_query_input))-> >> - dest_guid); >> - ib_gid_set_default(&path_rec.sgid, >> + >> p_query_input))->dest_guid); + >> ib_gid_set_default(&u.path_rec.sgid, >> ((osmv_guid_pair_t *) >> (p_query_req-> >> - >> p_query_input))-> >> - src_guid); >> + >> p_query_input))->src_guid); >> break; >> >> case OSMV_QUERY_PATH_REC_BY_GIDS: >> osm_log(p_log, OSM_LOG_DEBUG, >> "osmv_query_sa DBG:001 %s", >> "PATH_REC_BY_GIDS\n"); - memset(&path_rec, 0, >> sizeof(ib_path_rec_t)); + memset(&u.path_rec, 0, >> sizeof(ib_path_rec_t)); >> sa_mad_data.attr_id = IB_MAD_ATTR_PATH_RECORD; >> sa_mad_data.attr_offset = >> ib_get_attr_offset(sizeof(ib_path_rec_t)); >> sa_mad_data.comp_mask = >> - (IB_PR_COMPMASK_DGID | IB_PR_COMPMASK_SGID | >> IB_PR_COMPMASK_NUMBPATH); >> - path_rec.num_path = 0x7f; >> - sa_mad_data.p_attr = &path_rec; >> - memcpy(&path_rec.dgid, >> + (IB_PR_COMPMASK_DGID | IB_PR_COMPMASK_SGID | >> IB_PR_COMPMASK_NUMBPATH); + u.path_rec.num_path = 0x7f; >> + sa_mad_data.p_attr = &u.path_rec; >> + memcpy(&u.path_rec.dgid, >> &((osmv_gid_pair_t *) >> (p_query_req->p_query_input))-> >> dest_gid, sizeof(ib_gid_t)); >> - memcpy(&path_rec.sgid, >> + memcpy(&u.path_rec.sgid, >> &((osmv_gid_pair_t *) >> (p_query_req->p_query_input))-> >> src_gid, sizeof(ib_gid_t)); >> break; >> @@ -786,18 +795,18 @@ >> case OSMV_QUERY_PATH_REC_BY_LIDS: >> osm_log(p_log, OSM_LOG_DEBUG, >> "osmv_query_sa DBG:001 %s", >> "PATH_REC_BY_LIDS\n"); - memset(&path_rec, 0, >> sizeof(ib_path_rec_t)); + memset(&u.path_rec, 0, >> sizeof(ib_path_rec_t)); >> sa_mad_data.method = IB_MAD_METHOD_GET; >> sa_mad_data.attr_id = IB_MAD_ATTR_PATH_RECORD; >> sa_mad_data.attr_offset = >> ib_get_attr_offset(sizeof(ib_path_rec_t)); >> sa_mad_data.comp_mask = >> (IB_PR_COMPMASK_DLID | IB_PR_COMPMASK_SLID); >> - sa_mad_data.p_attr = &path_rec; >> - path_rec.dlid = >> + sa_mad_data.p_attr = &u.path_rec; >> + u.path_rec.dlid = >> ((osmv_lid_pair_t *) >> (p_query_req->p_query_input))-> >> dest_lid; >> - path_rec.slid = >> + u.path_rec.slid = >> ((osmv_lid_pair_t *) >> (p_query_req->p_query_input))->src_lid; >> break; > > Should this patch be pushed to Linux > (opensm/libvendor/osm_vendor_mlx_sa.c) ?
It would certainly assist in future ports to Windows, although not a requirement. Bottom line, the change does not address a specific bug but more of a coding/stack efficiency issue. Coding style was borrowed from 'osm_vendor_ibumad_sa.c'. Since the file appears to be of Mellanox origin and I assume still under their maintenance/use, perhaps they are the ones to make the call for Linux? > > -- Hal > >> _______________________________________________ >> ofw mailing list >> [email protected] >> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw _______________________________________________ ofw mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
