svn commit: r196915 - in stable/7/sys: dev/hwpmc kern

[Attilio Rao]

Author: attilio
Date: Mon Sep  7 08:52:15 2009
New Revision: 196915
URL: http://svn.freebsd.org/changeset/base/196915

Log:
  MFC r195159 and r195005:
  
  * Don't assume a fixed number of preloaded KLDs but calculate at runtime.
    This avoind ending up into an endless loop.
  * Fix a LOR between pmc_sx and allproc/proctree locks
  
  Sponsored by:	Sandvine Incorporated

Modified:
  stable/7/sys/dev/hwpmc/hwpmc_logging.c
  stable/7/sys/dev/hwpmc/hwpmc_mod.c
  stable/7/sys/kern/kern_linker.c

Modified: stable/7/sys/dev/hwpmc/hwpmc_logging.c
==============================================================================
--- stable/7/sys/dev/hwpmc/hwpmc_logging.c	Mon Sep  7 08:46:26 2009	(r196914)
+++ stable/7/sys/dev/hwpmc/hwpmc_logging.c	Mon Sep  7 08:52:15 2009	(r196915)
@@ -44,6 +44,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/module.h>
 #include <sys/mutex.h>
 #include <sys/pmc.h>
+#include <sys/pmckern.h>
 #include <sys/pmclog.h>
 #include <sys/proc.h>
 #include <sys/signalvar.h>
@@ -552,6 +553,12 @@ pmclog_configure_log(struct pmc_mdep *md
 	int error;
 	struct proc *p;
 
+	/*
+	 * As long as it is possible to get a LOR between pmc_sx lock and
+	 * proctree/allproc sx locks used for adding a new process, assure
+	 * the former is not held here.
+	 */
+	sx_assert(&pmc_sx, SA_UNLOCKED);
 	PMCDBG(LOG,CFG,1, "config po=%p logfd=%d", po, logfd);
 
 	p = po->po_owner;

Modified: stable/7/sys/dev/hwpmc/hwpmc_mod.c
==============================================================================
--- stable/7/sys/dev/hwpmc/hwpmc_mod.c	Mon Sep  7 08:46:26 2009	(r196914)
+++ stable/7/sys/dev/hwpmc/hwpmc_mod.c	Mon Sep  7 08:52:15 2009	(r196915)
@@ -2666,7 +2666,7 @@ static const char *pmc_op_to_name[] = {
 static int
 pmc_syscall_handler(struct thread *td, void *syscall_args)
 {
-	int error, is_sx_downgraded, op;
+	int error, is_sx_downgraded, is_sx_locked, op;
 	struct pmc_syscall_args *c;
 	void *arg;
 
@@ -2675,6 +2675,7 @@ pmc_syscall_handler(struct thread *td, v
 	DROP_GIANT();
 
 	is_sx_downgraded = 0;
+	is_sx_locked = 1;
 
 	c = (struct pmc_syscall_args *) syscall_args;
 
@@ -2723,9 +2724,11 @@ pmc_syscall_handler(struct thread *td, v
 		 * a log file configured, flush its buffers and
 		 * de-configure it.
 		 */
-		if (cl.pm_logfd >= 0)
+		if (cl.pm_logfd >= 0) {
+			sx_xunlock(&pmc_sx);
+			is_sx_locked = 0;
 			error = pmclog_configure_log(md, po, cl.pm_logfd);
-		else if (po->po_flags & PMC_PO_OWNS_LOGFILE) {
+		} else if (po->po_flags & PMC_PO_OWNS_LOGFILE) {
 			pmclog_process_closelog(po);
 			error = pmclog_flush(po);
 			if (error == 0) {
@@ -3776,10 +3779,12 @@ pmc_syscall_handler(struct thread *td, v
 		break;
 	}
 
-	if (is_sx_downgraded)
-		sx_sunlock(&pmc_sx);
-	else
-		sx_xunlock(&pmc_sx);
+	if (is_sx_locked != 0) {
+		if (is_sx_downgraded)
+			sx_sunlock(&pmc_sx);
+		else
+			sx_xunlock(&pmc_sx);
+	}
 
 	if (error)
 		atomic_add_int(&pmc_stats.pm_syscall_errors, 1);

Modified: stable/7/sys/kern/kern_linker.c
==============================================================================
--- stable/7/sys/kern/kern_linker.c	Mon Sep  7 08:46:26 2009	(r196914)
+++ stable/7/sys/kern/kern_linker.c	Mon Sep  7 08:52:15 2009	(r196915)
@@ -1882,62 +1882,41 @@ linker_basename(const char *path)
 }
 
 #ifdef HWPMC_HOOKS
-
-struct hwpmc_context {
-	int	nobjects;
-	int	nmappings;
-	struct pmckern_map_in *kobase;
-};
-
-static int
-linker_hwpmc_list_object(linker_file_t lf, void *arg)
-{
-	struct hwpmc_context *hc;
-
-	hc = arg;
-
-	/* If we run out of mappings, fail. */
-	if (hc->nobjects >= hc->nmappings)
-		return (1);
-
-	/* Save the info for this linker file. */
-	hc->kobase[hc->nobjects].pm_file = lf->filename;
-	hc->kobase[hc->nobjects].pm_address = (uintptr_t)lf->address;
-	hc->nobjects++;
-	return (0);
-}
-
 /*
  * Inform hwpmc about the set of kernel modules currently loaded.
  */
 void *
 linker_hwpmc_list_objects(void)
 {
-	struct hwpmc_context hc;
+	linker_file_t lf;
+	struct pmckern_map_in *kobase;
+	int i, nmappings;
 
-	hc.nmappings = 15;	/* a reasonable default */
+	nmappings = 0;
+	KLD_LOCK();
+	TAILQ_FOREACH(lf, &linker_files, link)
+		nmappings++;
 
- retry:
-	/* allocate nmappings+1 entries */
-	MALLOC(hc.kobase, struct pmckern_map_in *,
-	    (hc.nmappings + 1) * sizeof(struct pmckern_map_in), M_LINKER,
-	    M_WAITOK | M_ZERO);
-
-	hc.nobjects = 0;
-	if (linker_file_foreach(linker_hwpmc_list_object, &hc) != 0) {
-		hc.nmappings = hc.nobjects;
-		FREE(hc.kobase, M_LINKER);
-		goto retry;
+	/* Allocate nmappings + 1 entries. */
+	kobase = malloc((nmappings + 1) * sizeof(struct pmckern_map_in),
+	    M_LINKER, M_WAITOK | M_ZERO);
+	i = 0;
+	TAILQ_FOREACH(lf, &linker_files, link) {
+
+		/* Save the info for this linker file. */
+		kobase[i].pm_file = lf->filename;
+		kobase[i].pm_address = (uintptr_t)lf->address;
+		i++;
 	}
+	KLD_UNLOCK();
 
-	KASSERT(hc.nobjects > 0, ("linker_hpwmc_list_objects: no kernel "
-		"objects?"));
+	KASSERT(i > 0, ("linker_hwpmc_list_objects: no kernel objects?"));
 
 	/* The last entry of the malloced area comprises of all zeros. */
-	KASSERT(hc.kobase[hc.nobjects].pm_file == NULL,
+	KASSERT(kobase[i].pm_file == NULL,
 	    ("linker_hwpmc_list_objects: last object not NULL"));
 
-	return ((void *)hc.kobase);
+	return ((void *)kobase);
 }
 #endif