Evgeni Golov, 18. Februar 2022 12:49:
> On Fri, Feb 18, 2022 at 12:34:49PM +0100, Evgeni Golov wrote:
> > Hi Adriaan,
> >
> > I was looking at this for the next tuned upload, and have a few
> > questions.
> >
> > On Thu, May 20, 2021 at 01:27:01PM +0000, Schmidt, Adriaan wrote:
> > > Paths related to grub (required by the bootloader plugin):
> > >
> > > diff --git a/tuned/consts.py b/tuned/consts.py
> > > index 733ad51..f0acf9e 100644
> > > --- a/tuned/consts.py
> > > +++ b/tuned/consts.py
> > > @@ -24,7 +24,7 @@ ERROR_THRESHOLD = 3
> > >
> > > # bootloader plugin configuration
> > > BOOT_DIR = "/boot"
> > > -GRUB2_CFG_FILES = ["/etc/grub2.cfg", "/etc/grub2-efi.cfg"]
> > > +GRUB2_CFG_FILES = ["/boot/grub/grub.cfg"]
> >
> > This is the *generated* file, right?
> > So when the user regenerates it (e.g. by installing a new kernel) all
> > changes are wiped, until tuned detects that?
> >
> > Sounds like a source for possible confusion. How do you handle this in
> > your environment?
> >
> > (And you are aware of https://github.com/redhat-performance/tuned/pull/387,
> > it seems)
>
> I just realized, on EL-systems, those /etc paths are symlinks to the
> generated file in /boot anyways. So at least the expirience is identical
> here.
Sorry, it took me a while to look into this. I did not run any systematic tests,
but looking at the source, there is code to modify both the generated grub.cfg
and /etc/default/grub, so I'm assuming kernel updates and externally triggered
calls to update-grub should be fine.
And I just noticed that I have one more local change:
diff --git a/tuned/plugins/plugin_bootloader.py
b/tuned/plugins/plugin_bootloader.py
index c3e25a6..7d5c2e6 100644
--- a/tuned/plugins/plugin_bootloader.py
+++ b/tuned/plugins/plugin_bootloader.py
@@ -348,7 +348,7 @@ class BootloaderPlugin(base.Plugin):
def _update_grubenv(self, d):
log.debug("updating grubenv, setting %s" % str(d))
l = ["%s=%s" % (str(option), str(value)) for option, value in
d.items()]
- (rc, out) = self._cmd.execute(["grub2-editenv", "-", "set"] + l)
+ (rc, out) = self._cmd.execute(["grub-editenv", "-", "set"] + l)
if rc != 0:
log.warn("cannot update grubenv: '%s'" % out)
return False
I will propose something upstream to detect the executable, but for current
releases we'd need this patch.
> > > Python bindings for perf (required by the scheduler and irqbalance
> plugins):
> > > This is a little more tricky, because it needs to be fixed elsewhere...
> currently these plugins simply fail when trying to "import perf". The
> required module is part of the kernel sources, and is currently not packaged.
> > > Two things are required:
> > > * The package linux-perf needs to ship the binding itself (perf.so)
> > > * A wrapper is needed to select the correct version based on the
> running kernel, same as for the "perf" executable, where this wrapper is
> located in package linux-base
> > > For Linux 4.19, we posted a patch (https://bugs.debian.org/cgi-
> bin/bugreport.cgi?bug=860957#10), for 5.10 we have something similar which
> we'd be happy to contribute.
> >
> > I recall posting this bug, yeah.
> > Sadly, I don't see much that can be done here from the tuned side, until
> > the kernel packages aren't adjusted :(
Actually, there is some movement:
https://salsa.debian.org/kernel-team/linux/-/merge_requests/425
https://salsa.debian.org/kernel-team/linux/-/commit/91c110c0cfaa5366b880382f86a84a03d8011ffd
This removes the versioning of perf and the need for wrappers, which makes
it easier to package the Python bindings. The bindings themselves are still
not included, but we're working on it...
> > > Systemd unit file (tuned.service):
> > > Currently passes -P to have tuned write its own PID file, and -l to have
> tuned write its own log file.
> > > Wouldn't it be better practice, to
> > > * remove -P, as systemd will take care of the PID file
> >
> > I'd rather argue this is just default behaviour?
> > https://github.com/redhat-
> performance/tuned/commit/9520364fcae362e7181cd1057591054e3407c756
> > https://github.com/redhat-
> performance/tuned/blob/dc8808cb394e52e0d11c7d7b3a53264421d21d47/tuned.py#L77-
> L79
> >
> > > * remove -l, and have systemd direct tuned's stdout to the journal
> >
> > Can you propose those changes upstream? They do seem to make sense, but
> > I'd prefer not to diverge from upstream unless really necessary.
You are right. I must have read things wrong and assumed the service file was
part of the Debian packaging. Let's keep it the way it is.
Adriaan
> > Thanks!
> > Evgeni
