Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 2 Apr 2011 06:25:13 +0000 (UTC)
From:      Dmitry Chagin <dchagin@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org
Subject:   svn commit: r220263 - in stable/7/sys: kern sys
Message-ID:  <201104020625.p326PDlf072782@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: dchagin
Date: Sat Apr  2 06:25:13 2011
New Revision: 220263
URL: http://svn.freebsd.org/changeset/base/220263

Log:
  MFC r219041:
  
  ktrace_resize_pool() locking slightly reworked:
  1) do not take a lock around the single atomic operation.
  2) do not lose the invariant of lock by dropping/acquiring
     ktrace_mtx around free() or malloc().
  
  MFC r219042:
  
  Introduce preliminary support of the show description of the ABI of
  traced process by adding two new events which records value of process
  sv_flags to the trace file at process creation/execing/exiting time.
  
  MFC r219311:
  
  Partially rework r219042.
  The reason for this is a bug at ktrops() where process dereferenced
  without having a lock. This might cause a panic if ktrace was runned
  with -p flag and the specified process exited between the dropping
  a lock and writing sv_flags.
  
  Since it is impossible to acquire sx lock while holding mtx switch
  to use asynchronous enqueuerequest() instead of writerequest().
  
  Rename ktr_getrequest_ne() to more understandable name.
  
  MFC r219312:
  
  Fix indentation in comment, double ';' in variable declaration.
  
  PR:		ports/155083

Modified:
  stable/7/sys/kern/kern_exec.c
  stable/7/sys/kern/kern_fork.c
  stable/7/sys/kern/kern_ktrace.c
  stable/7/sys/sys/ktrace.h
Directory Properties:
  stable/7/sys/   (props changed)
  stable/7/sys/cddl/contrib/opensolaris/   (props changed)
  stable/7/sys/contrib/dev/acpica/   (props changed)
  stable/7/sys/contrib/pf/   (props changed)

Modified: stable/7/sys/kern/kern_exec.c
==============================================================================
--- stable/7/sys/kern/kern_exec.c	Sat Apr  2 06:13:31 2011	(r220262)
+++ stable/7/sys/kern/kern_exec.c	Sat Apr  2 06:25:13 2011	(r220263)
@@ -845,6 +845,12 @@ done2:
 		exit1(td, W_EXITCODE(0, SIGABRT));
 		/* NOT REACHED */
 	}
+
+#ifdef KTRACE
+	if (error == 0)
+		ktrprocctor(p);
+#endif
+
 	return (error);
 }
 

Modified: stable/7/sys/kern/kern_fork.c
==============================================================================
--- stable/7/sys/kern/kern_fork.c	Sat Apr  2 06:13:31 2011	(r220262)
+++ stable/7/sys/kern/kern_fork.c	Sat Apr  2 06:25:13 2011	(r220263)
@@ -613,10 +613,6 @@ again:
 
 	callout_init(&p2->p_itcallout, CALLOUT_MPSAFE);
 
-#ifdef KTRACE
-	ktrprocfork(p1, p2);
-#endif
-
 	/*
 	 * If PF_FORK is set, the child process inherits the
 	 * procfs ioctl flags from its parent.
@@ -661,6 +657,10 @@ again:
 	p2->p_acflag = AFORK;
 	PROC_UNLOCK(p2);
 
+#ifdef KTRACE
+	ktrprocfork(p1, p2);
+#endif
+
 	/*
 	 * Finish creating the child process.  It will return via a different
 	 * execution path later.  (ie: directly into user mode)

Modified: stable/7/sys/kern/kern_ktrace.c
==============================================================================
--- stable/7/sys/kern/kern_ktrace.c	Sat Apr  2 06:13:31 2011	(r220262)
+++ stable/7/sys/kern/kern_ktrace.c	Sat Apr  2 06:25:13 2011	(r220263)
@@ -56,6 +56,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/ktrace.h>
 #include <sys/sx.h>
 #include <sys/sysctl.h>
+#include <sys/sysent.h>
 #include <sys/syslog.h>
 #include <sys/sysproto.h>
 
@@ -92,6 +93,7 @@ struct ktr_request {
 	struct	ktr_header ktr_header;
 	void	*ktr_buffer;
 	union {
+		struct	ktr_proc_ctor ktr_proc_ctor;
 		struct	ktr_syscall ktr_syscall;
 		struct	ktr_sysret ktr_sysret;
 		struct	ktr_genio ktr_genio;
@@ -112,6 +114,8 @@ static int data_lengths[] = {
 	0,					/* KTR_USER */
 	0,					/* KTR_STRUCT */
 	0,					/* KTR_SYSCTL */
