Date: Tue, 1 Mar 2005 21:29:34 -0800 (PST) From: Doug White <dwhite@gumbysoft.com> To: freebsd-sparc64@freebsd.org Subject: Deadlock fix candidate Message-ID: <20050301211852.C73061@carver.gumbysoft.com>
next in thread | raw e-mail | index | archive | help
Hey folks, After consultation with alc and rwatson and (limited) testing by kris, I've come up the following patch as a fix for the sparc64 deadlocks that afflict the package cluster sparc boxes. Its an exact copy of what alc suggested (thanks :) ) and by assembly inspection generates the desired code. Originally I was going to redo the loop to be more like ipi_wait() with the declared volatile pointer, but a cast does the job. :) I'm trying to determine if this also fixes similar deadlocks/hangs on i386 and amd64. I haven't seen any assembly level changes with the patch on amd64, but I'm trying again on a fresh source tree in case I have pollution from test patches that cvsup hasn't caught. I can only reproduce the hang on amd64, but adding any sort of debugging makes it go away. Comments appreciated. If things look good I'll look to commit this tomorrow evening unless events dictate otherwise. Thanks! (Patch also available at http://people.freebsd.org/~dwhite/uipc_mbuf.c.20050301.patch) Index: uipc_mbuf.c =================================================================== RCS file: /home/ncvs/src/sys/kern/uipc_mbuf.c,v retrieving revision 1.143 diff -u -r1.143 uipc_mbuf.c --- uipc_mbuf.c 24 Feb 2005 00:40:33 -0000 1.143 +++ uipc_mbuf.c 1 Mar 2005 19:27:24 -0000 @@ -234,9 +234,12 @@ * This is tricky. We need to make sure to decrement the * refcount in a safe way but to also clean up if we're the * last reference. This method seems to do it without race. + * The volatile cast is required to emit the proper load + * instructions. Otherwise gcc will optimize the read outside + * of the while loop. */ while (dofree == 0) { - cnt = *(m->m_ext.ref_cnt); + cnt = *(volatile u_int *)(m->m_ext.ref_cnt); if (atomic_cmpset_int(m->m_ext.ref_cnt, cnt, cnt - 1)) { if (cnt == 1) dofree = 1; -- Doug White | FreeBSD: The Power to Serve dwhite@gumbysoft.com | www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050301211852.C73061>