[ofw] [PATCH] opensm - libvendor reduce stack utilization
Smith, Stan
stan.smith at intel.com
Fri May 21 08:40:44 PDT 2010
Hal Rosenstock wrote:
> On Thu, May 20, 2010 at 3:43 PM, Smith, Stan <stan.smith at intel.com>
> wrote:
>>
>> Reduce stack consumption by using a union of structs.
>> Borrowed from osm_vendor_umad_sa.c
>>
>> signed-off-by: stan smith <stan.smith at intel.com>
>>
>> --- 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
>> ofw at lists.openfabrics.org
>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
More information about the ofw
mailing list