[ofw][PATCH] [IBBUS] Right treatment for async process of send MAD completion
Fab Tillier
ftillier at microsoft.com
Mon Oct 26 11:47:29 PDT 2009
Alex Naslednikov wrote on Mon, 26 Oct 2009 at 11:06:50:
> Handling a situation when __complete_send_mad receives NULL instead of
> p_mad_wr parameter
> (may occur during Verifier low-mem simulation run)
> Signed-off by: Alexander Naslednikov (xalex at mellanox.co.il)
>
> Index: D:/windows/MLNX_WinOF_trunk/core/al/kernel/al_smi.c
> =================================================================== ---
> D:/windows/MLNX_WinOF_trunk/core/al/kernel/al_smi.c (revision 4987) +++
> D:/windows/MLNX_WinOF_trunk/core/al/kernel/al_smi.c (revision 4992) @@
> -893,7 +893,10 @@
>
> /* Construct a send work completion. */
> cl_memclr( &wc, sizeof( ib_wc_t ) );
> - wc.wr_id = p_mad_wr->send_wr.wr_id;
> + if (p_mad_wr) {
> + // Handling the special race where p_mad_wr that comes from spl_qp
> can be NULL
> + wc.wr_id = p_mad_wr->send_wr.wr_id;
This doesn't seem like the right solution, is it? Will the call to mad_disp_send_done handle a NULL MAD work request?
In fact, can you explain the code path that results in p_mad_wr being NULL? Again, this code should assert that a non-NULL p_mad_wr was supplied, and if that assertion fails, the calling code needs to be fixed.
Also, your curly brace placement is inconsistent with the rest of the code base.
> + }
> wc.wc_type = IB_WC_SEND;
> wc.status = wc_status;
> Index: D:/windows/MLNX_WinOF_trunk/core/al/al_mad.c
> =================================================================== ---
> D:/windows/MLNX_WinOF_trunk/core/al/al_mad.c (revision 4987) +++
> D:/windows/MLNX_WinOF_trunk/core/al/al_mad.c (revision 4992) @@ -191,7
> +191,7 @@
> static boolean_t
> __is_send_mad_done(
> IN ib_mad_send_handle_t h_send,
> - IN ib_wc_t *p_wc );
> + IN ib_wc_status_t status );
This change seems unrelated to the above fix. It should be checked in separately, as a refactoring change rather than a bug fix.
>
> static void
> __notify_send_comp(
> @@ -1929,7 +1929,7 @@
> }
>
> /* See if the send request has completed. */
> - if( __is_send_mad_done( h_send, p_wc ) )
> + if( __is_send_mad_done( h_send, p_wc->status ) )
> {
> /* The send has completed. */
> cl_qlist_remove_item( &h_mad_svc->send_list,
> @@ -2040,12 +2040,12 @@
> static boolean_t
> __is_send_mad_done(
> IN ib_mad_send_handle_t h_send,
> - IN ib_wc_t *p_wc )
> + IN ib_wc_status_t status )
> {
> AL_ENTER( AL_DBG_MAD_SVC );
>
> /* Complete the send if the request failed. */
> - if( p_wc->status != IB_WCS_SUCCESS )
> + if( status != IB_WCS_SUCCESS )
> {
> AL_PRINT_EXIT( TRACE_LEVEL_ERROR, AL_DBG_ERROR, ("y-send failed\n" )
> ); return TRUE;
-Fab
More information about the ofw
mailing list