On 3/19/2013 9:00 PM, Weiny, Ira wrote:
>> -----Original Message-----
>> From: [email protected] [mailto:linux-rdma-
>> [email protected]] On Behalf Of Hal Rosenstock
>> Sent: Tuesday, March 19, 2013 9:34 AM
>> To: linux-rdma ([email protected])
>> Subject: Re: [PATCH 07/07] opensm/perfmgr: add sl support
>>
>>
>> This doesn't appear to have made it to the list so I'm resending.
>>
>> -------- Original Message --------
>> Subject: Re: [PATCH 07/07] opensm/perfmgr: add sl support
>> Date: Tue, 19 Mar 2013 08:52:04 -0400
>> From: Hal Rosenstock <[email protected]>
>> Reply-To: [email protected] <[email protected]>
>> To: Ira Weiny <[email protected]>, Ira Weiny <[email protected]>
>> CC: Hal Rosenstock <[email protected]>
>>
>> On 2/28/2013 8:09 PM, Ira Weiny wrote:
>>>
>>> SL's are queried internally when running in a master SM
>>
>> Similarly, does PerfMgr really need it's own discovery code when not in
>> standalone mode or could this also be gleaned from existing data SM
>> structures ?
>
> Yes it could be, however, issuing internal "queries" seemed cleaner.
>
>>
>>> and queried from the SA when running in a standby or inactive SM.
>>>
>>> Signed-off-by: Ira Weiny <[email protected]>
>>> ---
>>> include/opensm/osm_perfmgr.h | 11 ++
>>> opensm/osm_perfmgr.c | 345
>> +++++++++++++++++++++++++++++++++++++++---
>>> 2 files changed, 336 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/include/opensm/osm_perfmgr.h
>>> b/include/opensm/osm_perfmgr.h index 4141d41..5ca1cf4 100644
>>> --- a/include/opensm/osm_perfmgr.h
>>> +++ b/include/opensm/osm_perfmgr.h
>>> @@ -2,6 +2,7 @@
>>> * Copyright (c) 2007 The Regents of the University of California.
>>> * Copyright (c) 2007-2009 Voltaire, Inc. All rights reserved.
>>> * Copyright (c) 2009,2010 HNR Consulting. All rights reserved.
>>> + * Copyright (c) 2013 Lawrence Livermore National Security, All
>>> + rights reserved
>>> *
>>> * This software is available to you under a choice of one of two
>>> * licenses. You may choose to be licensed under the terms of the
>>> GNU @@ -90,6 +91,11 @@ typedef enum {
>>> PERFMGR_SWEEP_SUSPENDED
>>> } osm_perfmgr_sweep_state_t;
>>>
>>> +typedef struct pr_map_item {
>>> + cl_map_item_t map_item;
>>> + ib_path_rec_t pr;
>>> +} pr_map_item_t;
>>> +
>>> typedef struct monitored_port {
>>> uint16_t pkey_ix;
>>> ib_net16_t orig_lid;
>>> @@ -150,6 +156,11 @@ typedef struct osm_perfmgr {
>>> int16_t local_port;
>>> int rm_nodes;
>>> boolean_t query_cpi;
>>> + cl_qmap_t path_rec_map;
>>
>> Where does cleanup of path_rec_map occur ? Did I miss that ?
>
> Insert_pr_map will remove entries when new ones with that DLID are added.
>
> I did not clean up on exit. I'll add that.
>
>>
>>> + /* when operating in stand alone mode we are required to query the
>>> + * remote master SA */
>>> + osm_bind_handle_t sa_bind_handle;
>>> + int pr_query_outstanding;
>>> } osm_perfmgr_t;
>>> /*
>>> * FIELDS
>>> diff --git a/opensm/osm_perfmgr.c b/opensm/osm_perfmgr.c index
>>> 9df28f9..b1cd419 100644
>>> --- a/opensm/osm_perfmgr.c
>>> +++ b/opensm/osm_perfmgr.c
>>> @@ -2,7 +2,7 @@
>>> * Copyright (c) 2007 The Regents of the University of California.
>>> * Copyright (c) 2007-2009 Voltaire, Inc. All rights reserved.
>>> * Copyright (c) 2009,2010 HNR Consulting. All rights reserved.
>>> - * Copyright (c) 2013 Lawrence Livermore National Security. All rights *
>> reserved.
>>> + * Copyright (c) 2013 Lawrence Livermore National Security. All rights
>> reserved.
>>> *
>>> * This software is available to you under a choice of one of two
>>> * licenses. You may choose to be licensed under the terms of the
>>> GNU @@ -123,6 +123,7 @@ static inline void diff_time(struct timeval
>>> *before, struct timeval *after, static void
>>> init_monitored_nodes(osm_perfmgr_t * pm) {
>>> cl_qmap_init(&pm->monitored_map);
>>> + cl_qmap_init(&pm->path_rec_map);
>>> pm->remove_list = NULL;
>>> cl_event_construct(&pm->sig_query);
>>> cl_event_init(&pm->sig_query, FALSE); @@ -254,6 +255,10 @@ Exit:
>>> OSM_LOG_EXIT(pm->log);
>>> }
>>>
>>> +static void perfmgr_sa_mad_recv_cb(osm_madw_t * p_madw, void
>> *bind_context,
>>> + osm_madw_t * p_req_madw);
>>> +static void perfmgr_sa_mad_send_err_cb(void *bind_context,
>>> + osm_madw_t * p_madw);
>>>
>> /**********************************************************
>> ************
>>> * Bind the PerfMgr to the vendor layer for MAD sends/receives
>>>
>>>
>> **********************************************************
>> ************
>>> / @@ -294,6 +299,22 @@ ib_api_status_t
>> osm_perfmgr_bind(osm_perfmgr_t
>>> * pm, ib_net64_t port_guid)
>>> OSM_LOG(pm->log, OSM_LOG_ERROR,
>>> "ERR 5404: Vendor specific bind failed (%s)\n",
>>> ib_get_err_str(status));
>>> + goto Exit;
>>> + }
>>> +
>>> + bind_info.mad_class = IB_MCLASS_SUBN_ADM;
>>> + bind_info.class_version = 2;
>>
>> Are any other bind parameters needed ?
>
> None that are different from the previous bind. One could argue for
> different timeout/retry parameters
> but we would have to add those to the config file which don't exist now.
OK; I see those parameters set by the existing bind for PerfMgr class.
>
>>
>>> +
>>> + pm->sa_bind_handle = osm_vendor_bind(pm->vendor, &bind_info,
>>> + pm->mad_pool,
>>> + perfmgr_sa_mad_recv_cb,
>>> + perfmgr_sa_mad_send_err_cb,
>> pm);
>>
>> Why not only do this when in standalone mode ? Is that due to the bind
>> being needed based on SM state and that isn't known here ?
>
> Yes. Also we could be master and then be reduced to standby mode later.
> The bind is inexpensive and seemed reasonable to just have available
> regardless of mode.
It's not the expense of the bind that concerns me; it's opening the
unnecessary "door" for additional received MADs that does.
>
>>
>>> +
>>> + if (pm->sa_bind_handle == OSM_BIND_INVALID_HANDLE) {
>>> + status = IB_ERROR;
>>> + OSM_LOG(pm->log, OSM_LOG_ERROR,
>>> + "ERR 540E: PM SA bind failed (%s)\n",
>>> + ib_get_err_str(status));
>>> }
>>>
>>> Exit:
>>> @@ -307,12 +328,17 @@ Exit:
>>> static void perfmgr_mad_unbind(osm_perfmgr_t * pm) {
>>> OSM_LOG_ENTER(pm->log);
>>> - if (pm->bind_handle == OSM_BIND_INVALID_HANDLE) {
>>> +
>>> + if (pm->bind_handle == OSM_BIND_INVALID_HANDLE)
>>> OSM_LOG(pm->log, OSM_LOG_ERROR, "ERR 5405: No
>> previous bind\n");
>>> - goto Exit;
>>> - }
>>> - osm_vendor_unbind(pm->bind_handle);
>>> -Exit:
>>> + else
>>> + osm_vendor_unbind(pm->bind_handle);
>>> +
>>> + if (pm->sa_bind_handle == OSM_BIND_INVALID_HANDLE)
>>> + OSM_LOG(pm->log, OSM_LOG_ERROR, "ERR 540F: No
>> previous SA bind\n");
>>> + else
>>> + osm_vendor_unbind(pm->sa_bind_handle);
>>> +
>>> OSM_LOG_EXIT(pm->log);
>>> }
>>>
>>> @@ -330,6 +356,250 @@ static ib_net32_t get_qp(monitored_node_t *
>> mon_node, uint8_t port)
>>> return qp;
>>> }
>>>
>>> +static inline boolean_t sm_not_active(osm_perfmgr_t *pm) {
>>> + return (pm->subn->sm_state == IB_SMINFO_STATE_STANDBY ||
>>> + pm->subn->sm_state == IB_SMINFO_STATE_NOTACTIVE); }
>>> +
>>> +static int get_sm_info(osm_perfmgr_t *pm, ib_net16_t *smlid, uint8_t
>>> +*smsl) {
>>> + int i, rc = -1;
>>> + uint32_t num_ports = 32;
>>> + ib_port_attr_t attr_array[32];
>>
>> 64 is better as there are 36 port switches. Also, this should be a define.
>
> Agreed. However, in the case of a switch the PM should be running on port 0
> only right?
Yes, in the case of a switch, PM could run on port 0.
>>
>>> +
>>> + osm_vendor_get_all_port_attr(pm->vendor, attr_array,
>> &num_ports);
>>> +
>>> + for(i = 0; i<num_ports; i++) {
>>> + if (attr_array[i].port_guid == pm->port_guid) {
>>> + *smlid = attr_array[i].sm_lid;
>>> + *smsl = attr_array[i].sm_sl;
>>> + rc = 0;
>>> + }
>>> + }
>>> +
>>> + return (rc);
>>> +}
>>> +
>>> +static void insert_pr_map(osm_perfmgr_t *pm, ib_path_rec_t *pr) {
>>> + pr_map_item_t *mi = calloc(1, sizeof(*mi));
>>> + if (mi) {
>>> + cl_map_item_t *tmp;
>>> + if ((tmp = cl_qmap_get(&pm->path_rec_map, (uint64_t)pr-
>>> dlid))
>>> + != cl_qmap_end(&pm->path_rec_map)) {
>>> + cl_qmap_remove_item(&pm->path_rec_map, tmp);
>>> + free(tmp);
>>> + }
>>> + memcpy(&mi->pr, pr, sizeof(mi->pr));
>>> + cl_qmap_insert(&pm->path_rec_map, (uint64_t)pr->dlid,
>>> + (cl_map_item_t *) mi);
>>> + } else {
>>> + OSM_LOG(pm->log, OSM_LOG_ERROR,
>>> + "ERR 54FC: Failed to allocate path "
>>> + "record map item for DLID %d",
>>
>> LIDs should be formatted as %u
>
> Ok.
>
>>
>>> + cl_ntoh16(pr->dlid));
>>> + }
>>> +}
>>> +
>>> +/**
>>>
>> +=========================================================
>> ============
>>> +====
>>> + * SA query call backs for the sa_bind_handle */ static void
>>> +perfmgr_sa_mad_recv_cb(osm_madw_t * p_madw, void *bind_context,
>>> + osm_madw_t * p_req_madw)
>>> +{
>>> + osm_perfmgr_t *pm = (osm_perfmgr_t *) bind_context;
>>> + ib_sa_mad_t *sa_mad;
>>> + int num_results;
>>> + int i;
>>> +
>>> + OSM_LOG_ENTER(pm->log);
>>> +
>>> + osm_madw_copy_context(p_madw, p_req_madw);
>>> + osm_mad_pool_put(pm->mad_pool, p_req_madw);
>>> +
>>> + sa_mad = osm_madw_get_sa_mad_ptr(p_madw);
>>> +
>>> + num_results = (p_madw->mad_size - IB_SA_MAD_HDR_SIZE) /
>>> + (sa_mad->attr_offset << 3);
>>> +
>>> + for (i = 0; i < num_results; i++) {
>>> + ib_path_rec_t *pr = (ib_path_rec_t *)
>>> + (sa_mad->data + (i*sizeof(ib_path_rec_t)));
>>> +
>>> + /* only want reversible paths */
>>> + if ((pr->num_path & 0x80) == 0)
>>
>> Shouldn't this just be part of the query and wouldn't need checking here ?
>
> Yea that would be better.
>
>>
>>> + continue;
>>> +
>>> + insert_pr_map(pm, pr);
>>> + }
>>> +
>>> + osm_mad_pool_put(pm->mad_pool, p_madw);
>>> + pm->pr_query_outstanding = 0;
>>> +
>>> + OSM_LOG_EXIT(pm->log);
>>> +}
>>> +
>>> +static void perfmgr_sa_mad_send_err_cb(void *bind_context,
>> osm_madw_t
>>> +* p_madw) {
>>> + osm_perfmgr_t *pm = (osm_perfmgr_t *) bind_context;
>>> + OSM_LOG(pm->log, OSM_LOG_ERROR, "ERR 540D: PM PathRecord
>> query "
>>> + "failed; sm LID %u MAD TID 0x%" PRIx64 "\n",
>>> + cl_ntoh16(p_madw->mad_addr.dest_lid),
>>> + cl_ntoh64(p_madw->p_mad->trans_id));
>>> + pm->pr_query_outstanding = -1;
>>> +}
>>> +
>>> +static void create_half_world_query(osm_perfmgr_t *pm, ib_sa_mad_t
>>> +*sa_mad) {
>>> + ib_path_rec_t *pr = (ib_path_rec_t *)sa_mad->data;
>>> +
>>> + sa_mad->base_ver = 1;
>>> + sa_mad->mgmt_class = IB_MCLASS_SUBN_ADM;
>>> + sa_mad->class_ver = 2;
>>> + sa_mad->method = IB_MAD_METHOD_GETTABLE;
>>> + sa_mad->status = 0;
>>> + sa_mad->trans_id =
>>> + cl_hton64((uint64_t) cl_atomic_inc(&pm->trans_id) &
>>> + (uint64_t) (0xFFFFFFFF));
>>> + if (sa_mad->trans_id == 0)
>>> + sa_mad->trans_id =
>>> + cl_hton64((uint64_t) cl_atomic_inc(&pm->trans_id) &
>>> + (uint64_t) (0xFFFFFFFF));
>>> + sa_mad->attr_id = IB_MAD_ATTR_PATH_RECORD;
>>> + sa_mad->attr_mod = 0;
>>> +
>>> + pr->slid = osm_port_get_base_lid(osm_get_port_by_guid(pm-
>>> subn,
>>> + pm->port_guid));
>>> + sa_mad->comp_mask = IB_PR_COMPMASK_SLID;
>>
>> Although OpenSM supports this, it would be best to have this as compliant
>> query which requires NumbPath and SGID. Also, previously discussed adding
>> in Reversible.
>
> Agreed.
>
>>
>>> +}
>>> +
>>> +static int send_sa_pr_query(osm_perfmgr_t *pm) {
>>> + ib_sa_mad_t *sa_mad = NULL;
>>> + osm_madw_t *p_madw = NULL;
>>> + ib_net16_t smlid;
>>> + uint8_t smsl;
>>> +
>>> + if (get_sm_info(pm, &smlid, &smsl) < 0) {
>>> + OSM_LOG(pm->log, OSM_LOG_ERROR, "ERR 54FE: "
>>> + "PM failed to find SM LID & SL for PR query\n");
>>> + return (-1);
>>> + }
>>> +
>>> + p_madw = osm_mad_pool_get(pm->mad_pool, pm-
>>> sa_bind_handle,
>>> + MAD_BLOCK_SIZE, NULL);
>>> + if (p_madw == NULL)
>>> + return -1;
>>> +
>>> + sa_mad = osm_madw_get_sa_mad_ptr(p_madw);
>>> +
>>> + create_half_world_query(pm, sa_mad);
>>> +
>>> + p_madw->mad_addr.dest_lid = smlid;
>>> + p_madw->mad_addr.addr_type.gsi.remote_qp = 1;
>>> + p_madw->mad_addr.addr_type.gsi.remote_qkey =
>>> + cl_hton32(IB_QP1_WELL_KNOWN_Q_KEY);
>>> + p_madw->mad_addr.addr_type.gsi.pkey_ix = 0;
>>> + p_madw->mad_addr.addr_type.gsi.service_level = smsl;
>>> + p_madw->mad_addr.addr_type.gsi.global_route = FALSE;
>>> + p_madw->resp_expected = TRUE;
>>> +
>>> + if (osm_vendor_send(pm->sa_bind_handle, p_madw, TRUE)
>>> + != IB_SUCCESS)
>>> + return (-1);
>>> +
>>> + return (0);
>>> +}
>>> +
>>> +static int get_internal_pr(osm_perfmgr_t *pm) {
>>> + ib_sa_mad_t sa_mad;
>>> + const osm_alias_guid_t *p_src_alias_guid, *p_dest_alias_guid;
>>> + const osm_port_t *p_src_port, *p_dest_port;
>>> + const ib_gid_t *p_sgid = NULL, *p_dgid = NULL;
>>> + osm_port_t *p_local_port;
>>> + cl_qlist_t pr_list;
>>> + cl_list_item_t *item;
>>> + unsigned num_rec, i;
>>> +
>>> + create_half_world_query(pm, &sa_mad);
>>> +
>>
>> Doesn't the SMDB RO lock needs to be held here or is it already held when
>> reaching here ?
>
> Yes, I think so. I think I had it locked around get_internal_pr in a
> previous version but it is not now. So yes.
>
>>
>>> + osm_pr_get_end_points(&pm->osm->sa, &sa_mad,
>>> + &p_src_alias_guid, &p_dest_alias_guid,
>>> + &p_src_port, &p_dest_port,
>>> + &p_sgid, &p_dgid);
>>> +
>>> + cl_qlist_init(&pr_list);
>>> + p_local_port = osm_get_port_by_guid(pm->subn, pm->port_guid);
>>> +
>>> + /* Get all alias GUIDs for the src port */
>>> + p_src_alias_guid = (osm_alias_guid_t *) cl_qmap_head(&pm->subn-
>>> alias_port_guid_tbl);
>>> + while (p_src_alias_guid !=
>>> + (osm_alias_guid_t *)
>>> +cl_qmap_end(&pm->subn->alias_port_guid_tbl)) {
>>> +
>>> + if (osm_get_port_by_alias_guid(pm->subn,
>> p_src_alias_guid->alias_guid)
>>> + == p_src_port) {
>>
>> Only base port GUIDs need checking.
>
> So you just need this?
>
> osm_pr_process_half(&pm->osm->sa, &sa_mad, p_local_port,
> p_src_alias_guid,
> p_dest_alias_guid,
> p_sgid, p_dgid, &pr_list);
>
> No loop?
A loop is needed to filter the response and only use the base port GUIDs.
>>
>>> + osm_pr_process_half(&pm->osm->sa, &sa_mad,
>> p_local_port,
>>> + p_src_alias_guid,
>> p_dest_alias_guid,
>>> + p_sgid, p_dgid, &pr_list);
>>> + }
>>> +
>>> + p_src_alias_guid = (osm_alias_guid_t *)
>> cl_qmap_next(&p_src_alias_guid->map_item);
>>> + }
>>> +
>>
>> SMDB RO lock would be released here if it's grabbed above.
>
> Yes
>
>>
>>> + num_rec = cl_qlist_count(&pr_list);
>>> + for (i = 0; i < num_rec; i++) {
>>> + ib_path_rec_t *pr;
>>> + item = cl_qlist_remove_head(&pr_list);
>>> + pr = (ib_path_rec_t *)((osm_sa_item_t *)item)->resp.data;
>>> +
>>> + /* only want reversible paths */
>>> + if ((pr->num_path & 0x80) == 0)
>>> + continue;
>>> +
>>> + insert_pr_map(pm, pr);
>>> + free(item);
>>> + }
>>> + pm->pr_query_outstanding = 0;
>>> + return (0);
>>> +}
>>> +
>>> +static ib_path_rec_t * get_pr_from_pr_map(osm_perfmgr_t *pm,
>>> +ib_net16_t dlid) {
>>> + cl_map_item_t *mi;
>>> + if ((mi = cl_qmap_get(&pm->path_rec_map, (uint64_t)dlid)) !=
>>> + cl_qmap_end(&pm->path_rec_map)) {
>>> + pr_map_item_t *pr = (pr_map_item_t *)mi;
>>> + return (&pr->pr);
>>> + }
>>> + return (NULL);
>>> +}
>>> +
>>> +static int8_t get_sl(osm_perfmgr_t *pm, monitored_node_t *
>> mon_node,
>>> +uint8_t port) {
>>> + uint16_t dlid;
>>> + ib_path_rec_t *pr;
>>> +
>>> + if (!mon_node || port >= mon_node->num_ports)
>>> + return (-1);
>>> +
>>> + dlid = mon_node->port[port].redirection ?
>>> + mon_node->port[port].lid :
>>> + mon_node->port[port].orig_lid;
>>> + pr = get_pr_from_pr_map(pm, dlid);
>>> + if (pr) {
>>> + OSM_LOG(pm->log, OSM_LOG_DEBUG, "PM %s port %d ->
>> SL 0x%x\n",
>>> + mon_node->name, port, ib_path_rec_sl(pr));
>>> + return ((int8_t)ib_path_rec_sl(pr));
>>> + }
>>> +
>>> + OSM_LOG(pm->log, OSM_LOG_ERROR, "ERR 54FD: "
>>> + "PM failed to find SL for %s port %d\n",
>>> + mon_node->name, port);
>>> + return (-1);
>>> +}
>>> +
>>> static ib_net16_t get_base_lid(osm_node_t * p_node, uint8_t port) {
>>> switch (p_node->node_info.node_type) { @@ -683,6 +953,7 @@
>> static
>>> void perfmgr_query_counters(cl_map_item_t * p_map_item, void
>> *context)
>>> /* issue the query for each port */
>>> for (port = mon_node->esp0 ? 0 : 1; port < num_ports; port++) {
>>> ib_net16_t lid;
>>> + int8_t sl;
>>>
>>> if (!osm_node_get_physp_ptr(node, port))
>>> continue;
>>> @@ -700,6 +971,9 @@ static void perfmgr_query_counters(cl_map_item_t
>> * p_map_item, void *context)
>>> }
>>>
>>> remote_qp = get_qp(mon_node, port);
>>> + sl = get_sl(pm, mon_node, port);
>>> + if (sl < 0)
>>> + continue;
>>>
>>> mad_context.perfmgr_context.node_guid = node_guid;
>>> mad_context.perfmgr_context.port = port; @@ -709,7
>> +983,7 @@ static
>>> void perfmgr_query_counters(cl_map_item_t * p_map_item, void
>> *context)
>>> status = perfmgr_send_cpi_mad(pm, lid, remote_qp,
>>> mon_node-
>>> port[port].pkey_ix,
>>> port, &mad_context,
>>> - 0); /* FIXME SL != 0 */
>>> + sl);
>>> if (status != IB_SUCCESS)
>>> OSM_LOG(pm->log, OSM_LOG_ERROR, "ERR
>> 5410: "
>>> "Failed to issue ClassPortInfo query "
>>> @@ -733,7 +1007,7 @@ static void
>> perfmgr_query_counters(cl_map_item_t * p_map_item, void *context)
>>> port,
>> IB_MAD_METHOD_GET,
>>> 0xffff,
>>> &mad_context,
>>> - 0); /* FIXME SL != 0 */
>>> + sl);
>>> if (status != IB_SUCCESS)
>>> OSM_LOG(pm->log, OSM_LOG_ERROR, "ERR
>> 5409: "
>>> "Failed to issue port counter query
>> for node 0x%"
>>> @@ -751,7 +1025,7 @@ static void
>> perfmgr_query_counters(cl_map_item_t * p_map_item, void *context)
>>> port,
>>>
>> IB_MAD_METHOD_GET,
>>> &mad_context,
>>> - 0); /* FIXME SL !=
>> 0 */
>>> + sl);
>>> if (status != IB_SUCCESS)
>>> OSM_LOG(pm->log,
>> OSM_LOG_ERROR,
>>> "ERR 5417: Failed to issue "
>>> @@ -1007,8 +1281,7 @@ void osm_perfmgr_process(osm_perfmgr_t *
>> pm)
>>> pm->sweep_state = PERFMGR_SWEEP_ACTIVE;
>>> cl_spinlock_release(&pm->lock);
>>>
>>> - if (pm->subn->sm_state == IB_SMINFO_STATE_STANDBY ||
>>> - pm->subn->sm_state == IB_SMINFO_STATE_NOTACTIVE)
>>> + if (sm_not_active(pm))
>>> perfmgr_discovery(pm->subn->p_osm);
>>>
>>> /* if redirection enabled, determine local port */ @@ -1034,6
>>> +1307,18 @@ void osm_perfmgr_process(osm_perfmgr_t * pm) #ifdef
>>> ENABLE_OSM_PERF_MGR_PROFILE
>>> gettimeofday(&before, NULL);
>>> #endif
>>> + pm->pr_query_outstanding = 1;
>>> + if (sm_not_active(pm)) {
>>> + /* FIXME register for UnPath/RePath rather than reissue
>> query */
>>> + if (send_sa_pr_query(pm)) {
>>> + OSM_LOG(pm->log, OSM_LOG_ERROR, "ERR 542F: "
>>> + "PM PathRecord query send
>> failed\n");
>>> + pm->pr_query_outstanding = -1;
>>> + }
>>> + } else {
>>> + get_internal_pr(pm);
>>> + }
>>> +
>>> /* With the global lock held, collect the node guids */
>>> /* FIXME we should be able to track SA notices
>>> * and not have to sweep the node_guid_tbl each pass @@ -1043,8
>>> +1328,15 @@ void osm_perfmgr_process(osm_perfmgr_t * pm)
>>> cl_qmap_apply_func(&pm->subn->node_guid_tbl, collect_guids,
>> pm);
>>> cl_plock_release(&pm->osm->lock);
>>>
>>> - /* then for each node query their counters */
>>> - cl_qmap_apply_func(&pm->monitored_map,
>> perfmgr_query_counters, pm);
>>> + /* Wait for PR query to complete */
>>> + while(pm->pr_query_outstanding == 1)
>>> + ;
>>
>> A better way to wait for PR response/timeout should be used.
>
> What is most concerning, the fear of an endless loop, or the busy waiting?
I was concerned with the busy waiting.
> In the former the vendor mad layer would have to fail.
> For the later we could add sleeps but that seemed inefficient.
> Adding another timer also seems like a waste since in the stand alone mode we
> are simply waiting for the query to return and in the integrated mode this is
> effectively a no-op.
In standalone mode, aren't there still other threads ? While this thread
is busy waiting, what are the other threads doing ?
-- Hal
> Ira
>
>>
>> -- Hal
>>
>>> +
>>> + if (pm->pr_query_outstanding == 0) {
>>> + cl_qmap_apply_func(&pm->monitored_map,
>> perfmgr_query_counters, pm);
>>> + } else {
>>> + pm->pr_query_outstanding = 0;
>>> + }
>>>
>>> /* clean out any nodes found to be removed during the sweep */
>>> remove_marked_nodes(pm);
>>> @@ -1235,6 +1527,7 @@ static void
>> perfmgr_check_overflow(osm_perfmgr_t * pm,
>>> counter_overflow_32(pc->rcv_pkts)))) {
>>> osm_node_t *p_node = NULL;
>>> ib_net16_t lid = 0;
>>> + int8_t sl;
>>>
>>> if (!mon_node->port[port].valid)
>>> goto Exit;
>>> @@ -1244,6 +1537,9 @@ static void
>> perfmgr_check_overflow(osm_perfmgr_t * pm,
>>> ") port %d; clearing counters\n",
>>> mon_node->name, mon_node->guid, port);
>>>
>>> + if ((sl = get_sl(pm, mon_node, port)) < 0)
>>> + goto Exit;
>>> +
>>> cl_plock_acquire(&pm->osm->lock);
>>> p_node =
>>> osm_get_node_by_guid(pm->subn,
>> cl_hton64(mon_node->guid)); @@
>>> -1276,8 +1572,7 @@ static void perfmgr_check_overflow(osm_perfmgr_t *
>> pm,
>>> status = perfmgr_send_pc_mad(pm, lid, remote_qp,
>> pkey_ix,
>>> port, IB_MAD_METHOD_SET,
>>> counter_select,
>>> - &mad_context,
>>> - 0); /* FIXME SL != 0 */
>>> + &mad_context, sl);
>>> if (status != IB_SUCCESS)
>>> OSM_LOG(pm->log, OSM_LOG_ERROR, "PerfMgr:
>> ERR 5411: "
>>> "Failed to send clear counters MAD for %s
>> (0x%"
>>> @@ -1320,6 +1615,7 @@ static void
>> perfmgr_check_pce_overflow(osm_perfmgr_t * pm,
>>> counter_overflow_64(pc->multicast_rcv_pkts)))) {
>>> osm_node_t *p_node = NULL;
>>> ib_net16_t lid = 0;
>>> + int8_t sl;
>>>
>>> if (!mon_node->port[port].valid)
>>> goto Exit;
>>> @@ -1329,6 +1625,9 @@ static void
>> perfmgr_check_pce_overflow(osm_perfmgr_t * pm,
>>> PRIx64 ") port %d; clearing counters\n",
>>> mon_node->name, mon_node->guid, port);
>>>
>>> + if ((sl = get_sl(pm, mon_node, port)) < 0)
>>> + goto Exit;
>>> +
>>> cl_plock_acquire(&pm->osm->lock);
>>> p_node =
>>> osm_get_node_by_guid(pm->subn,
>> cl_hton64(mon_node->guid)); @@
>>> -1350,8 +1649,7 @@ static void
>> perfmgr_check_pce_overflow(osm_perfmgr_t * pm,
>>> /* clear port counters */
>>> status = perfmgr_send_pce_mad(pm, lid, remote_qp,
>> pkey_ix,
>>> port, IB_MAD_METHOD_SET,
>>> - &mad_context,
>>> - 0); /* FIXME SL != 0 */
>>> + &mad_context, sl);
>>> if (status != IB_SUCCESS)
>>> OSM_LOG(pm->log, OSM_LOG_ERROR, "PerfMgr:
>> ERR 5419: "
>>> "Failed to send clear counters MAD for %s
>> (0x%"
>>> @@ -1475,6 +1773,7 @@ static boolean_t handle_redirect(osm_perfmgr_t
>> *pm,
>>> boolean_t valid = TRUE;
>>> int16_t pkey_ix = 0;
>>> uint8_t mad_method;
>>> + int8_t sl;
>>>
>>> OSM_LOG(pm->log, OSM_LOG_VERBOSE,
>>> "Redirection to LID %u GID %s QP 0x%x received\n", @@ -
>> 1528,6
>>> +1827,12 @@ static boolean_t handle_redirect(osm_perfmgr_t *pm,
>>> if (!valid)
>>> goto Exit;
>>>
>>> + sl = get_sl(pm, p_mon_node, port);
>>> + if (sl < 0) {
>>> + valid = FALSE;
>>> + goto Exit;
>>> + }
>>> +
>>> /* LID redirection support (easier than GID redirection) */
>>> cl_plock_acquire(&pm->osm->lock);
>>> p_mon_node->port[port].redirection = TRUE; @@ -1550,7 +1855,7
>> @@
>>> static boolean_t handle_redirect(osm_perfmgr_t *pm,
>>> status = perfmgr_send_cpi_mad(pm, cpi->redir_lid,
>>> cpi->redir_qp, pkey_ix,
>>> port, mad_context,
>>> - 0); /* FIXME SL != 0 */
>>> + sl);
>>> } else {
>>> /* reissue the original query to the redirected location */
>>> mad_method = mad_context-
>>> perfmgr_context.mad_method;
>>> @@ -1561,14 +1866,14 @@ static boolean_t
>> handle_redirect(osm_perfmgr_t *pm,
>>> mad_method,
>>> 0xffff,
>>> mad_context,
>>> - 0); /* FIXME SL != 0 */
>>> + sl);
>>> } else {
>>> status = perfmgr_send_pce_mad(pm, cpi->redir_lid,
>>> cpi->redir_qp,
>>> pkey_ix, port,
>>> mad_method,
>>> mad_context,
>>> - 0); /* FIXME SL != 0 */
>>> + sl);
>>> }
>>> }
>>> if (status != IB_SUCCESS)
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the
>> body of a message to [email protected] More majordomo info at
>> http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
