Subject: re: kern/34895: panic: sched_lock: locked
To: enami tsugutomo <enami@sm.sony.co.jp>
From: matthew green <mrg@eterna.com.au>
List: netbsd-bugs
Date: 10/26/2006 18:15:29
   >   	/* XXXSMP: works, but icky */
   >   	if (dolock)
   >   		SCHED_LOCK(s);
   > + 	else
   > + 		newkp = NULL;
   
   Are you sure that !dolock means the scheduler lock is already held?



hmmm, infact, kpsignal2() is sometimes called with it locked.  i
noticed a problem with the previous patch (the above else block
belongs with the above if() -- see the patch below), but i am
now realising that the (!dolock) case we can't call pool_get()
at all since the sched_lock is held the entire time... but the
patch below should avoid calling pool_get() in some of these
cases, i think restoring to the previous behaviour, plus bug
fixes..

but this should probably be reworked to panic() or otherwise
fail somehow if pool_get() is needed when kpsignal2() is called
with sched_lock already held... possibly this can only be
fixed properly by reworking to avoid this case.  all the uses
of "dolock" in kpsignal2() are marked XXX SMP....


.mrg.


Index: kern_sig.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_sig.c,v
retrieving revision 1.231
diff -p -r1.231 kern_sig.c
*** kern_sig.c	22 Oct 2006 20:48:45 -0000	1.231
--- kern_sig.c	26 Oct 2006 08:09:59 -0000
*************** kpsignal2(struct proc *p, const ksiginfo
*** 1153,1168 ****
  	}
  
  	/*
! 	 * Allocate a ksiginfo_t incase we need to insert it with
! 	 * the scheduler lock held.
  	 */
! 	newkp = pool_get(&ksiginfo_pool, PR_NOWAIT);
! 	if (newkp == NULL) {
  #ifdef DIAGNOSTIC
! 		printf("kpsignal2: couldn't allocated from ksiginfo_pool\n");
  #endif
! 		return;
! 	}
  
  	/* XXXSMP: works, but icky */
  	if (dolock)
--- 1153,1171 ----
  	}
  
  	/*
! 	 * Allocate a ksiginfo_t incase we need to insert it with the
! 	 * scheduler lock held, but only if this ksiginfo_t isn't empty.
  	 */
! 	if (dolock && !KSI_EMPTY_P(ksi)) {
! 		newkp = pool_get(&ksiginfo_pool, PR_NOWAIT);
! 		if (newkp == NULL) {
  #ifdef DIAGNOSTIC
! 			printf("kpsignal2: couldn't allocated from ksiginfo_pool\n");
  #endif
! 			return;
! 		}
! 	} else
! 		newkp = NULL;
  
  	/* XXXSMP: works, but icky */
  	if (dolock)