On 11/23/18 at 02:09pm, Kairui Song wrote:
Hi, thanks for the review!
I can use something like:
RUN+="/bin/sh -c "systemctl is-active kdump.service &>/dev/null
&&
/usr/bin/systemd-run --no-block /usr/lib/udev/kdump-udev-throttler""
In the udev rule to conditionally run the script. But I think this one look more
hacky, use a script is cleaner. I didn't find another doable way by just update
udev rules.
I feel above is better and easy to get when one read it first time.
But embed the logic in scripts one need time to understand the logic :)
But I have no strong opinion about this..
Pingfan and Bhupesh are cced, see what do they think
>
> I can add more comment/message to make the usage of kdump-udev-throttler
> cleaner. After this patch, kdump-udev-throttler still do the same
> thing, it just have
> extra ability to run itself in non-blocking mode, so just consider
> calling "kdump-udev-throttler"
> and "kdump-udev-throttler --no-block" have same effect, the only
> difference is the
> later one run in background so never blocks. And both will just exit
> if kdump is not running.
> So udev could just call kdump-udev-throttle, and don't need to bother with
> systemd-run directly, also avoided starting a transient systemd-run service
> if kdump is not running.
> On Fri, Nov 23, 2018 at 1:29 PM Dave Young <dyoung(a)redhat.com> wrote:
> >
> > Hi Kairui,
> > On 11/22/18 at 11:05pm, Kairui Song wrote:
> > > In commit 1c97aee and commit 227c185 udev rules was rewritten to use
> > > systemd-run to run in a non-blocking mode. The problem is that it's a
> > > bit noise, especially on machine bootup, systemd will always generate
> > > extra logs for service start, you might see your journal full of lines
> > > like these if you have many CPUs (each CPU generates a udev event on
> > > boot):
> > >
> > > ...
> > > Nov 22 22:23:05 localhost systemd[1]: Started
/usr/lib/udev/kdump-udev-throttler.
> > > Nov 22 22:23:05 localhost systemd[1]: Started
/usr/lib/udev/kdump-udev-throttler.
> > > Nov 22 22:23:05 localhost systemd[1]: Started
/usr/lib/udev/kdump-udev-throttler.
> > > Nov 22 22:23:05 localhost systemd[1]: Started
/usr/lib/udev/kdump-udev-throttler.
> > > ...
> > >
> > > While system is still booting up, kdump service is not started yet, so
> > > systemd-run calls will end up doing nothing, the throttler being called
> > > by systemd-run will just exit if kdump is not loaded.
> > >
> > > This patch avoid systemd-run from being called at first place if kdump
> > > service is not running, so there won't be unnecessary logs.
> > >
> > > As udev rules don't support shell expressions, this patch reuse
> > > the kdump-udev-throttler script, adding a --no-block option.
> > > kdump-udev-throttler will call it self with systemd-run to run in non
> > > blocking mode if --no-block is given, and exit before doing anything if
> > > kdump is not started. Udev just call "kdump-udev-throttler
--no-block"
> > > directly.
> > >
> > > Signed-off-by: Kairui Song <kasong(a)redhat.com>
> > > ---
> > > 98-kexec.rules | 2 +-
> > > kdump-udev-throttler | 13 ++++++++++---
> > > 2 files changed, 11 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/98-kexec.rules b/98-kexec.rules
> > > index 2f88c77..eca909e 100644
> > > --- a/98-kexec.rules
> > > +++ b/98-kexec.rules
> > > @@ -7,6 +7,6 @@ GOTO="kdump_reload_end"
> > >
> > > LABEL="kdump_reload"
> > >
> > > -RUN+="/usr/bin/systemd-run --no-block
/usr/lib/udev/kdump-udev-throttler"
> > > +RUN+="/usr/lib/udev/kdump-udev-throttler --no-block"
> >
> > This fix looks a hack, if it doable to conditionally run
"/usr/lib/udev/kdump-udev-throttler --no-block"
> > in udev rule?
> >
> > >
> > > LABEL="kdump_reload_end"
> > > diff --git a/kdump-udev-throttler b/kdump-udev-throttler
> > > index 6cbb99a..6899379 100755
> > > --- a/kdump-udev-throttler
> > > +++ b/kdump-udev-throttler
> > > @@ -13,13 +13,20 @@
> > > # In this way, we can make sure kdump service is restarted immediately
> > > # and for exactly once after udev events are settled.
> > >
> > > -
> > > throttle_lock="/var/lock/kdump-udev-throttle"
> > > -interval=2
> > >
> > > -# Don't reload kdump service if kdump service is not started by
systemd
> > > +# Don't do anything if kdump service is not started by systemd
> > > systemctl is-active kdump.service &>/dev/null || exit 0
> > >
> > > +if [[ $1 == '--no-block' ]]; then
> > > + systemd-run --no-block --quiet $0
> > > + if [[ $? -ne 0 ]]; then
> > > + echo "kdump-udev-throttler: Failed to call
systemd-run"
> > > + exit 1
> > > + fi
> > > + exit 0
> > > +fi
> > > +
> > > exec 9>$throttle_lock
> > > if [ $? -ne 0 ]; then
> > > echo "Failed to create the lock file! Fallback to
non-throttled kdump service restart"
> > > --
> > > 2.19.1
> > >
> >
> > Thanks
> > Dave
>
>
>
> --
> Best Regards,
> Kairui Song