* [PATCH] mdadm/systemd: change KillMode from none to mixed in service files @ 2021-12-01 6:22 Coly Li 2021-12-01 16:08 ` mtkaczyk 0 siblings, 1 reply; 10+ messages in thread From: Coly Li @ 2021-12-01 6:22 UTC (permalink / raw) To: linux-raid; +Cc: Coly Li, Benjamin Brunner, Neil Brown, Jes Sorensen For mdadm's systemd configuration, current systemd KillMode is "none" in following service files, - mdadm-grow-continue@.service - mdmon@.service This "none" mode is strongly recommended againsted by systemd developers (see man 5 system.kill for "KillMode=" section), and is considering to remove in future systemd version. A proper KillMode is "mixed", which is explained in the system.kill man page as, "If set to mixed, the SIGTERM signal (see below) is sent to the main process while the subsequent SIGKILL signal (see below) is sent to all remaining processes of the unit's control group." This patch changes KillMode in above listed service files from "none" to "mixed", to follow systemd recommendation and avoid potential unnecessary issue. Suggested-by: Benjamin Brunner <bbrunner@suse.com> Cc: Neil Brown <neilb@suse.de> Cc: Jes Sorensen <jsorensen@fb.com> --- systemd/mdadm-grow-continue@.service | 2 +- systemd/mdmon@.service | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/systemd/mdadm-grow-continue@.service b/systemd/mdadm-grow-continue@.service index 5c667d2..5fe9d99 100644 --- a/systemd/mdadm-grow-continue@.service +++ b/systemd/mdadm-grow-continue@.service @@ -14,4 +14,4 @@ ExecStart=BINDIR/mdadm --grow --continue /dev/%I StandardInput=null StandardOutput=null StandardError=null -KillMode=none +KillMode=mixed diff --git a/systemd/mdmon@.service b/systemd/mdmon@.service index 85a3a7c..5ef1bf5 100644 --- a/systemd/mdmon@.service +++ b/systemd/mdmon@.service @@ -25,4 +25,4 @@ Type=forking # it out) and systemd will remove it when transitioning from # initramfs to rootfs. #PIDFile=/run/mdadm/%I.pid -KillMode=none +KillMode=mixed -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] mdadm/systemd: change KillMode from none to mixed in service files 2021-12-01 6:22 [PATCH] mdadm/systemd: change KillMode from none to mixed in service files Coly Li @ 2021-12-01 16:08 ` mtkaczyk 2021-12-01 16:23 ` Coly Li 0 siblings, 1 reply; 10+ messages in thread From: mtkaczyk @ 2021-12-01 16:08 UTC (permalink / raw) To: Coly Li; +Cc: linux-raid, Benjamin Brunner, Neil Brown, Jes Sorensen Hi Coly, > This patch changes KillMode in above listed service files from "none" > to "mixed", to follow systemd recommendation and avoid potential > unnecessary issue. What about mdmonitor.service? Should we add it there too? Now it is not defined. > Cc: Jes Sorensen <jsorensen@fb.com> Not sure if it a problem but Jes uses: jes@trained-monkey.org for communication with linux-raid. Thanks, Mariusz ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mdadm/systemd: change KillMode from none to mixed in service files 2021-12-01 16:08 ` mtkaczyk @ 2021-12-01 16:23 ` Coly Li 2021-12-01 16:28 ` Benjamin Brunner 0 siblings, 1 reply; 10+ messages in thread From: Coly Li @ 2021-12-01 16:23 UTC (permalink / raw) To: mtkaczyk; +Cc: linux-raid, Benjamin Brunner, Neil Brown, Jes Sorensen On 12/2/21 12:08 AM, mtkaczyk wrote: > Hi Coly, > >> This patch changes KillMode in above listed service files from "none" >> to "mixed", to follow systemd recommendation and avoid potential >> unnecessary issue. > What about mdmonitor.service? Should we add it there too? Now it is not > defined. It was overlooked when I did grep KillMode. Yes, I agree mdmonitor.service should have a KillMode key word as well. Let me post a v2 version. >> Cc: Jes Sorensen <jsorensen@fb.com> > Not sure if it a problem but Jes uses: jes@trained-monkey.org for > communication with linux-raid. Copied. Thanks for the hint. Coly Li ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mdadm/systemd: change KillMode from none to mixed in service files 2021-12-01 16:23 ` Coly Li @ 2021-12-01 16:28 ` Benjamin Brunner 2021-12-01 16:43 ` Coly Li 0 siblings, 1 reply; 10+ messages in thread From: Benjamin Brunner @ 2021-12-01 16:28 UTC (permalink / raw) To: Coly Li, mtkaczyk; +Cc: linux-raid, Neil Brown, Jes Sorensen Hi all, On 01.12.21 17:23, Coly Li wrote: > On 12/2/21 12:08 AM, mtkaczyk wrote: >> Hi Coly, >> >>> This patch changes KillMode in above listed service files from "none" >>> to "mixed", to follow systemd recommendation and avoid potential >>> unnecessary issue. >> What about mdmonitor.service? Should we add it there too? Now it is not >> defined. > > It was overlooked when I did grep KillMode. Yes, I agree > mdmonitor.service should have a KillMode key word as well. > > Let me post a v2 version. > JFYI, when KillMode is not set, it defaults to KillMode=control-group, see https://www.freedesktop.org/software/systemd/man/systemd.kill.html. Therefore, it shouldn't be necessary to explicitly add it (as long as control-group is working in case of mdmonitor.service, of course). Best Benjamin >>> Cc: Jes Sorensen <jsorensen@fb.com> >> Not sure if it a problem but Jes uses: jes@trained-monkey.org for >> communication with linux-raid. > > Copied. Thanks for the hint. > > Coly Li > -- Benjamin Brunner Engineering Manager System Boot and Init SUSE Software Solutions Germany GmbH Maxfeldstr. 5 90409 Nürnberg Germany HRB 36809, AG Nürnberg Geschäftsführer: Ivo Totev ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mdadm/systemd: change KillMode from none to mixed in service files 2021-12-01 16:28 ` Benjamin Brunner @ 2021-12-01 16:43 ` Coly Li 2021-12-01 21:51 ` NeilBrown 0 siblings, 1 reply; 10+ messages in thread From: Coly Li @ 2021-12-01 16:43 UTC (permalink / raw) To: Benjamin Brunner; +Cc: linux-raid, Neil Brown, mtkaczyk, Jes Sorensen On 12/2/21 12:28 AM, Benjamin Brunner wrote: > Hi all, > > On 01.12.21 17:23, Coly Li wrote: >> On 12/2/21 12:08 AM, mtkaczyk wrote: >>> Hi Coly, >>> >>>> This patch changes KillMode in above listed service files from "none" >>>> to "mixed", to follow systemd recommendation and avoid potential >>>> unnecessary issue. >>> What about mdmonitor.service? Should we add it there too? Now it is not >>> defined. >> >> It was overlooked when I did grep KillMode. Yes, I agree >> mdmonitor.service should have a KillMode key word as well. >> >> Let me post a v2 version. >> > JFYI, when KillMode is not set, it defaults to KillMode=control-group, > see https://www.freedesktop.org/software/systemd/man/systemd.kill.html. > > Therefore, it shouldn't be necessary to explicitly add it (as long as > control-group is working in case of mdmonitor.service, of course). Hi Benjamin, Please correct me if I am wrong, I see the difference of the KillMode is, -- KillMode=mixed stops the processes more gentally, it kill the main process with SIGTERM and the remaining processes with SIGKILL. -- KillMode=control-group kills all in-cgroup processes with SIGKILL, which I feel a bit cruel for the main process. IMHO both method (explicit mixed mode or implicit control-group mode) should works for the fixing issue, but I feel removing the KillMode lines might be better as you indicated since the files can be a bit simpler. Do I understand you correct ? Thanks. Coly Li ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mdadm/systemd: change KillMode from none to mixed in service files 2021-12-01 16:43 ` Coly Li @ 2021-12-01 21:51 ` NeilBrown 2021-12-05 13:42 ` Coly Li 0 siblings, 1 reply; 10+ messages in thread From: NeilBrown @ 2021-12-01 21:51 UTC (permalink / raw) To: Coly Li; +Cc: Benjamin Brunner, linux-raid, mtkaczyk, Jes Sorensen On Thu, 02 Dec 2021, Coly Li wrote: > On 12/2/21 12:28 AM, Benjamin Brunner wrote: > > Hi all, > > > > On 01.12.21 17:23, Coly Li wrote: > >> On 12/2/21 12:08 AM, mtkaczyk wrote: > >>> Hi Coly, > >>> > >>>> This patch changes KillMode in above listed service files from "none" > >>>> to "mixed", to follow systemd recommendation and avoid potential > >>>> unnecessary issue. > >>> What about mdmonitor.service? Should we add it there too? Now it is not > >>> defined. > >> > >> It was overlooked when I did grep KillMode. Yes, I agree > >> mdmonitor.service should have a KillMode key word as well. > >> > >> Let me post a v2 version. > >> > > JFYI, when KillMode is not set, it defaults to KillMode=control-group, > > see https://www.freedesktop.org/software/systemd/man/systemd.kill.html. > > > > Therefore, it shouldn't be necessary to explicitly add it (as long as > > control-group is working in case of mdmonitor.service, of course). > > Hi Benjamin, > > Please correct me if I am wrong, I see the difference of the KillMode is, > -- KillMode=mixed stops the processes more gentally, it kill the main > process with SIGTERM and the remaining processes with SIGKILL. > -- KillMode=control-group kills all in-cgroup processes with SIGKILL, > which I feel a bit cruel for the main process. There is no point sending SIGTERM to a process which doesn't respond to it. mdmon is the only mdadm service which handles SIGTERM. So it might make sense to uise KillMode=mixed for that. For anything else, SIGKILL via KillMode=control-group is perfectly acceptable. NeilBrown > > IMHO both method (explicit mixed mode or implicit control-group mode) > should works for the fixing issue, but I feel removing the KillMode > lines might be better as you indicated since the files can be a bit > simpler. Do I understand you correct ? > > Thanks. > > Coly Li > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mdadm/systemd: change KillMode from none to mixed in service files 2021-12-01 21:51 ` NeilBrown @ 2021-12-05 13:42 ` Coly Li 2021-12-07 12:34 ` Benjamin Brunner 0 siblings, 1 reply; 10+ messages in thread From: Coly Li @ 2021-12-05 13:42 UTC (permalink / raw) To: NeilBrown, Benjamin Brunner; +Cc: linux-raid, mtkaczyk, Jes Sorensen On 12/2/21 5:51 AM, NeilBrown wrote: > On Thu, 02 Dec 2021, Coly Li wrote: >> On 12/2/21 12:28 AM, Benjamin Brunner wrote: >>> Hi all, >>> >>> On 01.12.21 17:23, Coly Li wrote: >>>> On 12/2/21 12:08 AM, mtkaczyk wrote: >>>>> Hi Coly, >>>>> >>>>>> This patch changes KillMode in above listed service files from "none" >>>>>> to "mixed", to follow systemd recommendation and avoid potential >>>>>> unnecessary issue. >>>>> What about mdmonitor.service? Should we add it there too? Now it is not >>>>> defined. >>>> It was overlooked when I did grep KillMode. Yes, I agree >>>> mdmonitor.service should have a KillMode key word as well. >>>> >>>> Let me post a v2 version. >>>> >>> JFYI, when KillMode is not set, it defaults to KillMode=control-group, >>> see https://www.freedesktop.org/software/systemd/man/systemd.kill.html. >>> >>> Therefore, it shouldn't be necessary to explicitly add it (as long as >>> control-group is working in case of mdmonitor.service, of course). >> Hi Benjamin, >> >> Please correct me if I am wrong, I see the difference of the KillMode is, >> -- KillMode=mixed stops the processes more gentally, it kill the main >> process with SIGTERM and the remaining processes with SIGKILL. >> -- KillMode=control-group kills all in-cgroup processes with SIGKILL, >> which I feel a bit cruel for the main process. > There is no point sending SIGTERM to a process which doesn't respond to > it. mdmon is the only mdadm service which handles SIGTERM. So it might > make sense to uise KillMode=mixed for that. > For anything else, SIGKILL via KillMode=control-group is perfectly > acceptable. Hi Neil, Thanks for the hint. So it seems remove the KillMode= lines from the systemd files might be best choice. Hi Benjamin, How do you think about removing the KillMode= lines to use SIGKILL as default action ? Thanks. Coly Li ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mdadm/systemd: change KillMode from none to mixed in service files 2021-12-05 13:42 ` Coly Li @ 2021-12-07 12:34 ` Benjamin Brunner 2021-12-08 16:25 ` Franck Bui 0 siblings, 1 reply; 10+ messages in thread From: Benjamin Brunner @ 2021-12-07 12:34 UTC (permalink / raw) To: Coly Li, NeilBrown, Franck Bui; +Cc: linux-raid, mtkaczyk, Jes Sorensen Hi, On 05.12.21 14:42, Coly Li wrote: > On 12/2/21 5:51 AM, NeilBrown wrote: >> On Thu, 02 Dec 2021, Coly Li wrote: >>> On 12/2/21 12:28 AM, Benjamin Brunner wrote: >>>> Hi all, >>>> >>>> On 01.12.21 17:23, Coly Li wrote: >>>>> On 12/2/21 12:08 AM, mtkaczyk wrote: >>>>>> Hi Coly, >>>>>> >>>>>>> This patch changes KillMode in above listed service files from >>>>>>> "none" >>>>>>> to "mixed", to follow systemd recommendation and avoid potential >>>>>>> unnecessary issue. >>>>>> What about mdmonitor.service? Should we add it there too? Now it >>>>>> is not >>>>>> defined. >>>>> It was overlooked when I did grep KillMode. Yes, I agree >>>>> mdmonitor.service should have a KillMode key word as well. >>>>> >>>>> Let me post a v2 version. >>>>> >>>> JFYI, when KillMode is not set, it defaults to KillMode=control-group, >>>> see https://www.freedesktop.org/software/systemd/man/systemd.kill.html. >>>> >>>> Therefore, it shouldn't be necessary to explicitly add it (as long as >>>> control-group is working in case of mdmonitor.service, of course). >>> Hi Benjamin, >>> >>> Please correct me if I am wrong, I see the difference of the KillMode >>> is, >>> -- KillMode=mixed stops the processes more gentally, it kill the main >>> process with SIGTERM and the remaining processes with SIGKILL. >>> -- KillMode=control-group kills all in-cgroup processes with SIGKILL, >>> which I feel a bit cruel for the main process. >> There is no point sending SIGTERM to a process which doesn't respond to >> it. mdmon is the only mdadm service which handles SIGTERM. So it might >> make sense to uise KillMode=mixed for that. >> For anything else, SIGKILL via KillMode=control-group is perfectly >> acceptable. > > Hi Neil, > > Thanks for the hint. So it seems remove the KillMode= lines from the > systemd files might be best choice. > > Hi Benjamin, > > How do you think about removing the KillMode= lines to use SIGKILL as > default action ? > From my PoV, this should be fine. To be on the safe side, I've added Franck Bui, our systemd maintainer, to the list of recipients. Franck, could you have a look as well, please? Thanks, Benjamin > Thanks. > > Coly Li > -- Benjamin Brunner Engineering Manager System Boot and Init SUSE Software Solutions Germany GmbH Maxfeldstr. 5 90409 Nürnberg Germany HRB 36809, AG Nürnberg Geschäftsführer: Ivo Totev ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mdadm/systemd: change KillMode from none to mixed in service files 2021-12-07 12:34 ` Benjamin Brunner @ 2021-12-08 16:25 ` Franck Bui 2021-12-08 21:27 ` NeilBrown 0 siblings, 1 reply; 10+ messages in thread From: Franck Bui @ 2021-12-08 16:25 UTC (permalink / raw) To: Benjamin Brunner, Coly Li, NeilBrown; +Cc: linux-raid, mtkaczyk, Jes Sorensen Hi, On 12/7/21 1:34 PM, Benjamin Brunner wrote: >>>> Please correct me if I am wrong, I see the difference of the KillMode is, >>>> -- KillMode=mixed stops the processes more gentally, it kill the main >>>> process with SIGTERM and the remaining processes with SIGKILL. >>>> -- KillMode=control-group kills all in-cgroup processes with SIGKILL, >>>> which I feel a bit cruel for the main process. I think there's a miss-understanding here. Regardless of whether "mixed" or "control-group" mode (or any other mode actually) is used, the process used to kill processes part of a service is always the same, only the list of the killed processes differs. The process is as follow: 1. send the signal specified by KillSignal= to the list of processes (if any), TERM is the default 2. wait until either the target of process(es) exit or a timeout expires 3. if the timeout expires send the signal specified by FinalKillSignal=, KILL is the default For "control-group", all remaining processes will receive the SIGTERM signal (by default) and if there are still processes after a period f time, they will get the SIGKILL signal. For "mixed", only the main process will receive the SIGTERM signal, and if there are still processes after a period of time, all remaining processes (including the main one) will receive the SIGKILL signal. >>> There is no point sending SIGTERM to a process which doesn't respond to >>> it. mdmon is the only mdadm service which handles SIGTERM. So it might >>> make sense to uise KillMode=mixed for that. >>> For anything else, SIGKILL via KillMode=control-group is perfectly >>> acceptable. I don't know enough mdadm to suggest a mode but maybe the clarification above will help you figuring this out. That said it sounds a bit strange that some processes don't respond to SIGTERM. Is that done because some services need to run lately during the shutdown process ? Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mdadm/systemd: change KillMode from none to mixed in service files 2021-12-08 16:25 ` Franck Bui @ 2021-12-08 21:27 ` NeilBrown 0 siblings, 0 replies; 10+ messages in thread From: NeilBrown @ 2021-12-08 21:27 UTC (permalink / raw) To: Franck Bui; +Cc: Benjamin Brunner, Coly Li, linux-raid, mtkaczyk, Jes Sorensen On Thu, 09 Dec 2021, Franck Bui wrote: > Hi, > > On 12/7/21 1:34 PM, Benjamin Brunner wrote: > >>>> Please correct me if I am wrong, I see the difference of the KillMode is, > >>>> -- KillMode=mixed stops the processes more gentally, it kill the main > >>>> process with SIGTERM and the remaining processes with SIGKILL. > >>>> -- KillMode=control-group kills all in-cgroup processes with SIGKILL, > >>>> which I feel a bit cruel for the main process. > > I think there's a miss-understanding here. > > Regardless of whether "mixed" or "control-group" mode (or any other mode > actually) is used, the process used to kill processes part of a service is > always the same, only the list of the killed processes differs. > > The process is as follow: > > 1. send the signal specified by KillSignal= to the list of processes (if > any), TERM is the default > 2. wait until either the target of process(es) exit or a timeout expires > 3. if the timeout expires send the signal specified by FinalKillSignal=, > KILL is the default > > For "control-group", all remaining processes will receive the SIGTERM signal (by > default) and if there are still processes after a period f time, they will get > the SIGKILL signal. > > For "mixed", only the main process will receive the SIGTERM signal, and if there > are still processes after a period of time, all remaining processes (including > the main one) will receive the SIGKILL signal. > > >>> There is no point sending SIGTERM to a process which doesn't respond to > >>> it. mdmon is the only mdadm service which handles SIGTERM. So it might > >>> make sense to uise KillMode=mixed for that. > >>> For anything else, SIGKILL via KillMode=control-group is perfectly > >>> acceptable. > > I don't know enough mdadm to suggest a mode but maybe the clarification above > will help you figuring this out. > > That said it sounds a bit strange that some processes don't respond to SIGTERM. > Is that done because some services need to run lately during the shutdown process ? When I wrote that they don't respond to SIGTERM, I meant that they don't take any special action. They don't ignore SIGTERM, so they will exit when they are sent it. i.e. it doesn't matter whether they get SIGTERM or SIGKILL - either way they will exit. NeilBrown ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-12-08 21:27 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-01 6:22 [PATCH] mdadm/systemd: change KillMode from none to mixed in service files Coly Li 2021-12-01 16:08 ` mtkaczyk 2021-12-01 16:23 ` Coly Li 2021-12-01 16:28 ` Benjamin Brunner 2021-12-01 16:43 ` Coly Li 2021-12-01 21:51 ` NeilBrown 2021-12-05 13:42 ` Coly Li 2021-12-07 12:34 ` Benjamin Brunner 2021-12-08 16:25 ` Franck Bui 2021-12-08 21:27 ` NeilBrown
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.