[openib-general] Re: [PATCH][kdapl] fix spin_lock_irqsave/spin_unlock_irqrestore implementation

James Lentini jlentini at netapp.com
Thu May 19 19:14:22 PDT 2005


Itamar,

Thank you for pointing this out.

Long term I think it will be better to keep the flags with the spin 
lock so that these "scoping" issues don't crop up and force us to pass 
the flags around. The fix for this is in revision 2420.

james

On Wed, 18 May 2005, Itamar wrote:

itamar> when spin_lock_irqsave and spin_unlock_irqrestore are not called in the same function 
itamar> need to pass the flags form the save the store
itamar> 
itamar> Signed-off-by: Itamar Rabenstein <itamar at mellanox.co.il>
itamar> 
itamar> Index: dapl_evd_util.c
itamar> ===================================================================
itamar> --- dapl_evd_util.c	(revision 2374)
itamar> +++ dapl_evd_util.c	(working copy)
itamar> @@ -378,20 +378,19 @@
itamar>   * that the lock is held.
itamar>   */
itamar>  
itamar> -static struct dat_event *dapl_evd_get_event(DAPL_EVD * evd_ptr)
itamar> +static struct dat_event *dapl_evd_get_event(DAPL_EVD * evd_ptr, unsigned long *flags)
itamar>  {
itamar>  	struct dat_event *event;
itamar> -	unsigned long flags;
itamar>  
itamar>  	if (evd_ptr->evd_producer_locking_needed) {
itamar> -		spin_lock_irqsave(&evd_ptr->header.lock, flags);
itamar> +		spin_lock_irqsave(&evd_ptr->header.lock, *flags);
itamar>  	}
itamar>  
itamar>  	event = (struct dat_event *) dapl_rbuf_remove(&evd_ptr->free_event_queue);
itamar>  
itamar>  	/* Release the lock if it was taken and the call failed.  */
itamar>  	if (!event && evd_ptr->evd_producer_locking_needed) {
itamar> -		spin_unlock_irqrestore(&evd_ptr->header.lock, flags);
itamar> +		spin_unlock_irqrestore(&evd_ptr->header.lock, *flags);
itamar>  	}
itamar>  
itamar>  	return event;
itamar> @@ -406,10 +405,10 @@
itamar>   */
itamar>  
itamar>  static void dapl_evd_post_event(DAPL_EVD *evd_ptr,
itamar> -				const struct dat_event *event_ptr)
itamar> +				const struct dat_event *event_ptr,
itamar> +				unsigned long flags)
itamar>  {
itamar>  	u32 dat_status;
itamar> -	unsigned long flags;
itamar>  	DAPL_CNO * cno_to_trigger = NULL;
itamar>  
itamar>  	dapl_dbg_log(DAPL_DBG_TYPE_EVD,
itamar> @@ -459,7 +458,7 @@
itamar>  			     DAPL_EVD * overflow_evd_ptr)
itamar>  {
itamar>  	struct dat_event *overflow_event;
itamar> -
itamar> +	unsigned long flags;
itamar>  	/* The overflow_evd_ptr mght be the same as evd.
itamar>  	 * In that case we've got a catastrophic overflow.
itamar>  	 */
itamar> @@ -469,7 +468,7 @@
itamar>  		return;
itamar>  	}
itamar>  
itamar> -	overflow_event = dapl_evd_get_event(overflow_evd_ptr);
itamar> +	overflow_event = dapl_evd_get_event(overflow_evd_ptr, &flags);
itamar>  	if (!overflow_event) {
itamar>  		/* this is not good */
itamar>  		overflow_evd_ptr->catastrophic_overflow = TRUE;
itamar> @@ -477,17 +476,18 @@
itamar>  		return;
itamar>  	}
itamar>  	dapl_evd_format_overflow_event(overflow_evd_ptr, overflow_event);
itamar> -	dapl_evd_post_event(overflow_evd_ptr, overflow_event);
itamar> +	dapl_evd_post_event(overflow_evd_ptr, overflow_event, flags);
itamar>  
itamar>  	return;
itamar>  }
itamar>  
itamar>  static struct dat_event *dapl_evd_get_and_init_event(DAPL_EVD *evd_ptr,
itamar> -						     enum dat_event_number evno)
itamar> +						     enum dat_event_number evno,
itamar> +							 unsigned long *flags)
itamar>  {
itamar>  	struct dat_event *event_ptr;
itamar>  
itamar> -	event_ptr = dapl_evd_get_event(evd_ptr);
itamar> +	event_ptr = dapl_evd_get_event(evd_ptr, flags);
itamar>  	if (!event_ptr)
itamar>  		dapl_evd_post_overflow_event(evd_ptr->header.owner_ia->
itamar>  					     async_error_evd, evd_ptr);
itamar> @@ -507,7 +507,8 @@
itamar>  				   DAT_CR_HANDLE cr_handle)
itamar>  {
itamar>  	struct dat_event *event_ptr;
itamar> -	event_ptr = dapl_evd_get_and_init_event(evd_ptr, event_number);
itamar> +	unsigned long flags;
itamar> +	event_ptr = dapl_evd_get_and_init_event(evd_ptr, event_number, &flags);
itamar>  	/*
itamar>  	 * Note event lock may be held on successful return
itamar>  	 * to be released by dapl_evd_post_event(), if provider side locking
itamar> @@ -525,7 +526,7 @@
itamar>  	event_ptr->event_data.cr_arrival_event_data.conn_qual = conn_qual;
itamar>  	event_ptr->event_data.cr_arrival_event_data.cr_handle = cr_handle;
itamar>  
itamar> -	dapl_evd_post_event(evd_ptr, event_ptr);
itamar> +	dapl_evd_post_event(evd_ptr, event_ptr, flags);
itamar>  
itamar>  	return DAT_SUCCESS;
itamar>  }
itamar> @@ -537,7 +538,8 @@
itamar>  				   void *private_data)
itamar>  {
itamar>  	struct dat_event *event_ptr;
itamar> -	event_ptr = dapl_evd_get_and_init_event(evd_ptr, event_number);
itamar> +	unsigned long flags;
itamar> +	event_ptr = dapl_evd_get_and_init_event(evd_ptr, event_number, &flags);
itamar>  	/*
itamar>  	 * Note event lock may be held on successful return
itamar>  	 * to be released by dapl_evd_post_event(), if provider side locking
itamar> @@ -554,7 +556,7 @@
itamar>  	    = private_data_size;
itamar>  	event_ptr->event_data.connect_event_data.private_data = private_data;
itamar>  
itamar> -	dapl_evd_post_event(evd_ptr, event_ptr);
itamar> +	dapl_evd_post_event(evd_ptr, event_ptr, flags);
itamar>  
itamar>  	return DAT_SUCCESS;
itamar>  }
itamar> @@ -564,7 +566,8 @@
itamar>  				    DAT_IA_HANDLE ia_handle)
itamar>  {
itamar>  	struct dat_event *event_ptr;
itamar> -	event_ptr = dapl_evd_get_and_init_event(evd_ptr, event_number);
itamar> +	unsigned long flags;
itamar> +	event_ptr = dapl_evd_get_and_init_event(evd_ptr, event_number, &flags);
itamar>  	/*
itamar>  	 * Note event lock may be held on successful return
itamar>  	 * to be released by dapl_evd_post_event(), if provider side locking
itamar> @@ -579,7 +582,7 @@
itamar>  	event_ptr->event_data.asynch_error_event_data.dat_handle =
itamar>  	    (DAT_HANDLE) ia_handle;
itamar>  
itamar> -	dapl_evd_post_event(evd_ptr, event_ptr);
itamar> +	dapl_evd_post_event(evd_ptr, event_ptr, flags);
itamar>  
itamar>  	return DAT_SUCCESS;
itamar>  }
itamar> @@ -589,7 +592,8 @@
itamar>  				 void *pointer)
itamar>  {
itamar>  	struct dat_event *event_ptr;
itamar> -	event_ptr = dapl_evd_get_and_init_event(evd_ptr, event_number);
itamar> +	unsigned long flags;
itamar> +	event_ptr = dapl_evd_get_and_init_event(evd_ptr, event_number, &flags);
itamar>  	/*
itamar>  	 * Note event lock may be held on successful return
itamar>  	 * to be released by dapl_evd_post_event(), if provider side locking
itamar> @@ -603,7 +607,7 @@
itamar>  
itamar>  	event_ptr->event_data.software_event_data.pointer = pointer;
itamar>  
itamar> -	dapl_evd_post_event(evd_ptr, event_ptr);
itamar> +	dapl_evd_post_event(evd_ptr, event_ptr, flags);
itamar>  
itamar>  	return DAT_SUCCESS;
itamar>  }
itamar> 
itamar> -- 
itamar> Itamar
itamar> 



More information about the general mailing list