Date: Sun, 16 Sep 2001 15:51:14 -0700 (PDT) From: Matt Dillon <dillon@earth.backplane.com> To: freebsd-current@freebsd.org Subject: syscall wrapper for Giant - patch for review Message-ID: <200109162251.f8GMpEd35341@earth.backplane.com> References: <XFMail.010830162911.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This patch implements syscall wrappers for Giant around various
subsystems. It is intended to allow more developers to work on -current,
in the main tree, by allowing the various Giant pushdown works in
progress to be comitted to the main tree.
This patch implements sysctl variables which can conditionally turn on and
off Giant for various subsystems. We still have to instrument the
subsystems, of course! I've included some example instrumentation
for kern/kern_prot.c. The patch is incomplete, obviously, but can be
implemented and committed incrementally.
I feel that there are two reasons why we are not able to build momentum
for -current or get more developers on board: First, too much work is
being done in P4 instead of the main tree. Second, the work is being
done in a kitchen-sink development mode, so when things *do* wind up
going into the main tree, the changes are so huge that the wider
development audience has no ability to track down bugs. Of all the work
being done only Julian's first-stage KSE work has no choice but to be
a kitchen sink affair. ALL of the other work can and should be done
incrementally.
This patch allows developers doing pushdown work (John, Alfred, Peter,
etc...) to do the work in the main tree without adversely effecting
other developers. By having the system boot-up with Giant turned on
around the subsystems under development other developers are mostly
(not completely, but mostly) shielded from the pushdown work while those
doing the pushdown work can test it by disabling Giant for the particular
subsystem they are working on, and ask other developers to test it when
they feel it is ready for a wider audience simply by asking people to
turn off the sysctl wrapper(s) in question. These sysctls will allow
developers to run their systems at varying levels of safety, something I
expect will be needed for several years before we find every last little
bug.
It has been rather difficult for me to convince some people of the
necessity of a more incremental approach, so I fully expect a number
of people to complain. Well, too bad. Life sucks. -current isn't going
to make it if we don't do this. I am accepting all critisism for this
approach but I fully intend to commit something in the next week or so
and to start instrumenting the code to allow the more incremental
approach to be taken.
I've also received luke-warm responses from some people in regards to my
desire to remove some of the massive inlines and macros that are bloating
-current today. I don't *care* whether the bloat is supposedly only
happening during development due to the debugging. We are going to be
'in development' for the next year at least, even longer, and some of us
run production systems with a moderate degree of debugging turned on
anyway and will do so in the future with -current. The macros and inlines
need to be moved into real procedures and I fully intend to start doing
that after I get the incremental sysctl stuff going.
-Matt
Index: sys/mutex.h
===================================================================
RCS file: /home/ncvs/src/sys/sys/mutex.h,v
retrieving revision 1.37
diff -u -r1.37 mutex.h
--- sys/mutex.h 2001/09/12 08:38:05 1.37
+++ sys/mutex.h 2001/09/16 22:24:30
@@ -115,6 +115,8 @@
#ifdef INVARIANT_SUPPORT
void _mtx_assert(struct mtx *m, int what, const char *file, int line);
#endif
+int mtx_lock_giant(int sysctlvar);
+void mtx_unlock_giant(int s);
/*
* We define our machine-independent (unoptimized) mutex micro-operations
@@ -312,6 +314,12 @@
*/
extern struct mtx sched_lock;
extern struct mtx Giant;
+
+/*
+ * Giant lock sysctl variables used by other modules
+ */
+extern int kern_giant_proc;
+extern int kern_giant_file;
/*
* Giant lock manipulation and clean exit macros.
Index: kern/kern_mutex.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_mutex.c,v
retrieving revision 1.68
diff -u -r1.68 kern_mutex.c
--- kern/kern_mutex.c 2001/09/12 08:37:44 1.68
+++ kern/kern_mutex.c 2001/09/16 22:23:21
@@ -697,3 +697,43 @@
WITNESS_DESTROY(&m->mtx_object);
}
+
+/*
+ * Encapsulated Giant mutex routines. These routines provide encapsulation
+ * control for the Giant mutex, allowing sysctls to be used to turn on and
+ * off Giant around certain subsystems. The default value for the sysctls
+ * are set to what developers believe is stable and working in regards to
+ * the Giant pushdown. callers of mtx_lock_giant() are expected to pass the
+ * return value to an accompanying mtx_unlock_giant() later on. If multiple
+ * subsystems are effected by a Giant wrap, all related sysctl variables
+ * must be zero for the subsystem call to operate without Giant (as determined
+ * by the caller).
+ */
+
+SYSCTL_NODE(_kern, OID_AUTO, giant, CTLFLAG_RD, NULL, "Giant mutex manipulation");
+
+static int kern_giant_all = 1;
+SYSCTL_INT(_kern_giant, OID_AUTO, all, CTLFLAG_RW, &kern_giant_all, 0, "");
+
+int kern_giant_proc = 1; /* Giant around PROC locks */
+int kern_giant_file = 1; /* Giant around struct file & filedesc */
+SYSCTL_INT(_kern_giant, OID_AUTO, proc, CTLFLAG_RW, &kern_giant_proc, 0, "");
+SYSCTL_INT(_kern_giant, OID_AUTO, file, CTLFLAG_RW, &kern_giant_file, 0, "");
+
+int
+mtx_lock_giant(int sysctlvar)
+{
+ if (sysctlvar || kern_giant_all) {
+ mtx_lock(&Giant);
+ return(1);
+ }
+ return(0);
+}
+
+void
+mtx_unlock_giant(int s)
+{
+ if (s)
+ mtx_unlock(&Giant);
+}
+
Index: kern/kern_prot.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_prot.c,v
retrieving revision 1.102
diff -u -r1.102 kern_prot.c
--- kern/kern_prot.c 2001/09/12 08:37:44 1.102
+++ kern/kern_prot.c 2001/09/16 22:28:33
@@ -86,15 +86,16 @@
struct getpid_args *uap;
{
struct proc *p = td->td_proc;
+ int s;
- mtx_lock(&Giant);
+ s = mtx_lock_giant(kern_giant_proc);
td->td_retval[0] = p->p_pid;
#if defined(COMPAT_43) || defined(COMPAT_SUNOS)
PROC_LOCK(p);
td->td_retval[1] = p->p_pptr->p_pid;
PROC_UNLOCK(p);
#endif
- mtx_unlock(&Giant);
+ mtx_unlock_giant(s);
return (0);
}
@@ -117,12 +118,13 @@
struct getppid_args *uap;
{
struct proc *p = td->td_proc;
+ int s;
- mtx_lock(&Giant);
+ s = mtx_lock_giant(kern_giant_proc);
PROC_LOCK(p);
td->td_retval[0] = p->p_pptr->p_pid;
PROC_UNLOCK(p);
- mtx_unlock(&Giant);
+ mtx_unlock_giant(s);
return (0);
}
@@ -170,8 +172,9 @@
struct proc *p = td->td_proc;
struct proc *pt;
int error = 0;
+ int s;
- mtx_lock(&Giant);
+ s = mtx_lock_giant(kern_giant_proc);
if (uap->pid == 0)
td->td_retval[0] = p->p_pgrp->pg_id;
else {
@@ -187,7 +190,7 @@
PROC_UNLOCK(pt);
}
done2:
- mtx_unlock(&Giant);
+ mtx_unlock_giant(s);
return (error);
}
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200109162251.f8GMpEd35341>