Add pmcstat flag for specifying kernel module paths

[stevek]

Summary

pmcstat assumes that the kernel modules can all be found in the same
place as the kernel itself. This is not necessarily true in all
configurations.

Since the kern.module_path sysctl provides the list of paths that the
kernel uses to find kernel modules, it would be better to use that
list (in addition to the existing assumption) to determine the paths
to use when searching for kernel modules.

This set of changes the uses kern.module_path list as a default for
the paths to search. Also, the -K flag has been added to allow the
user to override the default search paths.

Test Plan

Tested at Juniper on systems with kernel modules in per-package
directories where kernel modules are not found with the pre-existing
default assumptions.

[stevek]

Fix man page argument name for -K flag to be consistent.

[stevek]

usr.sbin/pmcstat/pmcstat.c

1139–1149

Thinking about it, I wonder if this loop should ensure that each path component obtained is a fully qualified path and not relative.

[jhb]

I think this isn't really the right way to fix this. The kernel linker knows the full path to the module and that is what kgdb uses to figure out where modules live (and they might not be in the module path at all if you have done 'make load' from a module build directory, or they could be in a "later" path in the module path if the user uses an absolute path with kldlad). pmcstat should be using the full path (though that might require fixing the pmcstat messages if pmcstat is relying on those rather than this.

It looks like pmc_kld_load() just needs to use 'lf->pathname' instead of 'lf->filename'. You could then change the path logic to do something like this:

if (image->pi_iskernelmodule && strchr(path, '/') == NULL)
    (void) snprintf(...);
else
    (void) snprintf(...);

That would then honor FSROOT, but otherwise use the pathname of the actual-loaded image (so once pmc.ko is fixed kernel modules would use full paths and could use the 'user' case of just adding the FSROOT prefix, and the other hack would only remain for legacy kernels).

[wblock]

usr.sbin/pmcstat/pmcstat.8

160

"argument" is not needed, can maybe shorten this a bit:

Set the list of directories to search for kernel modules.

161

With the change above, this line and the following sentence are not needed.

167

s/colon separated/colon-separated/

dir-list is referred to as an argument repeatedly, which is not necessary. I suggest removing the initial Argument and use:

.Ar dir-list
is a colon-separated list of paths to search for modules.

168

The default module path is obtained from the

usr.sbin/pmcstat/pmcstat.c

500

"\t -K dir-list\t set the search path for kernel modules\n"

1139

The man page says colon-separated, but it would probably be worth telling the user that semicolons can be used also (and that is what kern.module_path uses, for some reason I can't fathom).

[jtl]

Yay for this change!

And, yes, @jhb probably has a more robust solution, for reasons he states (e.g. modules that a user kldloads from a custom place).

In D8234#171288, @jhb wrote:

I think this isn't really the right way to fix this. The kernel linker knows the full path to the module and that is what kgdb uses to figure out where modules live (and they might not be in the module path at all if you have done 'make load' from a module build directory, or they could be in a "later" path in the module path if the user uses an absolute path with kldlad). pmcstat should be using the full path (though that might require fixing the pmcstat messages if pmcstat is relying on those rather than this.

It might (or might not) be relevant to Juniper that chroots might mess up reliance on the "full path" from the kernel's perspective.

[stevek]

In D8234#171524, @jtl wrote:

Yay for this change!

And, yes, @jhb probably has a more robust solution, for reasons he states (e.g. modules that a user kldloads from a custom place).

In D8234#171288, @jhb wrote:

I think this isn't really the right way to fix this. The kernel linker knows the full path to the module and that is what kgdb uses to figure out where modules live (and they might not be in the module path at all if you have done 'make load' from a module build directory, or they could be in a "later" path in the module path if the user uses an absolute path with kldlad). pmcstat should be using the full path (though that might require fixing the pmcstat messages if pmcstat is relying on those rather than this.

It might (or might not) be relevant to Juniper that chroots might mess up reliance on the "full path" from the kernel's perspective.

It might be relevant, since we use the init_chroot option in loader.conf and pre-load some kernel modules from loader.

I'll experiment with it some before sending an updated diff to this review.

Note that we probably need to have a wider discussion about what the kernel records for paths when one is using init_chroot. As it stands, anything that is recorded before the chroot usually is no longer accurate once the chroot occurs.