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

[James Lentini]

Wow, I didn't realize that we did this in the code. 

Can we simplify this? I'd rather not have the code taking a lock in 
one function and releasing it in another. This code came from the 
generic source forge implementation. Do we need to do 
"evd producer locking" for OpenIB? If not we can remove these locks.

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>