[openib-general] [PATCHv5] osm: partition manager force policy

[Hal Rosenstock]

On Mon, 2006-06-19 at 15:05, Eitan Zahavi wrote:
> Hi Hal
> 
> This is a 5th take after incorporating Sasha's last reported bug 
> on bad assignment of the used_blocks.
> 
> This code was run again through my verification flow and also Sasha
> had run some tests too.
> 
> Eitan
> 
> Signed-off-by:  Eitan Zahavi <eitan at mellanox.co.il>

[snip...]

> Index: opensm/osm_pkey.c
> ===================================================================
> --- opensm/osm_pkey.c	(revision 8113)
> +++ opensm/osm_pkey.c	(working copy)
> @@ -94,18 +94,22 @@ void osm_pkey_tbl_destroy( 
>  
>  /**********************************************************************
>   **********************************************************************/
> -int osm_pkey_tbl_init( 
> +ib_api_status_t
> +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 );
> +	cl_qlist_init( &p_pkey_tbl->pending );
> +	p_pkey_tbl->used_blocks = 0;
> +	p_pkey_tbl->max_blocks = 0;
>    return(IB_SUCCESS);
>  }
>  
>  /**********************************************************************
>   **********************************************************************/
> -void osm_pkey_tbl_sync_new_blocks(
> +void osm_pkey_tbl_init_new_blocks(
>    IN const osm_pkey_tbl_t *p_pkey_tbl)
>  {
>    ib_pkey_table_t *p_block, *p_new_block;
> @@ -123,16 +127,31 @@ void osm_pkey_tbl_sync_new_blocks(
>        p_new_block = (ib_pkey_table_t *)malloc(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);
> +		}
> +
>        memset(p_new_block, 0, sizeof(*p_new_block));
> -      cl_ptr_vector_set(&((osm_pkey_tbl_t *)p_pkey_tbl)->new_blocks, b, p_new_block);
>      }
> -    memcpy(p_new_block, p_block, sizeof(*p_new_block));
> +}
> +
> +/**********************************************************************
> + **********************************************************************/
> +void osm_pkey_tbl_cleanup_pending(
> +	IN osm_pkey_tbl_t *p_pkey_tbl)
> +{
> +	cl_list_item_t	*p_item;
> +	p_item = cl_qlist_remove_head( &p_pkey_tbl->pending );
> +	while (p_item != cl_qlist_end( &p_pkey_tbl->pending ) )
> +	{
> +		free( (osm_pending_pkey_t *)p_item );
>    }
>  }
>  
>  /**********************************************************************
>   **********************************************************************/
> -int osm_pkey_tbl_set( 
> +ib_api_status_t
> +osm_pkey_tbl_set(
>    IN osm_pkey_tbl_t *p_pkey_tbl,
>    IN uint16_t block, 
>    IN ib_pkey_table_t *p_tbl)
> @@ -203,7 +222,138 @@ int osm_pkey_tbl_set( 
>  
>  /**********************************************************************
>   **********************************************************************/
> -static boolean_t __osm_match_pkey (
> +ib_api_status_t
> +osm_pkey_tbl_make_block_pair( 
> +	osm_pkey_tbl_t   *p_pkey_tbl, 
> +	uint16_t          block_idx,
> +	ib_pkey_table_t **pp_old_block,
> +	ib_pkey_table_t **pp_new_block)
> +{
> +	if (block_idx >= p_pkey_tbl->max_blocks) return(IB_ERROR);
> +
> +	if (pp_old_block)
> +	{
> +		*pp_old_block = osm_pkey_tbl_block_get( p_pkey_tbl, block_idx );
> +		if (! *pp_old_block)
> +		{
> +			*pp_old_block = (ib_pkey_table_t *)malloc(sizeof(ib_pkey_table_t));
> +			if (!*pp_old_block) return(IB_ERROR);
> +			memset(*pp_old_block, 0, sizeof(ib_pkey_table_t));
> +			cl_ptr_vector_set(&p_pkey_tbl->blocks, block_idx, *pp_old_block);
> +		}
> +	}
> +	
> +	if (pp_new_block)
> +	{
> +		*pp_new_block = osm_pkey_tbl_new_block_get( p_pkey_tbl, block_idx );
> +		if (! *pp_new_block)
> +		{
> +			*pp_new_block = (ib_pkey_table_t *)malloc(sizeof(ib_pkey_table_t));
> +			if (!*pp_new_block) return(IB_ERROR);
> +			memset(*pp_new_block, 0, sizeof(ib_pkey_table_t));
> +			cl_ptr_vector_set(&p_pkey_tbl->new_blocks, block_idx, *pp_new_block);
> +		}
> +	}
> +	return( IB_SUCCESS );
> +}
> +
> +/**********************************************************************
> + **********************************************************************/
> +/*
> +  store the given pkey in the "new" blocks array 
> +  also makes sure the regular block exists.
> +*/
> +ib_api_status_t
> +osm_pkey_tbl_set_new_entry( 
> +	IN osm_pkey_tbl_t *p_pkey_tbl,
> +	IN uint16_t        block_idx,
> +	IN uint8_t         pkey_idx,
> +	IN uint16_t        pkey)
> +{  
> +	ib_pkey_table_t *p_old_block;
> +	ib_pkey_table_t *p_new_block;
> +	
> +	if (osm_pkey_tbl_make_block_pair(
> +			 p_pkey_tbl, block_idx, &p_old_block, &p_new_block))
> +		return( IB_ERROR );
> +		
> +	p_new_block->pkey_entry[pkey_idx] = pkey;
> +	if (p_pkey_tbl->used_blocks <= block_idx)
> +		p_pkey_tbl->used_blocks = block_idx + 1;
> +
> +	return( IB_SUCCESS );
> +}
> +
> +/**********************************************************************
> + **********************************************************************/
> +boolean_t
> +osm_pkey_find_next_free_entry(
> +	IN osm_pkey_tbl_t *p_pkey_tbl, 
> +	OUT uint16_t      *p_block_idx,
> +	OUT uint8_t       *p_pkey_idx)
> +{
> +	ib_pkey_table_t *p_new_block;
> +	
> +	CL_ASSERT(p_block_idx);
> +	CL_ASSERT(p_pkey_idx);
> +
> +	while ( *p_block_idx < p_pkey_tbl->max_blocks)
> +	{
> +		if (*p_pkey_idx > IB_NUM_PKEY_ELEMENTS_IN_BLOCK - 1)
> +		{
> +			*p_pkey_idx = 0;
> +			(*p_block_idx)++;
> +			if (*p_block_idx >= p_pkey_tbl->max_blocks) 
> +				return FALSE;
> +		}
> +
> +		p_new_block = osm_pkey_tbl_new_block_get( p_pkey_tbl, *p_block_idx);
> +
> +		if ( !p_new_block || 
> +			  ib_pkey_is_invalid(p_new_block->pkey_entry[*p_pkey_idx]))
> +			return TRUE;
> +		else
> +			(*p_pkey_idx)++;
> +	}
> +	return FALSE;
> +}
> +
> +/**********************************************************************
> + **********************************************************************/
> +ib_api_status_t
> +osm_pkey_tbl_get_block_and_idx(
> +	IN	 osm_pkey_tbl_t *p_pkey_tbl,
> +	IN	 uint16_t		 *p_pkey,
> +	OUT uint32_t		 *p_block_idx,
> +	OUT uint8_t			 *p_pkey_index)
> +{
> +	uint32_t			  num_of_blocks;
> +	uint32_t			  block_index;
> +	ib_pkey_table_t *block;
> +
> +	CL_ASSERT( p_pkey_tbl );

Should the other routines also assert on this or should this be
consistent with the others ?

> +	CL_ASSERT( p_block_idx != NULL );
> +	CL_ASSERT( p_pkey_idx != NULL );

There is no p_pkey_idx parameter. I presume this should be p_pkey_index.

Also, should there be:
	CL_ASSERT( p_pkey );
as well ?

> +	num_of_blocks = cl_ptr_vector_get_size( &p_pkey_tbl->blocks);
> +	for ( block_index = 0; block_index < num_of_blocks; block_index++ )
> +	{
> +		block = osm_pkey_tbl_block_get( p_pkey_tbl, block_index );
> +		if ( ( block->pkey_entry <= p_pkey ) &&
> +			  ( p_pkey < block->pkey_entry + IB_NUM_PKEY_ELEMENTS_IN_BLOCK))
> +		{
> +			*p_block_idx = block_index;
> +			*p_pkey_index = p_pkey - block->pkey_entry;
> +			return( IB_SUCCESS );
> +		}
> +	}
> +	return( IB_NOT_FOUND );
> +}
> +
> +/**********************************************************************
> + **********************************************************************/
> +static boolean_t 
> +__osm_match_pkey (
>    IN const ib_net16_t *pkey1,
>    IN const ib_net16_t *pkey2 ) {
>  
> @@ -306,7 +456,8 @@ osm_physp_share_pkey(
>    if (cl_is_map_empty(&pkey_tbl1->keys) || cl_is_map_empty(&pkey_tbl2->keys))
>      return TRUE;
>  
> -  return !ib_pkey_is_invalid(osm_physp_find_common_pkey(p_physp_1, p_physp_2));
> +	return 
> +		!ib_pkey_is_invalid(osm_physp_find_common_pkey(p_physp_1, p_physp_2));
>  }
>  
>  /**********************************************************************

[snip...]

Also, two things about osm_pkey_mgr.c:

Was there a need to reorder the routines ? This broke the diff so it had
to be done largely by hand.

Also, it would have been nice not to mix the format changes with the
substantive changes. Try to keep it to "one thought per patch".

This patch has been applied with cosmetic changes. We will go from
here...

-- Hal