[openib-general] Re: [PATCH][kdapl] replace spin_lock with spin_lock_irqsave in kdapltest

James Lentini jlentini at netapp.com
Tue May 31 07:46:16 PDT 2005


Itamar,

Why does this patch comment out uses of the g_PerfTestLock? 

james

On Sun, 29 May 2005, Itamar wrote:

itamar> With this patch i can run kdapltest -T T ... -t 4 -w 8 ...
itamar> I still see problems but in general this patch helps the stability a lot.
itamar> 
itamar> replace spin_lock with spin_lock_irqsave in kdapltest
itamar> Signed-off-by: Itamar Rabenstein <itamar at mellanox.co.il>
itamar> 
itamar> Index: test/dapl_transaction_stats.c
itamar> ===================================================================
itamar> --- test/dapl_transaction_stats.c	(revision 2509)
itamar> +++ test/dapl_transaction_stats.c	(working copy)
itamar> @@ -45,12 +45,13 @@
itamar>  DT_transaction_stats_set_ready (DT_Tdep_Print_Head *phead, 
itamar>  				Transaction_Stats_t * transaction_stats)
itamar>  {
itamar> -    DT_Mdep_Lock (&transaction_stats->lock);
itamar> +	unsigned long flags;
itamar> +    spin_lock_irqsave (&transaction_stats->lock,flags);
itamar>      transaction_stats->wait_count--;
itamar>  
itamar>      DT_Tdep_PT_Debug (1,(phead,"Received Sync Message from server (%d left)\n",
itamar>  		    transaction_stats->wait_count));
itamar> -    DT_Mdep_Unlock (&transaction_stats->lock);
itamar> +    spin_unlock_irqrestore (&transaction_stats->lock,flags);
itamar>  }
itamar>  
itamar>  boolean_t
itamar> @@ -86,7 +87,8 @@
itamar>  		       unsigned int bytes_rdma_read,
itamar>  		       unsigned int bytes_rdma_write)
itamar>  {
itamar> -    DT_Mdep_Lock (&transaction_stats->lock);
itamar> +	unsigned long flags;
itamar> +    spin_lock_irqsave (&transaction_stats->lock,flags);
itamar>  
itamar>      /* look for the longest time... */
itamar>      if (time_ms > transaction_stats->time_ms)
itamar> @@ -99,5 +101,5 @@
itamar>      transaction_stats->bytes_recv += bytes_recv;
itamar>      transaction_stats->bytes_rdma_read += bytes_rdma_read;
itamar>      transaction_stats->bytes_rdma_write += bytes_rdma_write;
itamar> -    DT_Mdep_Unlock (&transaction_stats->lock);
itamar> +    spin_unlock_irqrestore (&transaction_stats->lock,flags);
itamar>  }
itamar> Index: test/dapl_server.c
itamar> ===================================================================
itamar> --- test/dapl_server.c	(revision 2509)
itamar> +++ test/dapl_server.c	(working copy)
itamar> @@ -49,7 +49,7 @@
itamar>      unsigned char       *buffp          = NULL;
itamar>      unsigned char       *module         = "DT_cs_Server";
itamar>      int 		status 		= 0;
itamar> -
itamar> +	unsigned long flags;
itamar>      DAT_DTO_COOKIE	dto_cookie;
itamar>      struct dat_dto_completion_event_data dto_stat;
itamar>      u32          ret;
itamar> @@ -616,9 +616,9 @@
itamar>  
itamar>  
itamar>  	/* Count this new client and get ready for the next */
itamar> -	DT_Mdep_Lock (&ps_ptr->num_clients_lock);
itamar> +	spin_lock_irqsave (&ps_ptr->num_clients_lock,flags);
itamar>  	ps_ptr->num_clients++;
itamar> -	DT_Mdep_Unlock (&ps_ptr->num_clients_lock);
itamar> +	spin_unlock_irqrestore (&ps_ptr->num_clients_lock,flags);
itamar>  
itamar>  	/* we passed the pt_ptr to the thread and must now 'forget' it */
itamar>  	pt_ptr = NULL;
itamar> Index: test/dapl_thread.c
itamar> ===================================================================
itamar> --- test/dapl_thread.c	(revision 2509)
itamar> +++ test/dapl_thread.c	(working copy)
itamar> @@ -83,6 +83,7 @@
itamar>  	      unsigned int stacksize)
itamar>  {
itamar>      Thread         *thread_ptr;
itamar> +	unsigned long flags;
itamar>      thread_ptr = (Thread *) DT_MemListAlloc (pt_ptr, "thread.c", THREAD, sizeof (Thread));
itamar>      if (thread_ptr == NULL)
itamar>      {
itamar> @@ -93,9 +94,9 @@
itamar>      thread_ptr->thread_handle = 0;
itamar>      thread_ptr->stacksize = stacksize;
itamar>  
itamar> -    DT_Mdep_Lock (&pt_ptr->Thread_counter_lock);
itamar> +    spin_lock_irqsave (&pt_ptr->Thread_counter_lock,flags);
itamar>      pt_ptr->Thread_counter++;
itamar> -    DT_Mdep_Unlock (&pt_ptr->Thread_counter_lock);
itamar> +    spin_unlock_irqrestore (&pt_ptr->Thread_counter_lock,flags);
itamar>  
itamar>      DT_Mdep_Thread_Init_Attributes (thread_ptr);
itamar>  
itamar> @@ -108,11 +109,12 @@
itamar>  void
itamar>  DT_Thread_Destroy (Thread * thread_ptr, Per_Test_Data_t * pt_ptr)
itamar>  {
itamar> +	unsigned long flags;
itamar>      if (thread_ptr)
itamar>      {
itamar> -	DT_Mdep_Lock (&pt_ptr->Thread_counter_lock);
itamar> +	spin_lock_irqsave (&pt_ptr->Thread_counter_lock,flags);
itamar>  	pt_ptr->Thread_counter--;
itamar> -	DT_Mdep_Unlock (&pt_ptr->Thread_counter_lock);
itamar> +	spin_unlock_irqrestore (&pt_ptr->Thread_counter_lock,flags);
itamar>  
itamar>  	DT_Mdep_Thread_Destroy_Attributes (thread_ptr);
itamar>  	DT_MemListFree (pt_ptr, thread_ptr);
itamar> Index: test/dapl_test_data.c
itamar> ===================================================================
itamar> --- test/dapl_test_data.c	(revision 2509)
itamar> +++ test/dapl_test_data.c	(working copy)
itamar> @@ -27,7 +27,7 @@
itamar>  
itamar>  #include "dapl_proto.h"
itamar>  
itamar> -DT_Mdep_LockType     g_PerfTestLock;
itamar> +/*DT_Mdep_LockType     g_PerfTestLock;*/
itamar>  /*
itamar>   * check memory leaking int             alloc_count; DT_Mdep_LockType
itamar>   * Alloc_Count_Lock;
itamar> Index: test/dapl_transaction_test.c
itamar> ===================================================================
itamar> --- test/dapl_transaction_test.c	(revision 2509)
itamar> +++ test/dapl_transaction_test.c	(working copy)
itamar> @@ -99,7 +99,7 @@
itamar>      unsigned int      i;
itamar>      DT_Tdep_Print_Head *phead;
itamar>      int status = 0;
itamar> -
itamar> +	unsigned long flags;
itamar>      phead = pt_ptr->Params.phead;
itamar>  
itamar>      pt_ptr->Countdown_Counter = cmd->num_threads;
itamar> @@ -129,9 +129,9 @@
itamar>      }
itamar>      DT_Thread_Destroy (pt_ptr->thread, pt_ptr);	    /* destroy Master thread */
itamar>  
itamar> -    DT_Mdep_Lock (&pt_ptr->ps_ptr->num_clients_lock);
itamar> +    spin_lock_irqsave (&pt_ptr->ps_ptr->num_clients_lock,flags);
itamar>      pt_ptr->ps_ptr->num_clients--;
itamar> -    DT_Mdep_Unlock (&pt_ptr->ps_ptr->num_clients_lock);
itamar> +    spin_unlock_irqrestore (&pt_ptr->ps_ptr->num_clients_lock,flags);
itamar>  
itamar>      /* NB: Server has no pt_ptr->remote_netaddr */
itamar>      DT_PrintMemList (pt_ptr);	    /* check if we return all space allocated */
itamar> @@ -239,7 +239,7 @@
itamar>      enum dat_event_number    event_num;
itamar>      DT_Tdep_Print_Head *phead;
itamar>      int status = 0;
itamar> -
itamar> +	unsigned long flags;
itamar>      pt_ptr = test_ptr->pt_ptr;
itamar>      thread = test_ptr->thread;
itamar>      phead = pt_ptr->Params.phead;
itamar> @@ -502,7 +502,7 @@
itamar>       */
itamar>      if (pt_ptr->local_is_server)
itamar>      {
itamar> -	DT_Mdep_Lock (&pt_ptr->Thread_counter_lock);
itamar> +	spin_lock_irqsave (&pt_ptr->Thread_counter_lock,flags);
itamar>  	pt_ptr->Countdown_Counter--;
itamar>  	/* Deliberate pre-decrement.  Post decrement won't
itamar>  	 * work here, so don't do it.
itamar> @@ -512,7 +512,7 @@
itamar>  	    DT_Mdep_wait_object_wakeup (&pt_ptr->synch_wait_object);
itamar>  	}
itamar>  
itamar> -	DT_Mdep_Unlock (&pt_ptr->Thread_counter_lock);
itamar> +	spin_unlock_irqrestore (&pt_ptr->Thread_counter_lock,flags);
itamar>      }
itamar>  
itamar>      for (i = 0;  i < test_ptr->cmd->eps_per_thread;  i++)
itamar> Index: test/dapl_performance_server.c
itamar> ===================================================================
itamar> --- test/dapl_performance_server.c	(revision 2509)
itamar> +++ test/dapl_performance_server.c	(working copy)
itamar> @@ -36,7 +36,7 @@
itamar>      Performance_Test_t 		*test_ptr = NULL;
itamar>      DT_Tdep_Print_Head		*phead;
itamar>      int 			 status = 0;
itamar> -
itamar> +	unsigned long flags;
itamar>      phead = pt_ptr->Params.phead;
itamar>      DT_Tdep_PT_Debug (1,(phead,"Server: Starting performance test\n"));
itamar>  
itamar> @@ -75,9 +75,9 @@
itamar>      DT_Mdep_Thread_Detach (DT_Mdep_Thread_SELF ());	 /* AMM */
itamar>      DT_Thread_Destroy (pt_ptr->thread, pt_ptr);	    /* destroy Master thread */
itamar>  
itamar> -    DT_Mdep_Lock (&pt_ptr->ps_ptr->num_clients_lock);
itamar> +    spin_lock_irqsave (&pt_ptr->ps_ptr->num_clients_lock,flags);
itamar>      pt_ptr->ps_ptr->num_clients--;
itamar> -    DT_Mdep_Unlock (&pt_ptr->ps_ptr->num_clients_lock);
itamar> +    spin_unlock_irqrestore (&pt_ptr->ps_ptr->num_clients_lock,flags);
itamar>  
itamar>      DT_PrintMemList (pt_ptr);	    /* check if we return all space allocated */
itamar>      DT_Mdep_LockDestroy (&pt_ptr->Thread_counter_lock);
itamar> Index: test/dapl_memlist.c
itamar> ===================================================================
itamar> --- test/dapl_memlist.c	(revision 2509)
itamar> +++ test/dapl_memlist.c	(working copy)
itamar> @@ -42,6 +42,7 @@
itamar>  {
itamar>      void           *buffptr;
itamar>      MemListEntry_t *entry_ptr;
itamar> +	unsigned long flags;
itamar>      buffptr = NULL;
itamar>      entry_ptr = NULL;
itamar>  
itamar> @@ -64,10 +65,10 @@
itamar>      entry_ptr->MemType = t;
itamar>      entry_ptr->mem_ptr = buffptr;
itamar>  
itamar> -    DT_Mdep_Lock (&pt_ptr->MemListLock);
itamar> +    spin_lock_irqsave (&pt_ptr->MemListLock,flags);
itamar>      entry_ptr->next = pt_ptr->MemListHead;
itamar>      pt_ptr->MemListHead = entry_ptr;
itamar> -    DT_Mdep_Unlock (&pt_ptr->MemListLock);
itamar> +    spin_unlock_irqrestore (&pt_ptr->MemListLock,flags);
itamar>  
itamar>      return buffptr;
itamar>  }
itamar> @@ -76,12 +77,13 @@
itamar>  DT_MemListFree (Per_Test_Data_t * pt_ptr, void *ptr)
itamar>  {
itamar>      MemListEntry_t *pre, *cur;
itamar> +	unsigned long flags;
itamar>      if (pt_ptr == 0)	      /* not use mem_list */
itamar>      {
itamar>  	DT_Mdep_Free (ptr);
itamar>  	return;
itamar>      }
itamar> -    DT_Mdep_Lock (&pt_ptr->MemListLock);
itamar> +    spin_lock_irqsave (&pt_ptr->MemListLock,flags);
itamar>      pre = NULL;
itamar>      cur = pt_ptr->MemListHead;
itamar>      while (cur)
itamar> @@ -106,7 +108,7 @@
itamar>  	cur = cur->next;
itamar>      }
itamar>  unlock_and_return:
itamar> -    DT_Mdep_Unlock (&pt_ptr->MemListLock);
itamar> +    spin_unlock_irqrestore (&pt_ptr->MemListLock,flags);
itamar>  }
itamar>  
itamar>  void
itamar> @@ -119,9 +121,9 @@
itamar>      };
itamar>      DT_Tdep_Print_Head *phead;
itamar>      MemListEntry_t *cur;
itamar> -
itamar> +	unsigned long flags;
itamar>      phead = pt_ptr->Params.phead;
itamar> -    DT_Mdep_Lock (&pt_ptr->MemListLock);
itamar> +    spin_lock_irqsave (&pt_ptr->MemListLock,flags);
itamar>      cur = pt_ptr->MemListHead;
itamar>      if (cur != 0)
itamar>      {
itamar> @@ -133,5 +135,5 @@
itamar>  		    cur->filename, type[cur->MemType]);
itamar>  	cur = cur->next;
itamar>      }
itamar> -    DT_Mdep_Unlock (&pt_ptr->MemListLock);
itamar> +    spin_unlock_irqrestore (&pt_ptr->MemListLock,flags);
itamar>  }
itamar> Index: include/dapl_test_data.h
itamar> ===================================================================
itamar> --- include/dapl_test_data.h	(revision 2509)
itamar> +++ include/dapl_test_data.h	(working copy)
itamar> @@ -42,7 +42,7 @@
itamar>   * problems on the server side occasionally
itamar>   * the server will reject connections.
itamar>   */
itamar> -extern	 	DT_Mdep_LockType    g_PerfTestLock;
itamar> +/*extern	 	DT_Mdep_LockType    g_PerfTestLock; */
itamar>  
itamar>  /*
itamar>   * check memory leaking extern int              alloc_count ; extern
itamar> Index: kdapl/kdapl_module.c
itamar> ===================================================================
itamar> --- kdapl/kdapl_module.c	(revision 2509)
itamar> +++ kdapl/kdapl_module.c	(working copy)
itamar> @@ -95,10 +95,10 @@
itamar>  				break;
itamar>  			}
itamar>  
itamar> -			DT_Mdep_Lock(&kdapltest_lock);
itamar> +			spin_lock_irq(&kdapltest_lock);
itamar>  			kdapltest_num++;
itamar>  			instance = kdapltest_num;
itamar> -			DT_Mdep_Unlock(&kdapltest_lock);
itamar> +			spin_unlock_irq(&kdapltest_lock);
itamar>  
itamar>  			local_params->phead = KDT_Print_Alloc(instance);
itamar>  			if (!local_params->phead) {
itamar> @@ -201,7 +201,7 @@
itamar>  static __init int kdapltest_init(void)
itamar>  {
itamar>  
itamar> -	DT_Mdep_LockInit(&g_PerfTestLock);	/* For uDAPL, this is done in Tdep_init() */
itamar> +   /* DT_Mdep_LockInit(&g_PerfTestLock);	 For uDAPL, this is done in Tdep_init() */
itamar>  	kdapltest_major = register_chrdev(0, "kdapltest", &kdapltest_fops);
itamar>  	DT_Mdep_LockInit(&kdapltest_lock);
itamar>  	kdapltest_num = 0;
itamar> @@ -218,7 +218,7 @@
itamar>  	KDT_Print_Destroy();
itamar>  	KDT_Evd_Destroy();
itamar>  	DT_Mdep_LockDestroy(&kdapltest_lock);
itamar> -	DT_Mdep_LockDestroy(&g_PerfTestLock);	/* For uDAPL, this is done in Tdep_end() */
itamar> +   /* DT_Mdep_LockDestroy(&g_PerfTestLock);	 For uDAPL, this is done in Tdep_end() */  
itamar>  }
itamar>  
itamar>  module_init(kdapltest_init);
itamar> Index: kdapl/kdapl_tdep_print.c
itamar> ===================================================================
itamar> --- kdapl/kdapl_tdep_print.c	(revision 2509)
itamar> +++ kdapl/kdapl_tdep_print.c	(working copy)
itamar> @@ -106,7 +106,7 @@
itamar>      va_list		args;
itamar>      int			len;
itamar>      Tdep_Print_Entry 	*entry;
itamar> -
itamar> +	unsigned long flags;
itamar>      entry = DT_Mdep_Malloc (sizeof (Tdep_Print_Entry));
itamar>  
itamar>      va_start (args, fmt);
itamar> @@ -120,7 +120,7 @@
itamar>      }
itamar>      entry->next = NULL;
itamar>  
itamar> -    DT_Mdep_Lock (&phead->lock);
itamar> +    spin_lock_irqsave (&phead->lock,flags);
itamar>  
itamar>      if (phead->head == NULL)
itamar>      {
itamar> @@ -133,7 +133,7 @@
itamar>  	phead->tail = entry;
itamar>      }
itamar>      DT_Mdep_wait_object_wakeup (&phead->wait_object);
itamar> -    DT_Mdep_Unlock (&phead->lock);
itamar> +    spin_unlock_irqrestore (&phead->lock,flags);
itamar>  }
itamar>  
itamar>  int
itamar> @@ -143,10 +143,10 @@
itamar>      DT_Tdep_Print_Head  *phead;
itamar>      Tdep_Print_Entry    *entry;
itamar>      int rc;
itamar> -
itamar> +	unsigned long flags;
itamar>      buffer[0] = '\0';
itamar>  
itamar> -    DT_Mdep_Lock (&DT_Print_List_Lock);
itamar> +    spin_lock_irqsave (&DT_Print_List_Lock,flags);
itamar>      phead = DT_Print_List;
itamar>      while (phead)
itamar>      {
itamar> @@ -156,7 +156,7 @@
itamar>  	}
itamar>  	phead = phead->next;
itamar>      }
itamar> -    DT_Mdep_Unlock (&DT_Print_List_Lock);
itamar> +    spin_unlock_irqrestore (&DT_Print_List_Lock,flags);
itamar>  
itamar>      if (!phead)
itamar>      {
itamar> @@ -172,7 +172,7 @@
itamar>  	    return 0;
itamar>  	}
itamar>  
itamar> -	DT_Mdep_Lock (&phead->lock);
itamar> +	spin_lock_irqsave (&phead->lock,flags);
itamar>  	entry = phead->head;
itamar>  	if (entry)
itamar>  	{
itamar> @@ -184,7 +184,7 @@
itamar>  	    strncpy (buffer, entry->buffer, PRINT_MAX);
itamar>  	    DT_Mdep_Free (entry);
itamar>  	}
itamar> -	DT_Mdep_Unlock (&phead->lock);
itamar> +	spin_unlock_irqrestore (&phead->lock,flags);
itamar>      }
itamar>  
itamar>      len = strlen (buffer);
itamar> Index: kdapl/kdapl_tdep_evd.c
itamar> ===================================================================
itamar> --- kdapl/kdapl_tdep_evd.c	(revision 2509)
itamar> +++ kdapl/kdapl_tdep_evd.c	(working copy)
itamar> @@ -105,7 +105,7 @@
itamar>      u32 		dat_status;
itamar>      struct dat_upcall_object upcall;
itamar>      Tdep_Evd		*evd_ptr;
itamar> -
itamar> +	unsigned long flags;
itamar>      evd_ptr = NULL;
itamar>      dat_status = DAT_SUCCESS;
itamar>  
itamar> @@ -139,10 +139,10 @@
itamar>      DT_Mdep_wait_object_init (&evd_ptr->wait_object);
itamar>  
itamar>      /* add evd_ptr to front of evd list */
itamar> -    DT_Mdep_Lock (&DT_Evd_Lock);
itamar> +    spin_lock_irqsave (&DT_Evd_Lock, flags);
itamar>      evd_ptr->evd_next = DT_Evd_List;
itamar>      DT_Evd_List = evd_ptr;
itamar> -    DT_Mdep_Unlock (&DT_Evd_Lock);
itamar> +    spin_unlock_irqrestore (&DT_Evd_Lock, flags);
itamar>  
itamar>  error:
itamar>      if (dat_status != DAT_SUCCESS)
itamar> @@ -161,7 +161,7 @@
itamar>      u32 dat_status;
itamar>      Tdep_Evd   *evd_ptr;    
itamar>      Tdep_Event *event;
itamar> -
itamar> +	unsigned long flags;
itamar>      dat_status = DAT_SUCCESS;
itamar>  
itamar>      /* find the evd_ptr associated with evd_handle */
itamar> @@ -181,7 +181,7 @@
itamar>                 DAT_INTERNAL_ERROR;
itamar>      }
itamar>      /* Get event */
itamar> -    DT_Mdep_Lock (&DT_Evd_Lock);
itamar> +    spin_lock_irqsave (&DT_Evd_Lock, flags);
itamar>      event = evd_ptr->event_next; /* event present in evd_ptr corresponding to the given evd_handle */
itamar>      if (event)
itamar>      {
itamar> @@ -207,7 +207,7 @@
itamar>      {
itamar>          dat_status = DAT_QUEUE_EMPTY;
itamar>      }
itamar> -    DT_Mdep_Unlock (&DT_Evd_Lock);
itamar> +    spin_unlock_irqrestore (&DT_Evd_Lock, flags);
itamar>  
itamar>      return dat_status;
itamar>  }
itamar> @@ -261,7 +261,7 @@
itamar>      }
itamar>  
itamar>      /* Got event */
itamar> -    DT_Mdep_Lock (&DT_Evd_Lock);
itamar> +    spin_lock_irq (&DT_Evd_Lock);
itamar>      event = evd_ptr->event_next;
itamar>      if (event)
itamar>      {
itamar> @@ -286,7 +286,7 @@
itamar>  	}
itamar>      }
itamar>  
itamar> -    DT_Mdep_Unlock (&DT_Evd_Lock);
itamar> +    spin_unlock_irq (&DT_Evd_Lock);
itamar>      return dat_status;
itamar>  }
itamar>  
itamar> @@ -297,7 +297,7 @@
itamar>      Tdep_Evd	*last;
itamar>  
itamar>      last = NULL;
itamar> -    DT_Mdep_Lock (&DT_Evd_Lock);
itamar> +    spin_lock_irq (&DT_Evd_Lock);
itamar>      next = DT_Evd_List;
itamar>      if (next->evd_handle == evd_handle)
itamar>      {
itamar> @@ -315,7 +315,7 @@
itamar>  	    last->evd_next = next->evd_next;
itamar>  	}
itamar>      }
itamar> -    DT_Mdep_Unlock (&DT_Evd_Lock);
itamar> +    spin_unlock_irq (&DT_Evd_Lock);
itamar>  
itamar>      DT_Mdep_Free (next);
itamar>      return dat_evd_free(evd_handle);
itamar> @@ -331,7 +331,7 @@
itamar>  
itamar>      evd_ptr = (Tdep_Evd *) instance_data;
itamar>  
itamar> -    DT_Mdep_Lock (&DT_Evd_Lock);
itamar> +    spin_lock_irq (&DT_Evd_Lock);
itamar>      if (DT_Event_Free_List)
itamar>      {
itamar>  	event = DT_Event_Free_List;
itamar> @@ -356,6 +356,6 @@
itamar>  	    evd_ptr->event_last = event;
itamar>  	}
itamar>      }
itamar> -    DT_Mdep_Unlock (&DT_Evd_Lock);
itamar> +    spin_unlock_irq (&DT_Evd_Lock);
itamar>      DT_Mdep_wait_object_wakeup (&evd_ptr->wait_object);
itamar>  }
itamar> -- 
itamar> Itamar
itamar> 



More information about the general mailing list