From 6d69006c55a01bc62f55b1d4623fc13457564a16 Mon Sep 17 00:00:00 2001 From: bde Date: Sat, 27 Sep 2003 10:30:03 +0000 Subject: [PATCH] Quick fix for bitrot in locking in the SMP case. cd_getreg() and cd_setreg() were still using !(read_eflags() & PSL_I) as the condition for the lock hidden by COM_LOCK() (if any) being held. This worked when spin mutexes and/or critical_enter() used hard interrupt disablement, but it has caused recursion on the non-recursive mutex com_mtx since all relevant interrupt disablement became soft. The recursion is harmless unless there are other bugs, but it breaks an invariant so it is fatal if spinlocks are witnessed. --- sys/dev/cy/cy.c | 28 ++++++++++++++++++++++++---- sys/dev/cy/cy_isa.c | 28 ++++++++++++++++++++++++---- sys/i386/isa/cy.c | 28 ++++++++++++++++++++++++---- 3 files changed, 72 insertions(+), 12 deletions(-) diff --git a/sys/dev/cy/cy.c b/sys/dev/cy/cy.c index c8f24599352b..4bbedba65fec 100644 --- a/sys/dev/cy/cy.c +++ b/sys/dev/cy/cy.c @@ -2852,6 +2852,9 @@ cd_getreg(com, reg) int cy_align; register_t eflags; cy_addr iobase; +#ifdef SMP + int need_unlock; +#endif int val; basecom = com_addr(com->unit & ~(CD1400_NO_OF_CHANNELS - 1)); @@ -2860,13 +2863,20 @@ cd_getreg(com, reg) iobase = com->iobase; eflags = read_eflags(); critical_enter(); - if (eflags & PSL_I) +#ifdef SMP + need_unlock = 0; + if (!mtx_owned(&com_mtx)) { COM_LOCK(); + need_unlock = 1; + } +#endif if (basecom->car != car) cd_outb(iobase, CD1400_CAR, cy_align, basecom->car = car); val = cd_inb(iobase, reg, cy_align); - if (eflags & PSL_I) +#ifdef SMP + if (need_unlock) COM_UNLOCK(); +#endif critical_exit(); return (val); } @@ -2882,6 +2892,9 @@ cd_setreg(com, reg, val) int cy_align; register_t eflags; cy_addr iobase; +#ifdef SMP + int need_unlock; +#endif basecom = com_addr(com->unit & ~(CD1400_NO_OF_CHANNELS - 1)); car = com->unit & CD1400_CAR_CHAN; @@ -2889,13 +2902,20 @@ cd_setreg(com, reg, val) iobase = com->iobase; eflags = read_eflags(); critical_enter(); - if (eflags & PSL_I) +#ifdef SMP + need_unlock = 0; + if (!mtx_owned(&com_mtx)) { COM_LOCK(); + need_unlock = 1; + } +#endif if (basecom->car != car) cd_outb(iobase, CD1400_CAR, cy_align, basecom->car = car); cd_outb(iobase, reg, cy_align, val); - if (eflags & PSL_I) +#ifdef SMP + if (need_unlock) COM_UNLOCK(); +#endif critical_exit(); } diff --git a/sys/dev/cy/cy_isa.c b/sys/dev/cy/cy_isa.c index c8f24599352b..4bbedba65fec 100644 --- a/sys/dev/cy/cy_isa.c +++ b/sys/dev/cy/cy_isa.c @@ -2852,6 +2852,9 @@ cd_getreg(com, reg) int cy_align; register_t eflags; cy_addr iobase; +#ifdef SMP + int need_unlock; +#endif int val; basecom = com_addr(com->unit & ~(CD1400_NO_OF_CHANNELS - 1)); @@ -2860,13 +2863,20 @@ cd_getreg(com, reg) iobase = com->iobase; eflags = read_eflags(); critical_enter(); - if (eflags & PSL_I) +#ifdef SMP + need_unlock = 0; + if (!mtx_owned(&com_mtx)) { COM_LOCK(); + need_unlock = 1; + } +#endif if (basecom->car != car) cd_outb(iobase, CD1400_CAR, cy_align, basecom->car = car); val = cd_inb(iobase, reg, cy_align); - if (eflags & PSL_I) +#ifdef SMP + if (need_unlock) COM_UNLOCK(); +#endif critical_exit(); return (val); } @@ -2882,6 +2892,9 @@ cd_setreg(com, reg, val) int cy_align; register_t eflags; cy_addr iobase; +#ifdef SMP + int need_unlock; +#endif basecom = com_addr(com->unit & ~(CD1400_NO_OF_CHANNELS - 1)); car = com->unit & CD1400_CAR_CHAN; @@ -2889,13 +2902,20 @@ cd_setreg(com, reg, val) iobase = com->iobase; eflags = read_eflags(); critical_enter(); - if (eflags & PSL_I) +#ifdef SMP + need_unlock = 0; + if (!mtx_owned(&com_mtx)) { COM_LOCK(); + need_unlock = 1; + } +#endif if (basecom->car != car) cd_outb(iobase, CD1400_CAR, cy_align, basecom->car = car); cd_outb(iobase, reg, cy_align, val); - if (eflags & PSL_I) +#ifdef SMP + if (need_unlock) COM_UNLOCK(); +#endif critical_exit(); } diff --git a/sys/i386/isa/cy.c b/sys/i386/isa/cy.c index c8f24599352b..4bbedba65fec 100644 --- a/sys/i386/isa/cy.c +++ b/sys/i386/isa/cy.c @@ -2852,6 +2852,9 @@ cd_getreg(com, reg) int cy_align; register_t eflags; cy_addr iobase; +#ifdef SMP + int need_unlock; +#endif int val; basecom = com_addr(com->unit & ~(CD1400_NO_OF_CHANNELS - 1)); @@ -2860,13 +2863,20 @@ cd_getreg(com, reg) iobase = com->iobase; eflags = read_eflags(); critical_enter(); - if (eflags & PSL_I) +#ifdef SMP + need_unlock = 0; + if (!mtx_owned(&com_mtx)) { COM_LOCK(); + need_unlock = 1; + } +#endif if (basecom->car != car) cd_outb(iobase, CD1400_CAR, cy_align, basecom->car = car); val = cd_inb(iobase, reg, cy_align); - if (eflags & PSL_I) +#ifdef SMP + if (need_unlock) COM_UNLOCK(); +#endif critical_exit(); return (val); } @@ -2882,6 +2892,9 @@ cd_setreg(com, reg, val) int cy_align; register_t eflags; cy_addr iobase; +#ifdef SMP + int need_unlock; +#endif basecom = com_addr(com->unit & ~(CD1400_NO_OF_CHANNELS - 1)); car = com->unit & CD1400_CAR_CHAN; @@ -2889,13 +2902,20 @@ cd_setreg(com, reg, val) iobase = com->iobase; eflags = read_eflags(); critical_enter(); - if (eflags & PSL_I) +#ifdef SMP + need_unlock = 0; + if (!mtx_owned(&com_mtx)) { COM_LOCK(); + need_unlock = 1; + } +#endif if (basecom->car != car) cd_outb(iobase, CD1400_CAR, cy_align, basecom->car = car); cd_outb(iobase, reg, cy_align, val); - if (eflags & PSL_I) +#ifdef SMP + if (need_unlock) COM_UNLOCK(); +#endif critical_exit(); }