From 8919f505663760160bcf616f4755a2e4ff4863d7 Mon Sep 17 00:00:00 2001 From: thorpej Date: Thu, 28 Jul 2005 13:57:06 +0000 Subject: [PATCH] - Change FPCPU_LOCK() such that the caller is responsible for blocking IPIs in the MULTIPROCESSOR case. Adjust all callers. - In fpusave_cpu(), block IPIs for the entire duration (while we have CPUF_FPUSAVE set in ci_flags) to fix the deadlock that leads to "panic: fpsave ipi didn't", as described in PR port-alpha/26383. Many thanks to Michael Hitch for the analysis and initial patch which this one is derived from. --- sys/arch/alpha/alpha/machdep.c | 27 ++++++++++++++++++--------- sys/arch/alpha/alpha/trap.c | 14 ++++++++++---- sys/arch/alpha/include/pcb.h | 24 ++++++------------------ 3 files changed, 34 insertions(+), 31 deletions(-) diff --git a/sys/arch/alpha/alpha/machdep.c b/sys/arch/alpha/alpha/machdep.c index e282b3700876..59d453b62516 100644 --- a/sys/arch/alpha/alpha/machdep.c +++ b/sys/arch/alpha/alpha/machdep.c @@ -1,4 +1,4 @@ -/* $NetBSD: machdep.c,v 1.286 2005/06/03 15:06:40 jdc Exp $ */ +/* $NetBSD: machdep.c,v 1.287 2005/07/28 13:57:06 thorpej Exp $ */ /*- * Copyright (c) 1998, 1999, 2000 The NetBSD Foundation, Inc. @@ -75,7 +75,7 @@ #include /* RCS ID & Copyright macro defns */ -__KERNEL_RCSID(0, "$NetBSD: machdep.c,v 1.286 2005/06/03 15:06:40 jdc Exp $"); +__KERNEL_RCSID(0, "$NetBSD: machdep.c,v 1.287 2005/07/28 13:57:06 thorpej Exp $"); #include #include @@ -1706,6 +1706,7 @@ fpusave_cpu(struct cpu_info *ci, int save) KDASSERT(ci == curcpu()); #if defined(MULTIPROCESSOR) + s = splhigh(); /* block IPIs for the duration */ atomic_setbits_ulong(&ci->ci_flags, CPUF_FPUSAVE); #endif @@ -1720,16 +1721,17 @@ fpusave_cpu(struct cpu_info *ci, int save) alpha_pal_wrfen(0); - FPCPU_LOCK(&l->l_addr->u_pcb, s); + FPCPU_LOCK(&l->l_addr->u_pcb); l->l_addr->u_pcb.pcb_fpcpu = NULL; ci->ci_fpcurlwp = NULL; - FPCPU_UNLOCK(&l->l_addr->u_pcb, s); + FPCPU_UNLOCK(&l->l_addr->u_pcb); out: #if defined(MULTIPROCESSOR) atomic_clearbits_ulong(&ci->ci_flags, CPUF_FPUSAVE); + splx(s); #endif return; } @@ -1749,25 +1751,32 @@ fpusave_proc(struct lwp *l, int save) KDASSERT(l->l_addr != NULL); - FPCPU_LOCK(&l->l_addr->u_pcb, s); +#if defined(MULTIPROCESSOR) + s = splhigh(); /* block IPIs for the duration */ +#endif + FPCPU_LOCK(&l->l_addr->u_pcb); oci = l->l_addr->u_pcb.pcb_fpcpu; if (oci == NULL) { - FPCPU_UNLOCK(&l->l_addr->u_pcb, s); + FPCPU_UNLOCK(&l->l_addr->u_pcb); +#if defined(MULTIPROCESSOR) + splx(s); +#endif return; } #if defined(MULTIPROCESSOR) if (oci == ci) { KASSERT(ci->ci_fpcurlwp == l); - FPCPU_UNLOCK(&l->l_addr->u_pcb, s); + FPCPU_UNLOCK(&l->l_addr->u_pcb); + splx(s); fpusave_cpu(ci, save); return; } KASSERT(oci->ci_fpcurlwp == l); alpha_send_ipi(oci->ci_cpuid, ipi); - FPCPU_UNLOCK(&l->l_addr->u_pcb, s); + FPCPU_UNLOCK(&l->l_addr->u_pcb); spincount = 0; while (l->l_addr->u_pcb.pcb_fpcpu != NULL) { @@ -1778,7 +1787,7 @@ fpusave_proc(struct lwp *l, int save) } #else KASSERT(ci->ci_fpcurlwp == l); - FPCPU_UNLOCK(&l->l_addr->u_pcb, s); + FPCPU_UNLOCK(&l->l_addr->u_pcb); fpusave_cpu(ci, save); #endif /* MULTIPROCESSOR */ } diff --git a/sys/arch/alpha/alpha/trap.c b/sys/arch/alpha/alpha/trap.c index 778d273fcc2f..9fdd635d64d0 100644 --- a/sys/arch/alpha/alpha/trap.c +++ b/sys/arch/alpha/alpha/trap.c @@ -1,4 +1,4 @@ -/* $NetBSD: trap.c,v 1.97 2005/06/01 16:09:01 drochner Exp $ */ +/* $NetBSD: trap.c,v 1.98 2005/07/28 13:57:06 thorpej Exp $ */ /*- * Copyright (c) 2000, 2001 The NetBSD Foundation, Inc. @@ -100,7 +100,7 @@ #include /* RCS ID & Copyright macro defns */ -__KERNEL_RCSID(0, "$NetBSD: trap.c,v 1.97 2005/06/01 16:09:01 drochner Exp $"); +__KERNEL_RCSID(0, "$NetBSD: trap.c,v 1.98 2005/07/28 13:57:06 thorpej Exp $"); #include #include @@ -634,12 +634,18 @@ alpha_enable_fp(struct lwp *l, int check) KDASSERT(l->l_addr->u_pcb.pcb_fpcpu == NULL); #endif - FPCPU_LOCK(&l->l_addr->u_pcb, s); +#if defined(MULTIPROCESSOR) + s = splhigh(); /* block IPIs */ +#endif + FPCPU_LOCK(&l->l_addr->u_pcb); l->l_addr->u_pcb.pcb_fpcpu = ci; ci->ci_fpcurlwp = l; - FPCPU_UNLOCK(&l->l_addr->u_pcb, s); + FPCPU_UNLOCK(&l->l_addr->u_pcb); +#if defined(MULTIPROCESSOR) + splx(s); +#endif /* * Instrument FP usage -- if a process had not previously diff --git a/sys/arch/alpha/include/pcb.h b/sys/arch/alpha/include/pcb.h index 56ca9f70cd8d..e5fd7bd76dd6 100644 --- a/sys/arch/alpha/include/pcb.h +++ b/sys/arch/alpha/include/pcb.h @@ -1,4 +1,4 @@ -/* $NetBSD: pcb.h,v 1.12 2003/06/29 22:28:05 fvdl Exp $ */ +/* $NetBSD: pcb.h,v 1.13 2005/07/28 13:57:06 thorpej Exp $ */ /* * Copyright (c) 1994, 1995, 1996 Carnegie-Mellon University. @@ -64,25 +64,13 @@ struct pcb { struct simplelock pcb_fpcpu_slock; /* simple lock on fpcpu [SW] */ }; -#if defined(MULTIPROCESSOR) /* - * Need to block IPIs while holding the fpcpu_slock. + * MULTIPROCESSOR: + * Need to block IPIs while holding the fpcpu_slock. That is the + * responsibility of the CALLER! */ -#define FPCPU_LOCK(pcb, s) \ -do { \ - (s) = splhigh(); \ - simple_lock(&(pcb)->pcb_fpcpu_slock); \ -} while (/*CONSTCOND*/0) - -#define FPCPU_UNLOCK(pcb, s) \ -do { \ - simple_unlock(&(pcb)->pcb_fpcpu_slock); \ - splx((s)); \ -} while (/*CONSTCOND*/0) -#else -#define FPCPU_LOCK(pcb, s) simple_lock(&(pcb)->pcb_fpcpu_slock) -#define FPCPU_UNLOCK(pcb, s) simple_unlock(&(pcb)->pcb_fpcpu_slock) -#endif /* MULTIPROCESSOR */ +#define FPCPU_LOCK(pcb) simple_lock(&(pcb)->pcb_fpcpu_slock) +#define FPCPU_UNLOCK(pcb) simple_unlock(&(pcb)->pcb_fpcpu_slock) /* * The pcb is augmented with machine-dependent additional data for