[openib-general] RE: [PATCH 3/4] opensm: pkey manager performance improvement
Eitan Zahavi
eitan at mellanox.co.il
Tue May 2 01:45:02 PDT 2006
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:sashak at voltaire.com]
> Sent: Sunday, April 23, 2006 5:26 PM
> To: Hal Rosenstock
> Cc: openib-general at openib.org; 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 <sashak at voltaire.com>
> ---
>
> 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:
>
More information about the general
mailing list