Skip site navigation (1)Skip section navigation (2)
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>