Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 30 May 2023 23:37:47 GMT
From:      Jessica Clarke <jrtc27@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 53d0b9e438bc - main - pmc: Provide full path to modules from kernel linker
Message-ID:  <202305302337.34UNblBk056859@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by jrtc27:

URL: https://cgit.FreeBSD.org/src/commit/?id=53d0b9e438bc30ef12c24fddec13e97ade852164

commit 53d0b9e438bc30ef12c24fddec13e97ade852164
Author:     Jessica Clarke <jrtc27@FreeBSD.org>
AuthorDate: 2023-05-30 23:15:34 +0000
Commit:     Jessica Clarke <jrtc27@FreeBSD.org>
CommitDate: 2023-05-30 23:15:34 +0000

    pmc: Provide full path to modules from kernel linker
    
    This unifies the user object and kernel module paths in libpmcstat,
    allows modules loaded from non-standard locations (e.g. from a user's
    home directory when testing) to be found and, since buffer is what all
    the warnings here use (they were never updated when buffer_modules were
    added to pick based on where the file was found) has the side-effect of
    ensuring the messages are correct.
    
    This includes obsoleting the now-superfluous -k option in pmcstat.
    
    This change breaks the hwpmc ABI and will be followed by a bump to the
    pmc major version.
    
    Reviewed by:    jhb, jkoshy, mhorne
    Differential Revision:  https://reviews.freebsd.org/D40048
---
 lib/libpmcstat/libpmcstat.h       |  3 +-
 lib/libpmcstat/libpmcstat_image.c | 24 ++++------------
 sys/dev/hwpmc/hwpmc_mod.c         |  2 +-
 sys/kern/kern_linker.c            |  2 +-
 usr.sbin/pmcstat/pmcstat.8        | 12 --------
 usr.sbin/pmcstat/pmcstat.c        | 58 ++-------------------------------------
 6 files changed, 11 insertions(+), 90 deletions(-)

