On 02/01/18(Tue) 17:35, Mark Kettenis wrote:
> > Date: Tue, 2 Jan 2018 16:02:16 +0100
> > From: Martin Pieuchot <m...@openbsd.org>
> >
> > We're no longer using the 'mtx_lock' field, so remove it.
> >
> > While here remove the 'volatile' keyword from amd64's 'struct mutex'.
>
> You mean the mtx_owner member.
>
> > ok?
>
> I think that is wrong. Unless we always access that member using
> explicit atomic operations, we have to prevent the compiler from
> thinking that the variable can't change behind its back. So mtx_owner
> needs to be volatile everywhere.
Fine with me, updated diff below.
Index: arch/alpha/include/mutex.h
===================================================================
RCS file: /cvs/src/sys/arch/alpha/include/mutex.h,v
retrieving revision 1.8
diff -u -p -r1.8 mutex.h
--- arch/alpha/include/mutex.h 20 Apr 2017 13:57:29 -0000 1.8
+++ arch/alpha/include/mutex.h 2 Jan 2018 16:44:58 -0000
@@ -31,7 +31,7 @@
#include <sys/_lock.h>
struct mutex {
- void *mtx_owner;
+ volatile void *mtx_owner;
int mtx_wantipl;
int mtx_oldipl;
#ifdef WITNESS
Index: arch/hppa/include/mutex.h
===================================================================
RCS file: /cvs/src/sys/arch/hppa/include/mutex.h,v
retrieving revision 1.7
diff -u -p -r1.7 mutex.h
--- arch/hppa/include/mutex.h 20 Apr 2017 13:57:29 -0000 1.7
+++ arch/hppa/include/mutex.h 2 Jan 2018 16:45:07 -0000
@@ -39,7 +39,7 @@ struct mutex {
#endif
int mtx_wantipl;
int mtx_oldipl;
- void *mtx_owner;
+ volatile void *mtx_owner;
#ifdef WITNESS
struct lock_object mtx_lock_obj;
#endif
Index: arch/i386/i386/genassym.cf
===================================================================
RCS file: /cvs/src/sys/arch/i386/i386/genassym.cf,v
retrieving revision 1.39
diff -u -p -r1.39 genassym.cf
--- arch/i386/i386/genassym.cf 15 Mar 2016 03:17:51 -0000 1.39
+++ arch/i386/i386/genassym.cf 2 Jan 2018 14:56:57 -0000
@@ -136,7 +136,6 @@ member ih_next
endif
struct mutex
-member mtx_lock
member mtx_wantipl
member mtx_oldipl
member mtx_owner
Index: arch/i386/include/mutex.h
===================================================================
RCS file: /cvs/src/sys/arch/i386/include/mutex.h,v
retrieving revision 1.9
diff -u -p -r1.9 mutex.h
--- arch/i386/include/mutex.h 20 Apr 2017 13:57:29 -0000 1.9
+++ arch/i386/include/mutex.h 2 Jan 2018 16:45:13 -0000
@@ -29,15 +29,10 @@
#include <sys/_lock.h>
-/*
- * XXX - we don't really need the mtx_lock field, we can use mtx_oldipl
- * as the lock to save some space.
- */
struct mutex {
- volatile int mtx_lock;
int mtx_wantipl;
int mtx_oldipl;
- void *mtx_owner;
+ volatile void *mtx_owner;
#ifdef WITNESS
struct lock_object mtx_lock_obj;
#endif
@@ -59,10 +54,10 @@ struct mutex {
#ifdef WITNESS
#define MUTEX_INITIALIZER_FLAGS(ipl, name, flags) \
- { 0, __MUTEX_IPL(ipl), 0, NULL, MTX_LO_INITIALIZER(name, flags) }
+ { __MUTEX_IPL((ipl)), 0, NULL, MTX_LO_INITIALIZER(name, flags) }
#else
#define MUTEX_INITIALIZER_FLAGS(ipl, name, flags) \
- { 0, __MUTEX_IPL(ipl), 0, NULL }
+ { __MUTEX_IPL((ipl)), 0, NULL }
#endif
void __mtx_init(struct mutex *, int);
Index: arch/m88k/include/mutex.h
===================================================================
RCS file: /cvs/src/sys/arch/m88k/include/mutex.h,v
retrieving revision 1.5
diff -u -p -r1.5 mutex.h
--- arch/m88k/include/mutex.h 20 Apr 2017 13:57:29 -0000 1.5
+++ arch/m88k/include/mutex.h 2 Jan 2018 16:45:17 -0000
@@ -33,7 +33,7 @@ struct mutex {
volatile int mtx_lock; /* mutex.S relies upon this field being first */
int mtx_wantipl;
int mtx_oldipl;
- void *mtx_owner;
+ volatile void *mtx_owner;
#ifdef WITNESS
struct lock_object mtx_lock_obj;
#endif
Index: arch/mips64/include/mutex.h
===================================================================
RCS file: /cvs/src/sys/arch/mips64/include/mutex.h,v
retrieving revision 1.2
diff -u -p -r1.2 mutex.h
--- arch/mips64/include/mutex.h 20 Apr 2017 13:57:30 -0000 1.2
+++ arch/mips64/include/mutex.h 2 Jan 2018 16:45:23 -0000
@@ -31,7 +31,7 @@
#include <sys/_lock.h>
struct mutex {
- void *mtx_owner;
+ volatile void *mtx_owner;
int mtx_wantipl;
int mtx_oldipl;
#ifdef WITNESS