+	sizeof(struct ktr_proc_ctor),		/* KTR_PROCCTOR */
+	0,					/* KTR_PROCDTOR */
 };
 
 static STAILQ_HEAD(, ktr_request) ktr_free;
@@ -132,7 +136,8 @@ static struct sx ktrace_sx;
 
 static void ktrace_init(void *dummy);
 static int sysctl_kern_ktrace_request_pool(SYSCTL_HANDLER_ARGS);
-static u_int ktrace_resize_pool(u_int newsize);
+static u_int ktrace_resize_pool(u_int oldsize, u_int newsize);
+static struct ktr_request *ktr_getrequest_entered(struct thread *td, int type);
 static struct ktr_request *ktr_getrequest(int type);
 static void ktr_submitrequest(struct thread *td, struct ktr_request *req);
 static void ktr_freeproc(struct proc *p, struct ucred **uc,
@@ -143,6 +148,7 @@ static void ktr_writerequest(struct thre
 static int ktrcanset(struct thread *,struct proc *);
 static int ktrsetchildren(struct thread *,struct proc *,int,int,struct vnode *);
 static int ktrops(struct thread *,struct proc *,int,int,struct vnode *);
+static void ktrprocctor_entered(struct thread *, struct proc *);
 
 /*
  * ktrace itself generates events, such as context switches, which we do not
@@ -198,9 +204,7 @@ sysctl_kern_ktrace_request_pool(SYSCTL_H
 
 	/* Handle easy read-only case first to avoid warnings from GCC. */
 	if (!req->newptr) {
-		mtx_lock(&ktrace_mtx);
 		oldsize = ktr_requestpool;
-		mtx_unlock(&ktrace_mtx);
 		return (SYSCTL_OUT(req, &oldsize, sizeof(u_int)));
 	}
 
@@ -209,10 +213,8 @@ sysctl_kern_ktrace_request_pool(SYSCTL_H
 		return (error);
 	td = curthread;
 	ktrace_enter(td);
-	mtx_lock(&ktrace_mtx);
 	oldsize = ktr_requestpool;
-	newsize = ktrace_resize_pool(wantsize);
-	mtx_unlock(&ktrace_mtx);
+	newsize = ktrace_resize_pool(oldsize, wantsize);
 	ktrace_exit(td);
 	error = SYSCTL_OUT(req, &oldsize, sizeof(u_int));
 	if (error)
@@ -225,38 +227,40 @@ SYSCTL_PROC(_kern_ktrace, OID_AUTO, requ
     &ktr_requestpool, 0, sysctl_kern_ktrace_request_pool, "IU", "");
 
 static u_int
-ktrace_resize_pool(u_int newsize)
+ktrace_resize_pool(u_int oldsize, u_int newsize)
 {
+	STAILQ_HEAD(, ktr_request) ktr_new;
 	struct ktr_request *req;
 	int bound;
 
-	mtx_assert(&ktrace_mtx, MA_OWNED);
 	print_message = 1;
-	bound = newsize - ktr_requestpool;
+	bound = newsize - oldsize;
 	if (bound == 0)
 		return (ktr_requestpool);
-	if (bound < 0)
+	if (bound < 0) {
+		mtx_lock(&ktrace_mtx);
 		/* Shrink pool down to newsize if possible. */
 		while (bound++ < 0) {
 			req = STAILQ_FIRST(&ktr_free);
 			if (req == NULL)
-				return (ktr_requestpool);
+				break;
 			STAILQ_REMOVE_HEAD(&ktr_free, ktr_list);
 			ktr_requestpool--;
-			mtx_unlock(&ktrace_mtx);
 			free(req, M_KTRACE);
-			mtx_lock(&ktrace_mtx);
 		}
-	else
+	} else {
 		/* Grow pool up to newsize. */
+		STAILQ_INIT(&ktr_new);
 		while (bound-- > 0) {
-			mtx_unlock(&ktrace_mtx);
 			req = malloc(sizeof(struct ktr_request), M_KTRACE,
 			    M_WAITOK);
-			mtx_lock(&ktrace_mtx);
-			STAILQ_INSERT_HEAD(&ktr_free, req, ktr_list);
-			ktr_requestpool++;
+			STAILQ_INSERT_HEAD(&ktr_new, req, ktr_list);
 		}
+		mtx_lock(&ktrace_mtx);
+		STAILQ_CONCAT(&ktr_free, &ktr_new);
+		ktr_requestpool += (newsize - oldsize);
+	}
+	mtx_unlock(&ktrace_mtx);
 	return (ktr_requestpool);
 }
 
@@ -265,18 +269,15 @@ CTASSERT(sizeof(((struct ktr_header *)NU
     (sizeof((struct proc *)NULL)->p_comm));
 
 static struct ktr_request *
-ktr_getrequest(int type)
+ktr_getrequest_entered(struct thread *td, int type)
 {
 	struct ktr_request *req;
-	struct thread *td = curthread;
 	struct proc *p = td->td_proc;
 	int pm;
 
-	ktrace_enter(td);	/* XXX: In caller instead? */
 	mtx_lock(&ktrace_mtx);
 	if (!KTRCHECK(td, type)) {
 		mtx_unlock(&ktrace_mtx);
-		ktrace_exit(td);
 		return (NULL);
 	}
 	req = STAILQ_FIRST(&ktr_free);
@@ -302,11 +303,24 @@ ktr_getrequest(int type)
 		mtx_unlock(&ktrace_mtx);
 		if (pm)
 			printf("Out of ktrace request objects.\n");
-		ktrace_exit(td);
 	}
 	return (req);
 }
 
+static struct ktr_request *
+ktr_getrequest(int type)
+{
+	struct thread *td = curthread;
+	struct ktr_request *req;
+
+	ktrace_enter(td);
+	req = ktr_getrequest_entered(td, type);
+	if (req == NULL)
+		ktrace_exit(td);
+
+	return (req);
+}
+
 /*
  * Some trace generation environments don't permit direct access to VFS,
  * such as during a context switch where sleeping is not allowed.  Under these
@@ -320,7 +334,6 @@ ktr_enqueuerequest(struct thread *td, st
 	mtx_lock(&ktrace_mtx);
 	STAILQ_INSERT_TAIL(&td->td_proc->p_ktr, req, ktr_list);
 	mtx_unlock(&ktrace_mtx);
-	ktrace_exit(td);
 }
 
 /*
@@ -370,7 +383,6 @@ ktr_submitrequest(struct thread *td, str
 	ktr_writerequest(td, req);
 	ktr_freerequest(req);
 	sx_xunlock(&ktrace_sx);
-
 	ktrace_exit(td);
 }
 
@@ -488,6 +500,7 @@ ktrprocexec(struct proc *p, struct ucred
 void
 ktrprocexit(struct thread *td)
 {
+	struct ktr_request *req;
 	struct proc *p;
 	struct ucred *cred;
 	struct vnode *vp;
@@ -498,6 +511,9 @@ ktrprocexit(struct thread *td)
 		return;
 
 	ktrace_enter(td);
+	req = ktr_getrequest_entered(td, KTR_PROCDTOR);
+	if (req != NULL)
+		ktr_enqueuerequest(td, req);
 	sx_xlock(&ktrace_sx);
 	ktr_drain(td);
 	sx_xunlock(&ktrace_sx);
@@ -516,6 +532,36 @@ ktrprocexit(struct thread *td)
 	ktrace_exit(td);
 }
 
+static void
+ktrprocctor_entered(struct thread *td, struct proc *p)
+{
+	struct ktr_proc_ctor *ktp;
+	struct ktr_request *req;
+	struct thread *td2;
+
+	ktrace_assert(td);
+	td2 = FIRST_THREAD_IN_PROC(p);
+	req = ktr_getrequest_entered(td2, KTR_PROCCTOR);
+	if (req == NULL)
+		return;
+	ktp = &req->ktr_data.ktr_proc_ctor;
+	ktp->sv_flags = p->p_sysent->sv_flags;
+	ktr_enqueuerequest(td2, req);
+}
+
+void
+ktrprocctor(struct proc *p)
+{
+	struct thread *td = curthread;
+
+	if ((p->p_traceflag & KTRFAC_MASK) == 0)
+		return;
+
+	ktrace_enter(td);
+	ktrprocctor_entered(td, p);
+	ktrace_exit(td);
+}
+
 /*
  * When a process forks, enable tracing in the new process if needed.
  */
@@ -523,8 +569,7 @@ void
 ktrprocfork(struct proc *p1, struct proc *p2)
 {
 
-	PROC_LOCK_ASSERT(p1, MA_OWNED);
-	PROC_LOCK_ASSERT(p2, MA_OWNED);
+	PROC_LOCK(p1);
 	mtx_lock(&ktrace_mtx);
 	KASSERT(p2->p_tracevp == NULL, ("new process has a ktrace vnode"));
 	if (p1->p_traceflag & KTRFAC_INHERIT) {
@@ -537,6 +582,9 @@ ktrprocfork(struct proc *p1, struct proc
 		}
 	}
 	mtx_unlock(&ktrace_mtx);
+	PROC_UNLOCK(p1);
+
+	ktrprocctor(p2);
 }
 
 /*
@@ -660,6 +708,7 @@ ktrpsig(sig, action, mask, code)
 	sigset_t *mask;
 	int code;
 {
+	struct thread *td = curthread;
 	struct ktr_request *req;
 	struct ktr_psig	*kp;
 
@@ -671,13 +720,15 @@ ktrpsig(sig, action, mask, code)
 	kp->action = action;
 	kp->mask = *mask;
 	kp->code = code;
-	ktr_enqueuerequest(curthread, req);
+	ktr_enqueuerequest(td, req);
+	ktrace_exit(td);
 }
 
 void
 ktrcsw(out, user)
 	int out, user;
 {
+	struct thread *td = curthread;
 	struct ktr_request *req;
 	struct ktr_csw *kc;
 
@@ -687,7 +738,8 @@ ktrcsw(out, user)
 	kc = &req->ktr_data.ktr_csw;
 	kc->out = out;
 	kc->user = user;
-	ktr_enqueuerequest(curthread, req);
+	ktr_enqueuerequest(td, req);
+	ktrace_exit(td);
 }
 
 void
@@ -963,6 +1015,8 @@ ktrops(td, p, ops, facs, vp)
 			ktr_freeproc(p, &tracecred, &tracevp);
 	}
 	mtx_unlock(&ktrace_mtx);
+	if ((p->p_traceflag & KTRFAC_MASK) != 0)
+		ktrprocctor_entered(td, p);
 	PROC_UNLOCK(p);
 	if (tracevp != NULL) {
 		int vfslocked;

Modified: stable/7/sys/sys/ktrace.h
==============================================================================
--- stable/7/sys/sys/ktrace.h	Sat Apr  2 06:13:31 2011	(r220262)
+++ stable/7/sys/sys/ktrace.h	Sat Apr  2 06:25:13 2011	(r220263)
@@ -156,6 +156,7 @@ struct ktr_csw {
 #define KTR_STRUCT	8
 struct sockaddr;
 struct stat;
+struct sysentvec;
 
 /*
  * KTR_SYSCTL - name of a sysctl MIB
@@ -164,6 +165,19 @@ struct stat;
 	/* record contains null-terminated MIB name */
 
 /*
+ * KTR_PROCCTOR - trace process creation (multiple ABI support)
+ */
+#define KTR_PROCCTOR	10
+struct ktr_proc_ctor {
+	u_int	sv_flags;	/* struct sysentvec sv_flags copy */
+};
+
+/*
+ * KTR_PROCDTOR - trace process destruction (multiple ABI support)
+ */
+#define KTR_PROCDTOR	11
+
+/*
  * KTR_DROP - If this bit is set in ktr_type, then at least one event
  * between the previous record and this record was dropped.
  */
@@ -182,6 +196,8 @@ struct stat;
 #define KTRFAC_USER	(1<<KTR_USER)
 #define KTRFAC_STRUCT	(1<<KTR_STRUCT)
 #define KTRFAC_SYSCTL	(1<<KTR_SYSCTL)
+#define KTRFAC_PROCCTOR	(1<<KTR_PROCCTOR)
+#define KTRFAC_PROCDTOR	(1<<KTR_PROCDTOR)
 
 /*
  * trace flags (also in p_traceflags)
@@ -198,6 +214,7 @@ void	ktrgenio(int, enum uio_rw, struct u
 void	ktrsyscall(int, int narg, register_t args[]);
 void	ktrsysctl(int *name, u_int namelen);
 void	ktrsysret(int, int, register_t);
+void	ktrprocctor(struct proc *);
 void	ktrprocexec(struct proc *, struct ucred **, struct vnode **);
 void	ktrprocexit(struct thread *);
 void	ktrprocfork(struct proc *, struct proc *);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201104020625.p326PDlf072782>