[openib-general] Re: [PATCH] [kDAPL} Fix locking problem in some CM callback functions

James Lentini jlentini at netapp.com
Thu Jun 23 07:46:41 PDT 2005 

Committed in revision 2686.

On Wed, 22 Jun 2005, Hal Rosenstock wrote:

halr> Fix locking problem in some CM callback functions
halr> Since modify QP can sleep, dapl_ib_disconnect must be called without
halr> holding spinlock.
halr> 
halr> Signed-off-by: Itamar Rabenstein <itamar at mellanox.co.il>
halr> Signed-off-by: Hal Rosenstock <halr at voltaire.com>
halr> 
halr> Index: dapl_openib_util.h
halr> ===================================================================
halr> --- dapl_openib_util.h	(revision 2681)
halr> +++ dapl_openib_util.h	(working copy)
halr> @@ -125,7 +125,7 @@
halr>  
halr>  void dapl_ib_reinit_ep(struct dapl_ep *ep);
halr>  
halr> -void dapl_ib_disconnect_clean(struct dapl_ep *ep, boolean_t passive);
halr> +void dapl_ib_disconnect_clean(struct dapl_ep *ep);
halr>  
halr>  u32 dapl_ib_get_async_event(struct ib_event *cause,
halr>  			    enum dat_event_number *async_event);
halr> Index: dapl_cr.c
halr> ===================================================================
halr> --- dapl_cr.c	(revision 2681)
halr> +++ dapl_cr.c	(working copy)
halr> @@ -397,7 +397,7 @@
halr>  	struct dapl_ep *ep;
halr>  	struct dapl_evd *evd;
halr>  	u32 status;
halr> -
halr> +	
halr>  	dapl_dbg_log(DAPL_DBG_TYPE_CM | DAPL_DBG_TYPE_CALLBACK,
halr>  		     "--> dapl_cr_callback! cm_handle: %p event: %x sp: %p\n",
halr>  		     cm_ctx, event, sp);
halr> @@ -431,7 +431,6 @@
halr>  	}
halr>  
halr>  	status = DAT_INTERNAL_ERROR;	/* init to ERR */
halr> -
halr>  	switch (event) {
halr>  	case DAT_CONNECTION_REQUEST_EVENT:
halr>  		/*
halr> @@ -479,8 +478,7 @@
halr>  			/* If someone pulled the plug on the EP or connection,
halr>  			 * just exit
halr>  			 */
halr> -			spin_unlock_irqrestore(&ep->common.lock, 
halr> -					       ep->common.flags);
halr> +			spin_unlock_irqrestore(&ep->common.lock, ep->common.flags);
halr>  			status = DAT_SUCCESS;
halr>  			/* Set evd = NULL so we don't generate an event below */
halr>  			evd = NULL;
halr> @@ -504,36 +502,22 @@
halr>  			/* The disconnect has already occurred, we are now
halr>  			 * cleaned up and ready to exit
halr>  			 */
halr> -			spin_unlock_irqrestore(&ep->common.lock, 
halr> -					       ep->common.flags);
halr> +			spin_unlock_irqrestore(&ep->common.lock, ep->common.flags); 
halr>  			return;
halr>  		}
halr>  		ep->param.ep_state = DAT_EP_STATE_DISCONNECTED;
halr> -		dapl_ib_disconnect_clean(ep, FALSE);
halr>  		spin_unlock_irqrestore(&ep->common.lock, ep->common.flags);
halr> -
halr> +		dapl_ib_disconnect_clean(ep);
halr>  		break;
halr>  	case DAT_CONNECTION_EVENT_NON_PEER_REJECTED:
halr>  	case DAT_CONNECTION_EVENT_PEER_REJECTED:
halr>  	case DAT_CONNECTION_EVENT_UNREACHABLE:
halr> -		/*
halr> -		 * After posting an accept the requesting node has
halr> -		 * stopped talking.
halr> -		 */
halr> +	case DAT_CONNECTION_EVENT_BROKEN:
halr>  		spin_lock_irqsave(&ep->common.lock, ep->common.flags);
halr>  		ep->param.ep_state = DAT_EP_STATE_DISCONNECTED;
halr> -		dapl_ib_disconnect_clean(ep, FALSE);
halr>  		spin_unlock_irqrestore(&ep->common.lock, ep->common.flags);
halr> -
halr> +		dapl_ib_disconnect_clean(ep);
halr>  		break;
halr> -	case DAT_CONNECTION_EVENT_BROKEN:
halr> -		spin_lock_irqsave(&ep->common.lock, ep->common.flags);
halr> -		ep->param.ep_state = DAT_EP_STATE_DISCONNECTED;
halr> -		dapl_ib_disconnect_clean(ep, FALSE);
halr> -		spin_unlock_irqrestore(&ep->common.lock,
halr> -				       ep->common.flags);
halr> -
halr> -		break;
halr>  	default:
halr>  		evd = NULL;
halr>  		dapl_os_assert(0);	/* shouldn't happen */
halr> Index: dapl_evd.c
halr> ===================================================================
halr> --- dapl_evd.c	(revision 2681)
halr> +++ dapl_evd.c	(working copy)
halr> @@ -757,10 +757,8 @@
halr>  	 * when necessary
halr>  	 */
halr>  	spin_lock_irqsave(&ep->common.lock, ep->common.flags);
halr> -
halr>  	switch (event) {
halr>  	case DAT_CONNECTION_EVENT_ESTABLISHED:
halr> -	{
halr>  		/* 
halr>  		 * If we don't have an EP at this point we are very screwed up 
halr>  		 */
halr> @@ -784,65 +782,28 @@
halr>  				      private_data_size);
halr>  		}
halr>  		spin_unlock_irqrestore(&ep->common.lock, ep->common.flags);
halr> -
halr>  		break;
halr> -	}
halr>  	case DAT_CONNECTION_EVENT_DISCONNECTED:
halr> -	{
halr> -		/*
halr> -		 * EP is now fully disconnected; initiate any post processing
halr> -		 * to reset the underlying QP and get the EP ready for
halr> -		 * another connection
halr> -		 */
halr>  		ep->param.ep_state = DAT_EP_STATE_DISCONNECTED;
halr> -		dapl_ib_disconnect_clean(ep, TRUE);
halr> -		spin_unlock_irqrestore(&ep->common.lock,
halr> -				       ep->common.flags);
halr> -
halr> -		break;
halr> -	}
halr> -	case DAT_CONNECTION_EVENT_PEER_REJECTED:
halr> -	{
halr> -		ep->param.ep_state = DAT_EP_STATE_DISCONNECTED;
halr> -		dapl_ib_disconnect_clean(ep, TRUE);
halr>  		spin_unlock_irqrestore(&ep->common.lock, ep->common.flags);
halr> -
halr> +		dapl_ib_disconnect_clean(ep);
halr>  		break;
halr> -	}
halr> +	case DAT_CONNECTION_EVENT_PEER_REJECTED:
halr>  	case DAT_CONNECTION_EVENT_UNREACHABLE:
halr> -	{
halr> -		ep->param.ep_state = DAT_EP_STATE_DISCONNECTED;
halr> -		dapl_ib_disconnect_clean(ep, TRUE);
halr> -		spin_unlock_irqrestore(&ep->common.lock, ep->common.flags);
halr> -
halr> -		break;
halr> -	}
halr>  	case DAT_CONNECTION_EVENT_NON_PEER_REJECTED:
halr> -	{
halr> -		ep->param.ep_state = DAT_EP_STATE_DISCONNECTED;
halr> -		dapl_ib_disconnect_clean(ep, TRUE);
halr> -		spin_unlock_irqrestore(&ep->common.lock, ep->common.flags);
halr> -
halr> -		break;
halr> -	}
halr>  	case DAT_CONNECTION_EVENT_BROKEN:
halr> -	{
halr>  		ep->param.ep_state = DAT_EP_STATE_DISCONNECTED;
halr> -		dapl_ib_disconnect_clean(ep, FALSE);
halr>  		spin_unlock_irqrestore(&ep->common.lock, ep->common.flags);
halr> -
halr> +		dapl_ib_disconnect_clean(ep);
halr>  		break;
halr> -	}
halr>  	case DAT_CONNECTION_REQUEST_EVENT:
halr>  	default:
halr> -	{
halr>  		spin_unlock_irqrestore(&ep->common.lock, ep->common.flags);
halr>  		evd = NULL;
halr>  
halr>  		dapl_os_assert(0);	/* shouldn't happen */
halr>  		break;
halr>  	}
halr> -	}
halr>  
halr>  	/*
halr>  	 * Post the event
halr> Index: dapl_openib_cm.c
halr> ===================================================================
halr> --- dapl_openib_cm.c	(revision 2681)
halr> +++ dapl_openib_cm.c	(working copy)
halr> @@ -553,7 +553,6 @@
halr>   *
halr>   * Input:
halr>   *      ep_ptr          pointer to dapl_ep struct
halr> - *      active          Indicates active side of connection
halr>   *
halr>   * Output:
halr>   *      none
halr> @@ -562,13 +561,12 @@
halr>   *      void
halr>   *
halr>   */
halr> -void dapl_ib_disconnect_clean(struct dapl_ep *ep_ptr, boolean_t active)
halr> +void dapl_ib_disconnect_clean(struct dapl_ep *ep_ptr)
halr>  {
halr>  	int status;
halr>  
halr>  	dapl_dbg_log(DAPL_DBG_TYPE_CM,
halr> -		     "  >>> dapl_ib_disconnect_clean: EP: %p active %d\n",
halr> -		     ep_ptr, active);
halr> +		     "  >>> dapl_ib_disconnect_clean: EP: %p\n", ep_ptr);
halr>  
halr>  	/*
halr>  	 * Clean up outstanding connection state
halr> 
halr> 
halr>

More information about the general mailing list