Date: Tue, 9 Jul 2002 17:25:01 -0400 (EDT) From: "Andrew R. Reiter" <arr@watson.org> To: John Baldwin <jhb@freebsd.org> Cc: Perforce Change Reviews <perforce@freebsd.org> Subject: Re: PERFORCE change 13995 for review Message-ID: <Pine.NEB.3.96L.1020709172404.57544M-100000@fledge.watson.org> In-Reply-To: <200207092121.g69LLYBh092119@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 9 Jul 2002, John Baldwin wrote: :http://people.freebsd.org/~peter/p4db/chv.cgi?CH=13995 : :Change 13995 by jhb@jhb_laptop on 2002/07/09 14:21:31 : : Get rid of gross mtx_owned()'s. This still isn't pretty. : I need to figure out a better way of handling this. : :Affected files ... : :.. //depot/projects/smpng/sys/i386/i386/machdep.c#40 edit :.. //depot/projects/smpng/sys/i386/i386/sys_machdep.c#20 edit :.. //depot/projects/smpng/sys/i386/i386/vm_machdep.c#22 edit :.. //depot/projects/smpng/sys/i386/include/proc.h#7 edit : :Differences ... : :==== //depot/projects/smpng/sys/i386/i386/machdep.c#40 (text+ko) ==== : :@@ -825,8 +825,11 @@ : struct trapframe *regs = td->td_frame; : struct pcb *pcb = td->td_pcb; : :+ mtx_lock_spin(&sched_lock); : if (td->td_proc->p_md.md_ldt) : user_ldt_free(td); :+ else :+ mtx_unlock_spin(&sched_lock); : : bzero((char *)regs, sizeof(struct trapframe)); : regs->tf_eip = entry; : :==== //depot/projects/smpng/sys/i386/i386/sys_machdep.c#20 (text+ko) ==== : :@@ -286,7 +286,7 @@ : #endif : : /* :- * Must be called with either sched_lock free or held but not recursed. :+ * Must be called with sched_lock held but not recursed. : * If it does not return NULL, it will return with it owned. : */ : struct proc_ldt * :@@ -294,9 +294,8 @@ : { : struct proc_ldt *pldt, *new_ldt; : :- if (mtx_owned(&sched_lock)) :- mtx_unlock_spin(&sched_lock); :- mtx_assert(&sched_lock, MA_NOTOWNED); :+ mtx_assert(&sched_lock, MA_OWNED | MA_NOTRECURSED); :+ mtx_unlock_spin(&sched_lock); : MALLOC(new_ldt, struct proc_ldt *, sizeof(struct proc_ldt), : M_SUBPROC, M_WAITOK); : :@@ -327,7 +326,7 @@ : } : : /* :- * Must be called either with sched_lock free or held but not recursed. :+ * Must be called with sched_lock held but not recursed. : * If md_ldt is not NULL, it will return with sched_lock released. : */ : void :@@ -339,8 +338,6 @@ : if (pldt == NULL) : return; : :- if (!mtx_owned(&sched_lock)) :- mtx_lock_spin(&sched_lock); : mtx_assert(&sched_lock, MA_OWNED | MA_NOTRECURSED); : if (td == PCPU_GET(curthread)) { : lldt(_default_ldt); :@@ -407,7 +404,7 @@ : int error = 0, i, n; : int largest_ld; : struct mdproc *mdp = &td->td_proc->p_md; :- struct proc_ldt *pldt = mdp->md_ldt; :+ struct proc_ldt *pldt; : struct i386_ldt_args ua, *uap = &ua; : caddr_t old_ldt_base; : int old_ldt_len; :@@ -432,8 +429,12 @@ : return(EINVAL); : : /* allocate user ldt */ :+ mtx_lock_spin(&sched_lock); :+ pldt = mdp->md_ldt; : if (!pldt || largest_ld >= pldt->ldt_len) { :- struct proc_ldt *new_ldt = user_ldt_alloc(mdp, largest_ld); :+ struct proc_ldt *new_ldt; :+ :+ new_ldt = user_ldt_alloc(mdp, largest_ld); : if (new_ldt == NULL) : return ENOMEM; If this change is going to happen, might as well push the declaration northward towards the top of the function, no? Cheers, Andrew : if (pldt) { :@@ -463,7 +464,8 @@ : set_user_ldt(mdp); : mtx_unlock_spin(&sched_lock); : #endif :- } :+ } else :+ mtx_unlock_spin(&sched_lock); : : /* Check descriptors for access violations */ : for (i = 0, n = uap->start; i < uap->num; i++, n++) { : :==== //depot/projects/smpng/sys/i386/i386/vm_machdep.c#22 (text+ko) ==== : :@@ -135,7 +135,10 @@ : if ((flags & RFMEM) == 0) { : /* unshare user LDT */ : struct mdproc *mdp1 = &p1->p_md; :- struct proc_ldt *pldt = mdp1->md_ldt; :+ struct proc_ldt *pldt; :+ :+ mtx_lock_spin(&sched_lock); :+ pltd = mdp1->md_ldt; : if (pldt && pldt->ldt_refcnt > 1) { : pldt = user_ldt_alloc(mdp1, pldt->ldt_len); : if (pldt == NULL) :@@ -143,7 +146,8 @@ : mdp1->md_ldt = pldt; : set_user_ldt(mdp1); : user_ldt_free(td1); :- } :+ } else :+ mtx_unlock_spin(&sched_lock); : } : return; : } :@@ -210,7 +214,7 @@ : : /* Copy the LDT, if necessary. */ : mtx_lock_spin(&sched_lock); :- if (mdp2->md_ldt != 0) { :+ if (mdp2->md_ldt != NULL) { : if (flags & RFMEM) { : mdp2->md_ldt->ldt_refcnt++; : } else { :@@ -271,8 +275,11 @@ : ctob(IOPAGES + 1)); : pcb->pcb_ext = 0; : } :+ mtx_lock_spin(&sched_lock); : if (mdp->md_ldt) : user_ldt_free(td); :+ else :+ mtx_unlock_spin(&sched_lock); : if (pcb->pcb_flags & PCB_DBREGS) { : /* : * disable all hardware breakpoints : :==== //depot/projects/smpng/sys/i386/include/proc.h#7 (text+ko) ==== : :@@ -55,7 +55,7 @@ : }; : : struct mdproc { :- struct proc_ldt *md_ldt; /* per-process ldt */ :+ struct proc_ldt *md_ldt; /* (j) per-process ldt */ : }; : : #ifdef _KERNEL : -- Andrew R. Reiter arr@watson.org arr@FreeBSD.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe p4-projects" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1020709172404.57544M-100000>