From 1acf256d96ab7ec881c40f4eeeef2e064c7e7d20 Mon Sep 17 00:00:00 2001 From: John Baldwin Date: Wed, 30 May 2001 14:35:22 +0000 Subject: [PATCH] We can't grab the sched_lock in set_user_ldt() because when it is called from cpu_switch(), curproc has been changed, but the sched_lock owner will not be updated until we return to mi_switch(), thus we deadlock against ourselves. As a workaround, push the acquire and release of sched_lock out to the callers of set_user_ldt(). Note that we can't use a mtx_assert() in set_user_ldt for the same reason. Sleuting by: tmm Tested by: tmm, dougb --- sys/amd64/amd64/sys_machdep.c | 27 ++++++++++++++++++++++----- sys/amd64/amd64/vm_machdep.c | 4 +++- sys/i386/i386/sys_machdep.c | 27 ++++++++++++++++++++++----- sys/i386/i386/vm_machdep.c | 4 +++- 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/sys/amd64/amd64/sys_machdep.c b/sys/amd64/amd64/sys_machdep.c index 1df546c10432..1bcc9ad7cbee 100644 --- a/sys/amd64/amd64/sys_machdep.c +++ b/sys/amd64/amd64/sys_machdep.c @@ -240,16 +240,16 @@ i386_get_ioperm(p, args) /* * Update the GDT entry pointing to the LDT to point to the LDT of the * current process. + * + * This must be called with sched_lock held. Unfortunately, we can't use a + * mtx_assert() here because cpu_switch() calls this function after changing + * curproc but before sched_lock's owner is updated in mi_switch(). */ void set_user_ldt(struct pcb *pcb) { struct pcb_ldt *pcb_ldt; - if (pcb != PCPU_GET(curpcb)) - return; - - mtx_lock_spin(&sched_lock); pcb_ldt = pcb->pcb_ldt; #ifdef SMP gdt[PCPU_GET(cpuid) * NGDT + GUSERLDT_SEL].sd = pcb_ldt->ldt_sd; @@ -258,6 +258,17 @@ set_user_ldt(struct pcb *pcb) #endif lldt(GSEL(GUSERLDT_SEL, SEL_KPL)); PCPU_SET(currentldt, GSEL(GUSERLDT_SEL, SEL_KPL)); +} + +void +set_user_ldt_rv(struct pcb *pcb) +{ + + if (pcb != PCPU_GET(curpcb)) + return; + + mtx_lock_spin(&sched_lock); + set_user_ldt(pcb); mtx_unlock_spin(&sched_lock); } @@ -422,15 +433,21 @@ i386_set_ldt(p, args) kmem_free(kernel_map, (vm_offset_t)old_ldt_base, old_ldt_len * sizeof(union descriptor)); FREE(new_ldt, M_SUBPROC); +#ifndef SMP + mtx_lock_spin(&sched_lock); +#endif } else { pcb->pcb_ldt = pcb_ldt = new_ldt; +#ifdef SMP mtx_unlock_spin(&sched_lock); +#endif } #ifdef SMP /* signal other cpus to reload ldt */ - smp_rendezvous(NULL, (void (*)(void *))set_user_ldt, NULL, pcb); + smp_rendezvous(NULL, (void (*)(void *))set_user_ldt_rv, NULL, pcb); #else set_user_ldt(pcb); + mtx_unlock_spin(&sched_lock); #endif } diff --git a/sys/amd64/amd64/vm_machdep.c b/sys/amd64/amd64/vm_machdep.c index 44dc9d05e6cb..89762bc1fd0a 100644 --- a/sys/amd64/amd64/vm_machdep.c +++ b/sys/amd64/amd64/vm_machdep.c @@ -132,9 +132,11 @@ cpu_fork(p1, p2, flags) struct pcb_ldt *pcb_ldt = pcb1->pcb_ldt; if (pcb_ldt && pcb_ldt->ldt_refcnt > 1) { pcb_ldt = user_ldt_alloc(pcb1,pcb_ldt->ldt_len); - user_ldt_free(pcb1); + if (pcb_ldt == NULL) + panic("could not copy LDT"); pcb1->pcb_ldt = pcb_ldt; set_user_ldt(pcb1); + user_ldt_free(pcb1); } } return; diff --git a/sys/i386/i386/sys_machdep.c b/sys/i386/i386/sys_machdep.c index 1df546c10432..1bcc9ad7cbee 100644 --- a/sys/i386/i386/sys_machdep.c +++ b/sys/i386/i386/sys_machdep.c @@ -240,16 +240,16 @@ i386_get_ioperm(p, args) /* * Update the GDT entry pointing to the LDT to point to the LDT of the * current process. + * + * This must be called with sched_lock held. Unfortunately, we can't use a + * mtx_assert() here because cpu_switch() calls this function after changing + * curproc but before sched_lock's owner is updated in mi_switch(). */ void set_user_ldt(struct pcb *pcb) { struct pcb_ldt *pcb_ldt; - if (pcb != PCPU_GET(curpcb)) - return; - - mtx_lock_spin(&sched_lock); pcb_ldt = pcb->pcb_ldt; #ifdef SMP gdt[PCPU_GET(cpuid) * NGDT + GUSERLDT_SEL].sd = pcb_ldt->ldt_sd; @@ -258,6 +258,17 @@ set_user_ldt(struct pcb *pcb) #endif lldt(GSEL(GUSERLDT_SEL, SEL_KPL)); PCPU_SET(currentldt, GSEL(GUSERLDT_SEL, SEL_KPL)); +} + +void +set_user_ldt_rv(struct pcb *pcb) +{ + + if (pcb != PCPU_GET(curpcb)) + return; + + mtx_lock_spin(&sched_lock); + set_user_ldt(pcb); mtx_unlock_spin(&sched_lock); } @@ -422,15 +433,21 @@ i386_set_ldt(p, args) kmem_free(kernel_map, (vm_offset_t)old_ldt_base, old_ldt_len * sizeof(union descriptor)); FREE(new_ldt, M_SUBPROC); +#ifndef SMP + mtx_lock_spin(&sched_lock); +#endif } else { pcb->pcb_ldt = pcb_ldt = new_ldt; +#ifdef SMP mtx_unlock_spin(&sched_lock); +#endif } #ifdef SMP /* signal other cpus to reload ldt */ - smp_rendezvous(NULL, (void (*)(void *))set_user_ldt, NULL, pcb); + smp_rendezvous(NULL, (void (*)(void *))set_user_ldt_rv, NULL, pcb); #else set_user_ldt(pcb); + mtx_unlock_spin(&sched_lock); #endif } diff --git a/sys/i386/i386/vm_machdep.c b/sys/i386/i386/vm_machdep.c index 44dc9d05e6cb..89762bc1fd0a 100644 --- a/sys/i386/i386/vm_machdep.c +++ b/sys/i386/i386/vm_machdep.c @@ -132,9 +132,11 @@ cpu_fork(p1, p2, flags) struct pcb_ldt *pcb_ldt = pcb1->pcb_ldt; if (pcb_ldt && pcb_ldt->ldt_refcnt > 1) { pcb_ldt = user_ldt_alloc(pcb1,pcb_ldt->ldt_len); - user_ldt_free(pcb1); + if (pcb_ldt == NULL) + panic("could not copy LDT"); pcb1->pcb_ldt = pcb_ldt; set_user_ldt(pcb1); + user_ldt_free(pcb1); } } return;