[ofw] Using ib_local_mad instead of SM query
Alex Estrin
alex.estrin at qlogic.com
Sun Oct 12 09:08:58 PDT 2008
Hi Slava,
I think calling __endpt_mgr_add_local() after __port_get_bcast() most
likely will introduce racing condition
because __port_get_bcast() is async call and completes in __bcast_cb()
where adapter moves to active state and ready to process data packets.
I would consider possibility to call __endpt_mgr_add_local() from within
__bcast_cb() to ensure proper initialization ordering.
Thanks,
Alex.
> -----Original Message-----
> From: ofw-bounces at lists.openfabrics.org
> [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Slava Strebkov
> Sent: Sunday, October 12, 2008 10:25 AM
> To: ofw at lists.openfabrics.org
> Subject: [ofw] Using ib_local_mad instead of SM query
>
> The following patch is intended to reduce workload from SM .
>
> SM query in ipoib_port_up is replaced to ib_local_mad, but
> __endpt_mgr_add_local is called only once and only after
>
> __port_get_bcast succeeded. This is done to prevent assertion
> in __endpt_mgr_reset_all.
>
>
>
>
>
>
>
> Index: ulp/ipoib/kernel/ipoib_port.c
>
> ===================================================================
>
> --- ulp/ipoib/kernel/ipoib_port.c (revision 1648)
>
> +++ ulp/ipoib/kernel/ipoib_port.c (working copy)
>
> @@ -451,11 +451,6 @@
>
> IN
> ipoib_port_t* const p_port );
>
>
>
> static void
>
> -__port_info_cb(
>
> - IN
> ib_query_rec_t *p_query_rec );
>
> -
>
> -
>
> -static void
>
> __bcast_get_cb(
>
> IN
> ib_query_rec_t *p_query_rec );
>
>
>
> @@ -5074,12 +5069,13 @@
>
> IN const
> ib_pnp_port_rec_t* const p_pnp_rec )
>
> {
>
> ib_api_status_t status;
>
> - ib_query_req_t query;
>
> - ib_user_query_t info;
>
> - ib_portinfo_record_t port_rec;
>
> + ib_port_info_t *p_port_info;
>
> + ib_mad_t *mad_in = NULL;
>
> + ib_mad_t *mad_out = NULL;
>
>
>
> IPOIB_ENTER( IPOIB_DBG_INIT );
>
>
>
> + UNREFERENCED_PARAMETER(p_pnp_rec);
>
> /* Wait for all work requests to get flushed. */
>
> while( p_port->recv_mgr.depth || p_port->send_mgr.depth )
>
> cl_thread_suspend( 0 );
>
> @@ -5089,41 +5085,82 @@
>
> KeResetEvent( &p_port->sa_event );
>
> cl_obj_unlock( &p_port->obj );
>
>
>
> - info.method = IB_MAD_METHOD_GETTABLE;
>
> - info.attr_id = IB_MAD_ATTR_PORTINFO_RECORD;
>
> - info.attr_size = sizeof(ib_portinfo_record_t);
>
> - info.comp_mask = IB_PIR_COMPMASK_BASELID;
>
> - info.p_attr = &port_rec;
>
> + mad_out = (ib_mad_t*)cl_zalloc(256);
>
> + if(! mad_out)
>
> + {
>
> + IPOIB_PRINT_EXIT(
> TRACE_LEVEL_INFORMATION, IPOIB_DBG_ERROR,
>
> + ("failed to allocate mad
> mad_out\n"));
>
> + return;
>
> + }
>
> + mad_in = (ib_mad_t*)cl_zalloc(256);
>
> + if(! mad_in)
>
> + {
>
> + IPOIB_PRINT_EXIT(
> TRACE_LEVEL_INFORMATION, IPOIB_DBG_ERROR,
>
> + ("failed to allocate mad
> mad_in\n"));
>
> + return;
>
> + }
>
>
>
> - /* Query requires only the base LID. */
>
> - cl_memclr( &port_rec, sizeof(ib_portinfo_record_t) );
>
> - port_rec.port_info.base_lid = p_pnp_rec->p_port_attr->lid;
>
> + mad_in->attr_id = IB_MAD_ATTR_PORT_INFO;
>
> + mad_in->method = IB_MAD_METHOD_GET;
>
> + mad_in->base_ver = 1;
>
> + mad_in->class_ver =1;
>
> + mad_in->mgmt_class = IB_MCLASS_SUBN_LID;
>
> +
>
> + status = p_port->p_adapter->p_ifc->local_mad(
>
> + p_port->ib_mgr.h_ca ,p_port->port_num
> ,mad_in ,mad_out);
>
>
>
> - cl_memclr( &query, sizeof(ib_query_req_t) );
>
> - query.query_type = IB_QUERY_USER_DEFINED;
>
> - query.p_query_input = &info;
>
> - query.port_guid = p_port->p_adapter->guids.port_guid.guid;
>
> - query.timeout_ms = p_port->p_adapter->params.sa_timeout;
>
> - query.retry_cnt = p_port->p_adapter->params.sa_retry_cnt;
>
> - query.query_context = p_port;
>
> - query.pfn_query_cb = __port_info_cb;
>
> -
>
> - /* reference the object for the multicast query. */
>
> - ipoib_port_ref( p_port, ref_port_up );
>
> -
>
> - status = p_port->p_adapter->p_ifc->query(
>
> - p_port->p_adapter->h_al, &query,
> &p_port->ib_mgr.h_query );
>
> if( status != IB_SUCCESS )
>
> {
>
> KeSetEvent( &p_port->sa_event,
> EVENT_INCREMENT, FALSE );
>
> ipoib_set_inactive( p_port->p_adapter );
>
> - ipoib_port_deref( p_port, ref_port_up );
>
> IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR,
> IPOIB_DBG_ERROR,
>
> - ("ib_query returned %s\n",
>
> + ("ib_local_mad returned %s\n",
>
>
> p_port->p_adapter->p_ifc->get_err_str( status )) );
>
> - return;
>
> + goto up_done;
>
> }
>
>
>
> + p_port_info =
> (ib_port_info_t*)(((ib_smp_t*)mad_out)->data);
>
> +
>
> + IPOIB_PRINT( TRACE_LEVEL_INFORMATION, IPOIB_DBG_INIT,
>
> + ("Received port info: link
> width = %d.\n",
>
> + p_port_info->link_width_active) );
>
> + p_port->ib_mgr.rate =
>
> + ib_port_info_compute_rate( p_port_info );
>
> +
>
> + ipoib_set_rate( p_port->p_adapter,
>
> + p_port_info->link_width_active,
>
> + ib_port_info_get_link_speed_active( p_port_info ) );
>
> +
>
> + status = __port_get_bcast( p_port );
>
> + if (status != IB_SUCCESS)
>
> + {
>
> + IPOIB_PRINT( TRACE_LEVEL_ERROR,
> IPOIB_DBG_ERROR,
>
> + (" __port_get_bcast returned
> %s\n",p_port->p_adapter->p_ifc->get_err_str( status )));
>
> + }
>
> + else
>
> + {
>
> + if(! p_port->p_local_endpt)
>
> + {
>
> + status =
> __endpt_mgr_add_local( p_port, p_port_info );
>
> + }
>
> + }
>
> +
>
> +up_done:
>
> + if( status != IB_SUCCESS )
>
> + {
>
> + if( status != IB_CANCELED )
>
> + {
>
> + ipoib_set_inactive(
> p_port->p_adapter );
>
> + __endpt_mgr_reset_all( p_port );
>
> + }
>
> + KeSetEvent( &p_port->sa_event,
> EVENT_INCREMENT, FALSE );
>
> + }
>
> +
>
> + if(mad_out)
>
> + cl_free(mad_out);
>
> + if(mad_in)
>
> + cl_free(mad_in);
>
> +
>
> IPOIB_EXIT( IPOIB_DBG_INIT );
>
> }
>
>
>
> @@ -5187,111 +5224,7 @@
>
> }
>
>
>
>
>
> -static void
>
> -__port_info_cb(
>
> - IN
> ib_query_rec_t *p_query_rec )
>
> -{
>
> - ib_api_status_t status;
>
> - ipoib_port_t *p_port;
>
> - ib_portinfo_record_t *p_port_rec;
>
>
>
> - IPOIB_ENTER( IPOIB_DBG_INIT );
>
> -
>
> - p_port = (ipoib_port_t*)p_query_rec->query_context;
>
> -
>
> - cl_obj_lock( &p_port->obj );
>
> - p_port->ib_mgr.h_query = NULL;
>
> -
>
> - if( p_port->state != IB_QPS_INIT )
>
> - {
>
> - status = IB_CANCELED;
>
> - goto done;
>
> - }
>
> -
>
> - status = p_query_rec->status;
>
> -
>
> - switch( status )
>
> - {
>
> - case IB_SUCCESS:
>
> - /* Note that the we report the rate
> from the port info. */
>
> - p_port_rec = (ib_portinfo_record_t*)
>
> - ib_get_query_result(
> p_query_rec->p_result_mad, 0 );
>
> -
>
> - status = __endpt_mgr_add_local(
> p_port, &p_port_rec->port_info );
>
> - if( status == IB_SUCCESS )
>
> - {
>
> - IPOIB_PRINT(
> TRACE_LEVEL_INFORMATION, IPOIB_DBG_INIT,
>
> - ("Received
> port info: link width = %d.\n",
>
> -
> p_port_rec->port_info.link_width_active) );
>
> -
>
> - p_port->ib_mgr.rate =
>
> -
> ib_port_info_compute_rate( &p_port_rec->port_info );
>
> -
>
> - ipoib_set_rate( p_port->p_adapter,
>
> -
> p_port_rec->port_info.link_width_active,
>
> -
> ib_port_info_get_link_speed_active( &p_port_rec->port_info ) );
>
> -
>
> - status =
> __port_get_bcast( p_port );
>
> - }
>
> - else
>
> - {
>
> - IPOIB_PRINT(
> TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
>
> -
> ("__endpt_mgr_add_local returned %s\n",
>
> -
> p_port->p_adapter->p_ifc->get_err_str( status )) );
>
> - }
>
> - break;
>
> -
>
> - case IB_CANCELED:
>
> - IPOIB_PRINT(TRACE_LEVEL_INFORMATION,
> IPOIB_DBG_INIT,
>
> - ("Instance destroying -
> Aborting.\n") );
>
> - break;
>
> -
>
> - case IB_TIMEOUT:
>
> - NdisWriteErrorLogEntry(
> p_port->p_adapter->h_adapter,
>
> -
> EVENT_IPOIB_PORT_INFO_TIMEOUT, 0 );
>
> - IPOIB_PRINT( TRACE_LEVEL_INFORMATION,
> IPOIB_DBG_INIT,
>
> - ("Port info query timed
> out.\n") );
>
> - break;
>
> -
>
> - case IB_REMOTE_ERROR:
>
> - NdisWriteErrorLogEntry(
> p_port->p_adapter->h_adapter,
>
> - EVENT_IPOIB_PORT_INFO_REJECT, 0 );
>
> - IPOIB_PRINT( TRACE_LEVEL_INFORMATION,
> IPOIB_DBG_INIT,
>
> - ("Port info query
> rejected by SA.\n") );
>
> - break;
>
> -
>
> - default:
>
> - NdisWriteErrorLogEntry(
> p_port->p_adapter->h_adapter,
>
> -
> EVENT_IPOIB_QUERY_PORT_INFO, 1, p_query_rec->status );
>
> - /* Hopefully we'll get an SM change
> event that will restart things. */
>
> - IPOIB_PRINT( TRACE_LEVEL_INFORMATION,
> IPOIB_DBG_INIT,
>
> - ("Port info query failed.\n") );
>
> - }
>
> -
>
> -done:
>
> - cl_obj_unlock( &p_port->obj );
>
> -
>
> - if( status != IB_SUCCESS )
>
> - {
>
> - if( status != IB_CANCELED )
>
> - {
>
> - ipoib_set_inactive(
> p_port->p_adapter );
>
> - __endpt_mgr_reset_all( p_port );
>
> - }
>
> - KeSetEvent( &p_port->sa_event,
> EVENT_INCREMENT, FALSE );
>
> - }
>
> -
>
> - /* Return the response MAD to AL. */
>
> - if( p_query_rec->p_result_mad )
>
> - p_port->p_adapter->p_ifc->put_mad(
> p_query_rec->p_result_mad );
>
> -
>
> - /* Release the reference taken when issuing the
> port info query. */
>
> - ipoib_port_deref( p_port, ref_port_info_cb );
>
> -
>
> - IPOIB_EXIT( IPOIB_DBG_INIT );
>
> -}
>
> -
>
> -
>
> static ib_api_status_t
>
> __port_get_bcast(
>
> IN
> ipoib_port_t* const p_port )
>
>
>
>
>
> Slava
>
>
>
>
More information about the ofw
mailing list