Date: Fri, 20 Apr 2001 07:35:24 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Mark Murray <mark@grondar.za> Cc: smp@FreeBSD.ORG Subject: Re: Please review - header cleanups Message-ID: <Pine.BSF.4.21.0104200718080.12888-100000@besplex.bde.org> In-Reply-To: <200104190634.f3J6Xvw99980@gratis.grondar.za>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 19 Apr 2001, Mark Murray wrote: > Said Bruce Evans <bde@zeta.org.au>: > > Possible implementations: > > 1) Do the same things as are planned for `struct timespec': use a tiny > > header that declares just `struct mtx' and include this header as > > necessary. > > Ok so this would be a header with only the "struct mtx { ... };" in it, > and it would be included in all the headers that need to know the size > and shape of struct mtx (including sys/mutex.h)? --- diff -c2 mutex_types.h~ mutex_types.h *** mutex_types.h~ Fri Apr 20 06:09:31 2001 --- mutex_types.h Thu Apr 19 21:13:40 2001 *************** *** 0 **** --- 1,27 ---- + #ifndef _SYS_MUTEX_TYPES_H_ + #define _SYS_MUTEX_TYPES_H_ + + struct lock_object { + struct lock_class *lo_class; + const char *lo_name; + const char *lo_file; /* File and line of last acquire. */ + int lo_line; + u_int lo_flags; + STAILQ_ENTRY(lock_object) lo_list; /* List of all locks in system. */ + struct witness *lo_witness; + }; + + /* + * Sleep/spin mutex + */ + + struct mtx { + struct lock_object mtx_object; /* Common lock properties. */ + volatile uintptr_t mtx_lock; /* owner (and state for sleep locks) */ + volatile u_int mtx_recurse; /* number of recursive holds */ + critical_t mtx_savecrit; /* saved flags (for spin locks) */ + TAILQ_HEAD(, proc) mtx_blocked; /* threads blocked on this lock */ + LIST_ENTRY(mtx) mtx_contested; /* list of all contested locks */ + }; + + #endif /* !_SYS_MUTEX_TYPES_H_ */ Index: lock.h =================================================================== RCS file: /home/ncvs/src/sys/sys/lock.h,v retrieving revision 1.29 diff -c -2 -r1.29 lock.h *** lock.h 2001/04/06 21:37:52 1.29 --- lock.h 2001/04/19 11:02:46 *************** *** 40,43 **** --- 40,44 ---- #include <sys/queue.h> + #include <sys/mutex_types.h> /* *************** *** 60,75 **** #define LC_SLEEPABLE 0x00000004 /* Sleeping allowed with this lock. */ #define LC_RECURSABLE 0x00000008 /* Locks of this type may recurse. */ - - struct witness; - - struct lock_object { - struct lock_class *lo_class; - const char *lo_name; - const char *lo_file; /* File and line of last acquire. */ - int lo_line; - u_int lo_flags; - STAILQ_ENTRY(lock_object) lo_list; /* List of all locks in system. */ - struct witness *lo_witness; - }; #define LO_CLASSFLAGS 0x0000ffff /* Class specific flags. */ --- 61,64 ---- Index: mbuf.h =================================================================== RCS file: /home/ncvs/src/sys/sys/mbuf.h,v retrieving revision 1.76 diff -c -2 -r1.76 mbuf.h *** mbuf.h 2001/04/05 03:55:27 1.76 --- mbuf.h 2001/04/19 11:03:22 *************** *** 39,46 **** #ifdef _KERNEL ! #include <sys/condvar.h> /* XXX */ ! #include <sys/lock.h> /* XXX */ ! #include <sys/mutex.h> /* XXX */ ! #endif /* _KERNEL */ /* --- 39,45 ---- #ifdef _KERNEL ! #include <sys/condvar.h> /* XXX */ ! #include <sys/mutex_types.h> ! #endif /* Index: mutex.h =================================================================== RCS file: /home/ncvs/src/sys/sys/mutex.h,v retrieving revision 1.29 diff -c -2 -r1.29 mutex.h *** mutex.h 2001/03/28 09:03:24 1.29 --- mutex.h 2001/04/19 11:02:38 *************** *** 35,38 **** --- 35,39 ---- #ifndef LOCORE #include <sys/queue.h> + #include <sys/mutex_types.h> #ifdef _KERNEL *************** *** 79,97 **** /* - * Sleep/spin mutex - */ - - struct lock_object; - - struct mtx { - struct lock_object mtx_object; /* Common lock properties. */ - volatile uintptr_t mtx_lock; /* owner (and state for sleep locks) */ - volatile u_int mtx_recurse; /* number of recursive holds */ - critical_t mtx_savecrit; /* saved flags (for spin locks) */ - TAILQ_HEAD(, proc) mtx_blocked; /* threads blocked on this lock */ - LIST_ENTRY(mtx) mtx_contested; /* list of all contested locks */ - }; - - /* * XXX: Friendly reminder to fix things in MP code that is presently being * XXX: worked on. --- 80,83 ---- Index: resourcevar.h =================================================================== RCS file: /home/ncvs/src/sys/sys/resourcevar.h,v retrieving revision 1.21 diff -c -2 -r1.21 resourcevar.h *** resourcevar.h 2001/03/28 09:17:56 1.21 --- resourcevar.h 2001/04/19 11:04:07 *************** *** 40,45 **** #include <sys/resource.h> #include <sys/queue.h> ! #include <sys/lock.h> /* XXX */ ! #include <sys/mutex.h> /* XXX */ /* --- 40,46 ---- #include <sys/resource.h> #include <sys/queue.h> ! #ifdef _KERNEL ! #include <sys/mutex_types.h> ! #endif /* Index: sx.h =================================================================== RCS file: /home/ncvs/src/sys/sys/sx.h,v retrieving revision 1.6 diff -c -2 -r1.6 sx.h *** sx.h 2001/03/28 09:03:24 1.6 --- sx.h 2001/04/19 11:04:44 *************** *** 32,40 **** #ifndef LOCORE ! #include <sys/lock.h> /* XXX */ ! #include <sys/mutex.h> /* XXX */ #include <sys/condvar.h> /* XXX */ - - struct lock_object; struct sx { --- 32,37 ---- #ifndef LOCORE ! #include <sys/mutex_types.h> #include <sys/condvar.h> /* XXX */ struct sx { Index: ucred.h =================================================================== RCS file: /home/ncvs/src/sys/sys/ucred.h,v retrieving revision 1.22 diff -c -2 -r1.22 ucred.h *** ucred.h 2001/03/28 09:17:56 1.22 --- ucred.h 2001/04/19 11:22:22 *************** *** 38,43 **** #define _SYS_UCRED_H_ ! #include <sys/lock.h> /* XXX */ ! #include <sys/mutex.h> /* XXX */ /* --- 38,43 ---- #define _SYS_UCRED_H_ ! #include <sys/queue.h> ! #include <sys/mutex_types.h> /* Index: user.h =================================================================== RCS file: /home/ncvs/src/sys/sys/user.h,v retrieving revision 1.36 diff -c -2 -r1.36 user.h *** user.h 2001/03/28 09:17:56 1.36 --- user.h 2001/04/19 11:05:13 *************** *** 46,51 **** #include <sys/ucred.h> #include <sys/uio.h> ! #include <sys/lock.h> /* XXX */ ! #include <sys/mutex.h> /* XXX */ #include <sys/proc.h> #include <vm/vm.h> /* XXX */ --- 46,50 ---- #include <sys/ucred.h> #include <sys/uio.h> ! #include <sys/mutex_types.h> #include <sys/proc.h> #include <vm/vm.h> /* XXX */ Index: vnode.h =================================================================== RCS file: /home/ncvs/src/sys/sys/vnode.h,v retrieving revision 1.143 diff -c -2 -r1.143 vnode.h *** vnode.h 2001/04/17 04:33:33 1.143 --- vnode.h 2001/04/19 11:16:37 *************** *** 38,43 **** #define _SYS_VNODE_H_ - #include <sys/lock.h> - #include <sys/mutex.h> #include <sys/queue.h> #include <sys/selinfo.h> --- 38,43 ---- #define _SYS_VNODE_H_ #include <sys/queue.h> + #include <sys/lock.h> /* XXX */ + #include <sys/mutex_types.h> #include <sys/selinfo.h> --- This removes all the nested includes of <sys/mutex.h> except the one in <sys/buf.h> (the inline functions there should probably not be inline). It only removes a couple of nested includes of <sys/lock.h> (ones related to <sys/mutex.h>). LINT compiles after adding includes of <sys/mutex.h> (and sometimes <sys/lock.h>) to "only" about 50 .c files. Many more probably depend on the pollution in <sys/buf.h> :-(. About 1/2 of the 50 really should include <sys/mutex.h>. The others mostly use the PROC_* locking macros in <sys/proc.h> these expand to mtx_lock(), etc. mtx_lock() is another macro so it can't just be declared. It needs the MPASS() macros from <sys/lock.h>, so everything that uses the PROC_* macros needs both <sys/mutex.h> and <sys/lock.h>. > > 2) Do the same things as are done for size_t: define a macro that declares > > `struct mtx' in a not so tiny secondary header; include this header and > > expand it as necessary. This is uglier than (1), but doesn't require > > so many headers. > > I think I prefer 1). I've put 2 structs in <sys/mutex_types.h> to test things quickly. Strictly, struct lock_object belongs in a header by itself. > > 3) Combination/variation of on (1)-(2): conditionally declare various > > structs and types in a not so tiny secondary header; include this > > header with only the required declarations selected. This method is > > used in glibc. This is not as ugly as (2), but I think it is slower > > than both (1) and (2). > > ONE header to declare _all_/lots_of the "internal" structures? Hmmm... See glibc or stddef.h under contrib/gcc. The __need_foo_t stuff selects the types that will be declared. Ugly, isn't it. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-smp" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0104200718080.12888-100000>