Hi Sasha, This is an important improvement to the previous algorithm. I read through but am not sure this does not break the logic. I hope we can get the random test flow written soon - to gain confidence. But since we did not thoroughly test the previous one - it is OK to commit this in my mind.
Thanks Eitan Zahavi Design Technology Director Mellanox Technologies LTD Tel:+972-4-9097208 Fax:+972-4-9593245 P.O. Box 586 Yokneam 20692 ISRAEL > -----Original Message----- > From: Sasha Khapyorsky [mailto:[EMAIL PROTECTED] > Sent: Sunday, April 23, 2006 5:26 PM > To: Hal Rosenstock > Cc: [email protected]; Eitan Zahavi; Ofer Gigi; Yael Kalka > Subject: [PATCH 3/4] opensm: pkey manager performance improvement > > > Send changed pkey table blocks to ports only after full update and not > after each pkey value change/update. > > Signed-off-by: Sasha Khapyorsky <[EMAIL PROTECTED]> > --- > > osm/include/opensm/osm_pkey.h | 51 +++++++++ > osm/opensm/osm_pkey.c | 32 ++++++ > osm/opensm/osm_pkey_mgr.c | 233 ++++++++++++++++++++--------------------- > 3 files changed, 197 insertions(+), 119 deletions(-) > > diff --git a/osm/include/opensm/osm_pkey.h b/osm/include/opensm/osm_pkey.h > index d4ee9a1..f5e8c11 100644 > --- a/osm/include/opensm/osm_pkey.h > +++ b/osm/include/opensm/osm_pkey.h > @@ -90,16 +90,28 @@ struct _osm_physp; > typedef struct _osm_pkey_tbl > { > cl_ptr_vector_t blocks; > + cl_ptr_vector_t new_blocks; > cl_map_t keys; > } osm_pkey_tbl_t; > /* > * FIELDS > * blocks > -* The IBA defined blocks of pkey values > +* The IBA defined blocks of pkey values, updated from the net > +* > +* new_blocks > +* The blocks of pkey values, will be used for updates by SM > * > * keys > * A set holding all keys > * > +* NOTES > +* 'blocks' vector should be used to store pkey values obtained from > +* the port and SM pkey manager should not change it directly, for this > +* purpose 'new_blocks' should be used. > +* > +* The only pkey values stored in 'blocks' vector will be mapped with > +* 'keys' map > +* > *********/ > > /****f* OpenSM: osm_pkey_tbl_construct > @@ -214,6 +226,43 @@ static inline ib_pkey_table_t *osm_pkey_ > * > *********/ > > +/****f* OpenSM: osm_pkey_tbl_new_block_get > +* NAME > +* osm_pkey_tbl_new_block_get > +* > +* DESCRIPTION > +* The same as above but for new block > +* > +* SYNOPSIS > +*/ > +static inline ib_pkey_table_t *osm_pkey_tbl_new_block_get( > + const osm_pkey_tbl_t *p_pkey_tbl, uint16_t block) > +{ > + return (block < cl_ptr_vector_get_size(&p_pkey_tbl->new_blocks)) ? > + cl_ptr_vector_get(&p_pkey_tbl->new_blocks, block) : NULL; > +}; > +/* > + *********/ > + > +/****f* OpenSM: osm_pkey_tbl_sync_new_blocks > +* NAME > +* osm_pkey_tbl_sync_new_blocks > +* > +* DESCRIPTION > +* Syncs new_blocks vector content with current pkey table blocks > +* > +* SYNOPSIS > +*/ > +void osm_pkey_tbl_sync_new_blocks( > + const osm_pkey_tbl_t *p_pkey_tbl); > +/* > +* p_pkey_tbl > +* [in] Pointer to osm_pkey_tbl_t object. > +* > +* NOTES > +* > +*********/ > + > /****f* OpenSM: osm_pkey_tbl_set > * NAME > * osm_pkey_tbl_set > diff --git a/osm/opensm/osm_pkey.c b/osm/opensm/osm_pkey.c > index 5a4ca0d..d661bd6 100644 > --- a/osm/opensm/osm_pkey.c > +++ b/osm/opensm/osm_pkey.c > @@ -67,6 +67,7 @@ void osm_pkey_tbl_construct( > IN osm_pkey_tbl_t *p_pkey_tbl) > { > cl_ptr_vector_construct( &p_pkey_tbl->blocks ); > + cl_ptr_vector_construct( &p_pkey_tbl->new_blocks ); > cl_map_construct( &p_pkey_tbl->keys ); > } > > @@ -82,6 +83,11 @@ void osm_pkey_tbl_destroy( > cl_free(cl_ptr_vector_get( &p_pkey_tbl->blocks, i )); > cl_ptr_vector_destroy( &p_pkey_tbl->blocks ); > > + num_blocks = (uint16_t)(cl_ptr_vector_get_size( &p_pkey_tbl->new_blocks )); > + for (i = 0; i < num_blocks; i++) > + cl_free(cl_ptr_vector_get( &p_pkey_tbl->new_blocks, i )); > + cl_ptr_vector_destroy( &p_pkey_tbl->new_blocks ); > + > cl_map_remove_all( &p_pkey_tbl->keys ); > cl_map_destroy( &p_pkey_tbl->keys ); > } > @@ -92,12 +98,38 @@ int osm_pkey_tbl_init( > IN osm_pkey_tbl_t *p_pkey_tbl) > { > cl_ptr_vector_init( &p_pkey_tbl->blocks, 0, 1); > + cl_ptr_vector_init( &p_pkey_tbl->new_blocks, 0, 1); > cl_map_init( &p_pkey_tbl->keys, 1 ); > return(IB_SUCCESS); > } > > /********************************************************************** > **********************************************************************/ > +void osm_pkey_tbl_sync_new_blocks( > + IN const osm_pkey_tbl_t *p_pkey_tbl) > +{ > + ib_pkey_table_t *p_block, *p_new_block; > + int16_t b, num_blocks, new_blocks; > + > + num_blocks = cl_ptr_vector_get_size(&p_pkey_tbl->blocks); > + new_blocks = cl_ptr_vector_get_size(&p_pkey_tbl->new_blocks); > + > + for (b = 0; b < num_blocks; b++) { > + p_block = cl_ptr_vector_get(&p_pkey_tbl->blocks, b); > + if ( b < new_blocks ) > + p_new_block = cl_ptr_vector_get(&p_pkey_tbl->new_blocks, b); > + else { > + p_new_block = (ib_pkey_table_t *)cl_zalloc(sizeof(*p_new_block)); > + if (!p_new_block) > + break; > + cl_ptr_vector_set(&((osm_pkey_tbl_t *)p_pkey_tbl)->new_blocks, b, > p_new_block); > + } > + cl_memcpy(p_new_block, p_block, sizeof(*p_new_block)); > + } > +} > + > +/********************************************************************** > + **********************************************************************/ > int osm_pkey_tbl_set( > IN osm_pkey_tbl_t *p_pkey_tbl, > IN uint16_t block, > diff --git a/osm/opensm/osm_pkey_mgr.c b/osm/opensm/osm_pkey_mgr.c > index 7b3da26..da8dfa8 100644 > --- a/osm/opensm/osm_pkey_mgr.c > +++ b/osm/opensm/osm_pkey_mgr.c > @@ -131,86 +131,45 @@ pkey_mgr_enforce_partition( > **********************************************************************/ > > /* > - * Send a new entry for the pkey table for this port when this pkey > + * Prepare a new entry for the pkey table for this port when this pkey > * does not exist. Update existed entry when membership was changed. > */ > > -static boolean_t > -pkey_mgr_process_physical_port( > +static void pkey_mgr_process_physical_port( > IN osm_log_t *p_log, > IN const osm_req_t *p_req, > IN const ib_net16_t pkey, > IN osm_physp_t *p_physp ) > { > - boolean_t return_val = FALSE; /* TRUE if pkey was inserted or updated */ > - ib_api_status_t status; > osm_node_t *p_node = osm_physp_get_node_ptr( p_physp ); > - ib_pkey_table_t *block = NULL; > + ib_pkey_table_t *block; > uint16_t block_index; > uint16_t num_of_blocks; > const osm_pkey_tbl_t *p_pkey_tbl; > ib_net16_t *p_orig_pkey; > + char *stat = NULL; > uint32_t i; > - boolean_t block_found = FALSE; > - > - OSM_LOG_ENTER( p_log, pkey_mgr_process_physical_port ); > > p_pkey_tbl = osm_physp_get_pkey_tbl( p_physp ); > num_of_blocks = osm_pkey_tbl_get_num_blocks( p_pkey_tbl ); > > p_orig_pkey = cl_map_get( &p_pkey_tbl->keys, ib_pkey_get_base( pkey ) ); > > - if ( p_orig_pkey && *p_orig_pkey == pkey ) > - { > - if ( osm_log_is_active( p_log, OSM_LOG_VERBOSE ) ) > - { > - osm_log( p_log, OSM_LOG_VERBOSE, > - "pkey_mgr_process_physical_port: " > - "No need to insert pkey 0x%04x for node 0x%016" PRIx64 > - " port %u\n", > - cl_ntoh16( pkey ), > - cl_ntoh64( osm_node_get_node_guid( p_node ) ), > - osm_physp_get_port_num( p_physp ) ); > - } > - goto _done; > - } > - else if ( !p_orig_pkey ) > + if ( !p_orig_pkey ) > { > for ( block_index = 0; block_index < num_of_blocks; block_index++ ) > { > - block = osm_pkey_tbl_block_get( p_pkey_tbl, block_index ); > + block = osm_pkey_tbl_new_block_get( p_pkey_tbl, block_index ); > for ( i = 0; i < IB_NUM_PKEY_ELEMENTS_IN_BLOCK; i++ ) > { > if ( ib_pkey_is_invalid( block->pkey_entry[i] ) ) > { > block->pkey_entry[i] = pkey; > - block_found = TRUE; > - break; > + stat = "inserted"; > + goto _done; > } > } > - if ( block_found ) > - { > - break; > - } > } > - } > - else > - { > - *p_orig_pkey = pkey; > - for ( block_index = 0; block_index < num_of_blocks; block_index++ ) > - { > - block = osm_pkey_tbl_block_get( p_pkey_tbl, block_index ); > - i = p_orig_pkey - block->pkey_entry; > - if ( i < IB_NUM_PKEY_ELEMENTS_IN_BLOCK ) > - { > - block_found = TRUE; > - break; > - } > - } > - } > - > - if ( block_found == FALSE ) > - { > osm_log( p_log, OSM_LOG_ERROR, > "pkey_mgr_process_physical_port: ERR 0501: " > "No empty pkey entry was found to insert 0x%04x for node " > @@ -218,46 +177,40 @@ pkey_mgr_process_physical_port( > cl_ntoh16( pkey ), > cl_ntoh64( osm_node_get_node_guid( p_node ) ), > osm_physp_get_port_num( p_physp ) ); > - goto _done; > } > - > - status = > - pkey_mgr_update_pkey_entry( p_req, p_physp, block, block_index ); > - > - if ( status != IB_SUCCESS ) > + else if ( *p_orig_pkey != pkey ) > { > - osm_log( p_log, OSM_LOG_ERROR, > - "pkey_mgr_process_physical_port: " > - "pkey_mgr_update_pkey_entry() failed to update " > - "pkey table block %d for node 0x%016" PRIx64 " port %u\n", > - block_index, > - cl_ntoh64( osm_node_get_node_guid( p_node ) ), > - osm_physp_get_port_num( p_physp ) ); > - goto _done; > + for ( block_index = 0; block_index < num_of_blocks; block_index++ ) > + { > + /* we need real block (not just new_block) in order > + * to resolve block/pkey indices */ > + block = osm_pkey_tbl_block_get( p_pkey_tbl, block_index ); > + i = p_orig_pkey - block->pkey_entry; > + if (i < IB_NUM_PKEY_ELEMENTS_IN_BLOCK) { > + block = osm_pkey_tbl_new_block_get( p_pkey_tbl, block_index ); > + block->pkey_entry[i] = pkey; > + stat = "updated"; > + goto _done; > + } > + } > } > > - return_val = TRUE; /* pkey was inserted/updated */ > - > - if ( osm_log_is_active( p_log, OSM_LOG_VERBOSE ) ) > - { > + _done: > + if (stat) { > osm_log( p_log, OSM_LOG_VERBOSE, > "pkey_mgr_process_physical_port: " > - "pkey 0x%04x was inserted for node 0x%016" PRIx64 > + "pkey 0x%04x was %s for node 0x%016" PRIx64 > " port %u\n", > - cl_ntoh16( pkey ), > + cl_ntoh16( pkey ), stat, > cl_ntoh64( osm_node_get_node_guid( p_node ) ), > osm_physp_get_port_num( p_physp ) ); > } > - > - _done: > - OSM_LOG_EXIT( p_log ); > - return ( return_val ); > } > > > /********************************************************************** > **********************************************************************/ > -static void > +static boolean_t > pkey_mgr_update_peer_port( > osm_log_t *p_log, > const osm_req_t *p_req, > @@ -274,21 +227,22 @@ pkey_mgr_update_peer_port( > uint16_t block_index; > uint16_t num_of_blocks; > ib_api_status_t status = IB_SUCCESS; > + boolean_t ret_val = FALSE; > > p = osm_port_get_default_phys_ptr( p_port ); > if ( !osm_physp_is_valid( p ) ) > - return; > + return FALSE; > peer = osm_physp_get_remote( p ); > if ( !peer || !osm_physp_is_valid( peer ) ) > - return; > + return FALSE; > p_node = osm_physp_get_node_ptr( peer ); > if ( osm_node_get_type( p_node ) != IB_NODE_TYPE_SWITCH ) > - return; > + return FALSE; > > p_sw = osm_get_switch_by_guid( p_subn, osm_node_get_node_guid( p_node )); > if (!p_sw || !(p_si = osm_switch_get_si_ptr( p_sw )) || > !p_si->enforce_cap) > - return; > + return FALSE; > > if (pkey_mgr_enforce_partition( p_req, peer, enforce ) != IB_SUCCESS) { > osm_log( p_log, OSM_LOG_ERROR, > @@ -300,7 +254,7 @@ pkey_mgr_update_peer_port( > } > > if (enforce == FALSE) > - return; > + return FALSE; > > p_pkey_tbl = osm_physp_get_pkey_tbl( p ); > p_peer_pkey_tbl = osm_physp_get_pkey_tbl( peer ); > @@ -310,15 +264,15 @@ pkey_mgr_update_peer_port( > > for ( block_index = 0; block_index < num_of_blocks; block_index++ ) > { > - block = osm_pkey_tbl_block_get( p_pkey_tbl, block_index ); > + block = osm_pkey_tbl_new_block_get( p_pkey_tbl, block_index ); > peer_block = osm_pkey_tbl_block_get( p_peer_pkey_tbl, block_index ); > - if ( cl_memcmp( peer_block, block, sizeof( *block ) ) ) > + if ( cl_memcmp( peer_block, block, sizeof( *peer_block ) ) ) > { > - cl_memcpy( peer_block, block, sizeof( *block ) ); > status = > - pkey_mgr_update_pkey_entry( p_req, peer, peer_block, > - block_index ); > - if ( status != IB_SUCCESS ) > + pkey_mgr_update_pkey_entry( p_req, peer, block, block_index ); > + if ( status == IB_SUCCESS ) > + ret_val = TRUE; > + else > osm_log( p_log, OSM_LOG_ERROR, > "pkey_mgr_update_peer_port: " > "pkey_mgr_update_pkey_entry() failed to update " > @@ -330,7 +284,7 @@ pkey_mgr_update_peer_port( > } > } > > - if ( num_of_blocks && status == IB_SUCCESS && > + if ( ret_val == TRUE && > osm_log_is_active( p_log, OSM_LOG_VERBOSE ) ) > { > osm_log( p_log, OSM_LOG_VERBOSE, > @@ -340,11 +294,61 @@ pkey_mgr_update_peer_port( > cl_ntoh64( osm_node_get_node_guid( p_node ) ), > osm_physp_get_port_num( peer ) ); > } > + > + return ret_val; > } > > /********************************************************************** > **********************************************************************/ > -static boolean_t > +static boolean_t pkey_mgr_update_port( > + osm_log_t *p_log, > + osm_req_t *p_req, > + const osm_port_t * const p_port ) > +{ > + osm_physp_t *p; > + osm_node_t *p_node; > + ib_pkey_table_t *block, *new_block; > + const osm_pkey_tbl_t *p_pkey_tbl; > + uint16_t block_index; > + uint16_t num_of_blocks; > + ib_api_status_t status; > + boolean_t ret_val = FALSE; > + > + p = osm_port_get_default_phys_ptr( p_port ); > + if ( !osm_physp_is_valid( p ) ) > + return FALSE; > + > + p_pkey_tbl = osm_physp_get_pkey_tbl(p); > + num_of_blocks = osm_pkey_tbl_get_num_blocks( p_pkey_tbl ); > + > + for ( block_index = 0; block_index < num_of_blocks; block_index++ ) > + { > + block = osm_pkey_tbl_block_get( p_pkey_tbl, block_index ); > + new_block = osm_pkey_tbl_new_block_get( p_pkey_tbl, block_index ); > + > + if (!new_block || !cl_memcmp( new_block, block, sizeof( *block ) ) ) > + continue; > + > + status = > + pkey_mgr_update_pkey_entry( p_req, p, new_block, block_index ); > + if (status == IB_SUCCESS) > + ret_val = TRUE; > + else > + osm_log( p_log, OSM_LOG_ERROR, > + "pkey_mgr_update_port: " > + "pkey_mgr_update_pkey_entry() failed to update " > + "pkey table block %d for node 0x%016" PRIx64 " port %u\n", > + block_index, > + cl_ntoh64( osm_node_get_node_guid( p_node ) ), > + osm_physp_get_port_num( p ) ); > + } > + > + return ret_val; > +} > + > +/********************************************************************** > + **********************************************************************/ > +static void > pkey_mgr_process_partition_table( > osm_log_t *p_log, > const osm_req_t *p_req, > @@ -356,7 +360,6 @@ pkey_mgr_process_partition_table( > cl_map_iterator_t i, i_next; > ib_net16_t pkey = p_prtn->pkey; > osm_physp_t *p_physp; > - boolean_t result = FALSE; > > if ( full ) > pkey = cl_hton16( cl_ntoh16( pkey ) | 0x8000 ); > @@ -367,23 +370,9 @@ pkey_mgr_process_partition_table( > i = i_next; > i_next = cl_map_next( i ); > p_physp = cl_map_obj( i ); > - if ( p_physp && osm_physp_is_valid( p_physp ) && > - pkey_mgr_process_physical_port( p_log, p_req, pkey, p_physp ) ) > - { > - result = TRUE; > - if ( osm_log_is_active( p_log, OSM_LOG_VERBOSE ) ) > - osm_log( p_log, OSM_LOG_VERBOSE, > - "pkey_mgr_process_partition_table: " > - "Adding 0x%04x to pkey table of node " > - "0x%016" PRIx64 " port %u\n", > - cl_ntoh16( pkey ), > - cl_ntoh64( osm_node_get_node_guid > - ( osm_physp_get_node_ptr( p_physp ) ) ), > - osm_physp_get_port_num( p_physp ) ); > - } > + if ( p_physp && osm_physp_is_valid( p_physp ) ) > + pkey_mgr_process_physical_port( p_log, p_req, pkey, p_physp ); > } > - > - return result; > } > > /********************************************************************** > @@ -397,6 +386,7 @@ osm_pkey_mgr_process( > osm_prtn_t *p_prtn; > osm_port_t *p_port; > osm_signal_t signal = OSM_SIGNAL_DONE; > + osm_physp_t *p_physp; > > CL_ASSERT( p_osm ); > > @@ -411,34 +401,41 @@ osm_pkey_mgr_process( > goto _err; > } > > - p_tbl = &p_osm->subn.prtn_pkey_tbl; > + p_tbl = &p_osm->subn.port_guid_tbl; > + p_next = cl_qmap_head( p_tbl ); > + while ( p_next != cl_qmap_end( p_tbl ) ) > + { > + p_port = ( osm_port_t * ) p_next; > + p_next = cl_qmap_next( p_next ); > + p_physp = osm_port_get_default_phys_ptr( p_port ); > + if (osm_physp_is_valid( p_physp ) ) > + osm_pkey_tbl_sync_new_blocks(osm_physp_get_pkey_tbl(p_physp)); > + } > > + p_tbl = &p_osm->subn.prtn_pkey_tbl; > p_next = cl_qmap_head( p_tbl ); > while ( p_next != cl_qmap_end( p_tbl ) ) > { > p_prtn = ( osm_prtn_t * ) p_next; > p_next = cl_qmap_next( p_next ); > - > - if ( pkey_mgr_process_partition_table( &p_osm->log, &p_osm->sm.req, p_prtn, > FALSE ) ) > - signal = OSM_SIGNAL_DONE_PENDING; > - if ( pkey_mgr_process_partition_table( &p_osm->log, &p_osm->sm.req, p_prtn, > TRUE ) ) > - signal = OSM_SIGNAL_DONE_PENDING; > + pkey_mgr_process_partition_table( &p_osm->log, &p_osm->sm.req, p_prtn, > FALSE ); > + pkey_mgr_process_partition_table( &p_osm->log, &p_osm->sm.req, p_prtn, > TRUE ); > } > > p_tbl = &p_osm->subn.port_guid_tbl; > - > p_next = cl_qmap_head( p_tbl ); > while ( p_next != cl_qmap_end( p_tbl ) ) > { > p_port = ( osm_port_t * ) p_next; > p_next = cl_qmap_next( p_next ); > - > - if ( osm_node_get_type( osm_port_get_parent_node( p_port ) ) != > - IB_NODE_TYPE_SWITCH ) > - { > - pkey_mgr_update_peer_port( &p_osm->log, &p_osm->sm.req, &p_osm->subn, > - p_port, !p_osm->subn.opt.no_partition_enforcement ); > - } > + if (pkey_mgr_update_port(&p_osm->log, &p_osm->sm.req, p_port)) > + signal = OSM_SIGNAL_DONE_PENDING; > + if (osm_node_get_type( osm_port_get_parent_node( p_port ) ) != > + IB_NODE_TYPE_SWITCH && > + pkey_mgr_update_peer_port( &p_osm->log, &p_osm->sm.req, > + &p_osm->subn, p_port, > + !p_osm->subn.opt.no_partition_enforcement )) > + signal = OSM_SIGNAL_DONE_PENDING; > } > > _err: > _______________________________________________ openib-general mailing list [email protected] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
