[ofw][PATCH] [IBBUS] Right treatment for async process of send MAD completion

[Fab Tillier]

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