On 11/27/2013 9:42 AM, Line Holen wrote: > On 11/27/13 13:16, Hal Rosenstock wrote: >> On 11/15/2013 7:15 AM, Line Holen wrote: >>> The retry counter is now only updated if a packet is actually sent. >>> (But as before the initial request is also counted.) >>> >>> Prior to this change the actual maximum number of packets sent were >>> polling retry number minus one. >>> >>> Signed-off-by: Line Holen<[email protected]> >>> >>> --- >>> >>> diff --git a/opensm/osm_sm_state_mgr.c b/opensm/osm_sm_state_mgr.c >>> index 596ad8f..6eff9ee 100644 >>> --- a/opensm/osm_sm_state_mgr.c >>> +++ b/opensm/osm_sm_state_mgr.c >>> @@ -197,16 +197,14 @@ void osm_sm_state_mgr_polling_callback(IN void >>> *context) >>> } >>> >>> /* >>> - * Incr the retry number. >>> - * If it reached the max_retry_number in the subnet opt - call >>> + * If retry number reached the max_retry_number in the subnet >>> opt - call >>> * osm_sm_state_mgr_process with signal >>> OSM_SM_SIGNAL_POLLING_TIMEOUT >>> */ >>> - sm->retry_number++; >>> OSM_LOG(sm->p_log, OSM_LOG_VERBOSE, "SM State %d (%s), Retry >>> number:%d\n", >>> sm->p_subn->sm_state, >>> osm_get_sm_mgr_state_str(sm->p_subn->sm_state), >>> sm->retry_number); >>> >>> - if (sm->retry_number>= sm->p_subn->opt.polling_retry_number) { >>> + if (sm->retry_number> sm->p_subn->opt.polling_retry_number) { >>> OSM_LOG(sm->p_log, OSM_LOG_DEBUG, >>> "Reached polling_retry_number value in retry_number. " >>> "Go to DISCOVERY state\n"); >>> @@ -214,6 +212,9 @@ void osm_sm_state_mgr_polling_callback(IN void >>> *context) >>> goto Exit; >>> } >>> >>> + /* Increment the retry number */ >>> + sm->retry_number++; >> Would it be better to increment retry number if >> sm_state_mgr_send_master_sm_info_req call just below this succeeds ? >> >> -- Hal > I'm not sure really.
All I was proposing was a minor variation to what you proposed: to add a status return to sm_state_mgr_send_master_sm_info_req and only increment the retry_number if that call was "successful". > The current placement was to avoid potential race > with response handling > and the clearing of the counter there (incrementing after the response > were received). Maybe I'm missing something but I don't see how this changes any potential race condition other than perhaps a smaller time window. > Seemed to me that this could happen with the current locking. Yes, it looks to me like the locking here needs fixing. I'll send a patch for this shortly... -- Hal > > Line >> >>> + >>> /* Send a SubnGet(SMInfo) request to the remote sm (depends on >>> our state) */ >>> sm_state_mgr_send_master_sm_info_req(sm); >>> >>> -- >>> 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