diff --git a/lib/libpmcstat/libpmcstat.h b/lib/libpmcstat/libpmcstat.h
index 8e8d94b116d8..07d82d4d0e57 100644
--- a/lib/libpmcstat/libpmcstat.h
+++ b/lib/libpmcstat/libpmcstat.h
@@ -98,7 +98,7 @@ struct pmcstat_args {
 #define	FLAG_READ_LOGFILE		0x00000200	/* -R file */
 #define	FLAG_DO_GPROF			0x00000400	/* -g */
 #define	FLAG_HAS_SAMPLESDIR		0x00000800	/* -D dir */
-#define	FLAG_HAS_KERNELPATH		0x00001000	/* -k kernel */
+/* was FLAG_HAS_KERNELPATH		0x00001000 */
 #define	FLAG_DO_PRINT			0x00002000	/* -o */
 #define	FLAG_DO_CALLGRAPHS		0x00004000	/* -G or -F */
 #define	FLAG_DO_ANNOTATE		0x00008000	/* -m */
@@ -121,7 +121,6 @@ struct pmcstat_args {
 	char	*pa_outputpath;		/* path to output log */
 	void	*pa_logparser;		/* log file parser */
 	const char	*pa_fsroot;	/* FS root where executables reside */
-	char	*pa_kernel;		/* pathname of the kernel */
 	const char	*pa_samplesdir;	/* directory for profile files */
 	const char	*pa_mapfilename;/* mapfile name */
 	FILE	*pa_graphfile;		/* where to send the callgraph */
diff --git a/lib/libpmcstat/libpmcstat_image.c b/lib/libpmcstat/libpmcstat_image.c
index c63f00e03253..bd62a4818f2c 100644
--- a/lib/libpmcstat/libpmcstat_image.c
+++ b/lib/libpmcstat/libpmcstat_image.c
@@ -315,7 +315,6 @@ pmcstat_image_get_elf_params(struct pmcstat_image *image,
 	GElf_Shdr sh;
 	enum pmcstat_image_type image_type;
 	char buffer[PATH_MAX];
-	char buffer_modules[PATH_MAX];
 
 	assert(image->pi_type == PMCSTAT_IMAGE_UNKNOWN);
 
@@ -330,32 +329,19 @@ pmcstat_image_get_elf_params(struct pmcstat_image *image,
 	assert(path != NULL);
 
 	/*
-	 * Look for kernel modules under FSROOT/KERNELPATH/NAME and
-	 * FSROOT/boot/modules/NAME, and user mode executable objects
-	 * under FSROOT/PATHNAME.
+	 * Look for files under FSROOT/PATHNAME.
 	 */
-	if (image->pi_iskernelmodule) {
-		(void) snprintf(buffer, sizeof(buffer), "%s%s/%s",
-		    args->pa_fsroot, args->pa_kernel, path);
-		(void) snprintf(buffer_modules, sizeof(buffer_modules),
-		    "%s/boot/modules/%s", args->pa_fsroot, path);
-	} else {
-		(void) snprintf(buffer, sizeof(buffer), "%s%s",
-		    args->pa_fsroot, path);
-	}
+	(void) snprintf(buffer, sizeof(buffer), "%s%s",
+	    args->pa_fsroot, path);
 
 	e = NULL;
 	fd = open(buffer, O_RDONLY, 0);
-	if (fd < 0 && !image->pi_iskernelmodule) {
+	if (fd < 0) {
 		warnx("WARNING: Cannot open \"%s\".",
 		    buffer);
 		goto done;
 	}
-	if (fd < 0 && (fd = open(buffer_modules, O_RDONLY, 0)) < 0) {
-		warnx("WARNING: Cannot open \"%s\" or \"%s\".",
-		    buffer, buffer_modules);
-		goto done;
-	}
+
 	if (elf_version(EV_CURRENT) == EV_NONE) {
 		warnx("WARNING: failed to init elf\n");
 		goto done;
diff --git a/sys/dev/hwpmc/hwpmc_mod.c b/sys/dev/hwpmc/hwpmc_mod.c
index 29518152760f..830e73941fb6 100644
--- a/sys/dev/hwpmc/hwpmc_mod.c
+++ b/sys/dev/hwpmc/hwpmc_mod.c
@@ -5402,7 +5402,7 @@ pmc_kld_load(void *arg __unused, linker_file_t lf)
 	CK_LIST_FOREACH(po, &pmc_ss_owners, po_ssnext)
 		if (po->po_flags & PMC_PO_OWNS_LOGFILE)
 			pmclog_process_map_in(po, (pid_t) -1,
-			    (uintfptr_t) lf->address, lf->filename);
+			    (uintfptr_t) lf->address, lf->pathname);
 	PMC_EPOCH_EXIT();
 
 	/*
diff --git a/sys/kern/kern_linker.c b/sys/kern/kern_linker.c
index f5e3195714e0..9dc7ff42cc38 100644
--- a/sys/kern/kern_linker.c
+++ b/sys/kern/kern_linker.c
@@ -2119,7 +2119,7 @@ linker_hwpmc_list_objects(void)
 	i = 0;
 	TAILQ_FOREACH(lf, &linker_files, link) {
 		/* Save the info for this linker file. */
-		kobase[i].pm_file = lf->filename;
+		kobase[i].pm_file = lf->pathname;
 		kobase[i].pm_address = (uintptr_t)lf->address;
 		i++;
 	}
diff --git a/usr.sbin/pmcstat/pmcstat.8 b/usr.sbin/pmcstat/pmcstat.8
index dade290c21b1..8e0a077f5115 100644
--- a/usr.sbin/pmcstat/pmcstat.8
+++ b/usr.sbin/pmcstat/pmcstat.8
@@ -57,7 +57,6 @@
 .Op Fl f Ar pluginopt
 .Op Fl g
 .Op Fl i Ar lwp
-.Op Fl k Ar kerneldir
 .Op Fl l Ar secs
 .Op Fl m Ar pathname
 .Op Fl n Ar rate
@@ -336,17 +335,6 @@ which you can get from
 .Xr ps 1
 .Fl o
 .Li lwp .
-.It Fl k Ar kerneldir
-Set the pathname of the kernel directory to argument
-.Ar kerneldir .
-This directory specifies where
-.Nm
-should look for the kernel and its modules.
-The default is to use the path of the running kernel obtained from the
-.Va kern.bootfile
-sysctl.
-Modules will also be searched for in /boot/modules if not found in
-.Ar kerneldir .
 .It Fl l Ar secs
 Set system-wide performance measurement duration for
 .Ar secs
diff --git a/usr.sbin/pmcstat/pmcstat.c b/usr.sbin/pmcstat/pmcstat.c
index 02d80b205736..fa590432667a 100644
--- a/usr.sbin/pmcstat/pmcstat.c
+++ b/usr.sbin/pmcstat/pmcstat.c
@@ -383,7 +383,6 @@ pmcstat_show_usage(void)
 	    "\t -f spec\t pass \"spec\" to as plugin option\n"
 	    "\t -g\t\t produce gprof(1) compatible profiles\n"
 	    "\t -i lwp\t\t filter on thread id \"lwp\" in post-processing\n"
-	    "\t -k dir\t\t set the path to the kernel\n"
 	    "\t -l secs\t set duration time\n"
 	    "\t -m file\t print sampled PCs to \"file\"\n"
 	    "\t -n rate\t set sampling rate\n"
@@ -456,7 +455,7 @@ main(int argc, char **argv)
 	int use_cumulative_counts;
 	short cf, cb;
 	uint64_t current_sampling_count;
-	char *end, *tmp, *event;
+	char *end, *event;
 	const char *errmsg, *graphfilename;
 	enum pmcstat_state runstate;
 	struct pmc_driverstats ds_start, ds_end;
@@ -465,7 +464,6 @@ main(int argc, char **argv)
 	struct kevent kev;
 	struct winsize ws;
 	struct stat sb;
-	char buffer[PATH_MAX];
 	uint32_t caps;
 
 	check_driver_stats      = 0;
@@ -510,16 +508,6 @@ main(int argc, char **argv)
 	caps = 0;
 	CPU_ZERO(&cpumask);
 
-
-	/* Default to using the running system kernel. */
-	len = 0;
-	if (sysctlbyname("kern.bootfile", NULL, &len, NULL, 0) == -1)
-		err(EX_OSERR, "ERROR: Cannot determine path of running kernel");
-	args.pa_kernel = malloc(len);
-	if (args.pa_kernel == NULL)
-		errx(EX_SOFTWARE, "ERROR: Out of memory.");
-	if (sysctlbyname("kern.bootfile", args.pa_kernel, &len, NULL, 0) == -1)
-		err(EX_OSERR, "ERROR: Cannot determine path of running kernel");
 	len = sizeof(domains);
 	if (sysctlbyname("vm.ndomains", &domains, &len, NULL, 0) == -1)
 		err(EX_OSERR, "ERROR: Cannot get number of domains");
@@ -623,12 +611,8 @@ main(int argc, char **argv)
 			break;
 
 		case 'k':	/* pathname to the kernel */
-			free(args.pa_kernel);
-			args.pa_kernel = strdup(optarg);
-			if (args.pa_kernel == NULL)
-				errx(EX_SOFTWARE, "ERROR: Out of memory");
-			args.pa_required |= FLAG_DO_ANALYSIS;
-			args.pa_flags    |= FLAG_HAS_KERNELPATH;
+			warnx("WARNING: -k is obsolete, has no effect "
+			    "and will be removed in FreeBSD 15.");
 			break;
 
 		case 'L':
@@ -1029,12 +1013,6 @@ main(int argc, char **argv)
 "ERROR: option -O is used only with options -E, -P, -S and -W."
 		    );
 
-	/* -k kernel path require -g/-G/-m/-T or -R */
-	if ((args.pa_flags & FLAG_HAS_KERNELPATH) &&
-	    (args.pa_flags & FLAG_DO_ANALYSIS) == 0 &&
-	    (args.pa_flags & FLAG_READ_LOGFILE) == 0)
-	    errx(EX_USAGE, "ERROR: option -k is only used with -g/-R/-m/-T.");
-
 	/* -D only applies to gprof output mode (-g) */
 	if ((args.pa_flags & FLAG_HAS_SAMPLESDIR) &&
 	    (args.pa_flags & FLAG_DO_GPROF) == 0)
@@ -1058,36 +1036,6 @@ main(int argc, char **argv)
 "ERROR: option -O is required if counting and sampling PMCs are specified together."
 		    );
 
-	/*
-	 * Check if 'kerneldir' refers to a file rather than a
-	 * directory.  If so, use `dirname path` to determine the
-	 * kernel directory.
-	 */
-	(void) snprintf(buffer, sizeof(buffer), "%s%s", args.pa_fsroot,
-	    args.pa_kernel);
-	if (stat(buffer, &sb) < 0)
-		err(EX_OSERR, "ERROR: Cannot locate kernel \"%s\"",
-		    buffer);
-	if (!S_ISREG(sb.st_mode) && !S_ISDIR(sb.st_mode))
-		errx(EX_USAGE, "ERROR: \"%s\": Unsupported file type.",
-		    buffer);
-	if (!S_ISDIR(sb.st_mode)) {
-		tmp = args.pa_kernel;
-		args.pa_kernel = strdup(dirname(args.pa_kernel));
-		if (args.pa_kernel == NULL)
-			errx(EX_SOFTWARE, "ERROR: Out of memory");
-		free(tmp);
-		(void) snprintf(buffer, sizeof(buffer), "%s%s",
-		    args.pa_fsroot, args.pa_kernel);
-		if (stat(buffer, &sb) < 0)
-			err(EX_OSERR, "ERROR: Cannot stat \"%s\"",
-			    buffer);
-		if (!S_ISDIR(sb.st_mode))
-			errx(EX_USAGE,
-			    "ERROR: \"%s\" is not a directory.",
-			    buffer);
-	}
-
 	/*
 	 * If we have a callgraph be created, select the outputfile.
 	 */



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