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@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"
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"
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@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 $0if [[ $? -ne 0 ]]; thenecho "kdump-udev-throttler: Failed to call systemd-run"exit 1fiexit 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
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 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@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@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 $0if [[ $? -ne 0 ]]; thenecho "kdump-udev-throttler: Failed to call systemd-run"exit 1fiexit 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
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@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@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 $0if [[ $? -ne 0 ]]; thenecho "kdump-udev-throttler: Failed to call systemd-run"exit 1fiexit 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
On 12/04/2018 04:36 PM, Dave Young wrote:
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 am fine with both. But I prefer the "RUN+="/usr/lib/udev/kdump-udev-throttler --no-block" Since it avoids to duplicate the code.
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@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@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 $0if [[ $? -ne 0 ]]; thenecho "kdump-udev-throttler: Failed to call systemd-run"exit 1fiexit 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
On Tue, Dec 4, 2018 at 2:06 PM Dave Young dyoung@redhat.com wrote:
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
Yeah, I think the invocation via conditional udev rule looks better. May be if we can add some comments on top of the rule, it will help the reader gather the intent better.
Thanks, Bhupesh
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@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@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 $0if [[ $? -ne 0 ]]; thenecho "kdump-udev-throttler: Failed to call systemd-run"exit 1fiexit 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
-
Bhupesh Sharma -
Dave Young -
Kairui Song -
piliu