All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Marzinski <bmarzins@redhat.com>
To: Martin Wilck <mwilck@suse.com>
Cc: Antonio Alvarez Feijoo <antonio.feijoo@suse.com>,
	dm-devel@lists.linux.dev
Subject: Re: [PATCH] multipath.rules.in: fix race in unit cancel-multipath-wait-$kernel
Date: Fri, 28 Aug 2026 14:00:05 -0400	[thread overview]
Message-ID: <apHMpfHQ5zivcVAB@redhat.com> (raw)
In-Reply-To: <a709459222a440dc7f821796b1a98ebaeb44572b.camel@suse.com>

On Fri, Aug 28, 2026 at 03:57:38PM +0200, Martin Wilck wrote:
> @Ben and others, I wasn't certain about this at first. Below is my flow
> of thoughts from the SUSE bugzilla, through which I came to think that
> it's correct. But this is quite subtle, so please give it some own
> thoughts ;-)
> 
> Martin
> 
> IIUC, if the "Conflicts" is removed, the timer unit will continue
> running through the switch root phase, and will be taken up by systemd
> through serialization 

But like Antonio said, isolate will stop the timer, since it's not a
dependency of initrd-switch-root.target, right? I'm a little fuzzy on
exactly how all the units get stopped, and you might be right that a
udev rule could run and trigger a timer after the isolate could catch
it. But I think you are right that this would be harmless if it could
occur.

-Ben

> 
> [*] upstream dracut version history related to multipathd.socket:
> 
> 247d2b ("fix(multipathd.service): adapt to upstream multipath-tools
> unit file") (dracut 056) added socket dep to the multipathd.service
> unit file.
> 02e646f ("fix(multipath): install multipathd.socket") (dracut 058) made
> sure the socket unit file was installed.
> 297525c ("fix(multipath): remove dependency on multipathd.socket")
> (dracut 060) removed both
> 
> 5e87b68 ("refactor(multipath): remove custom multipathd.service")
> (dracut 107) removed the .service file.
> 
> > ---
> >  multipath/multipath.rules.in | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/multipath/multipath.rules.in
> > b/multipath/multipath.rules.in
> > index 2ac1972f..7b605dd4 100644
> > --- a/multipath/multipath.rules.in
> > +++ b/multipath/multipath.rules.in
> > @@ -68,8 +68,18 @@ ENV{.SAVED_FM_WAIT_UNTIL}=="?*",
> > GOTO="pretend_mpath"
> >  # the --on-active timeout.
> >  #
> >  # We must trigger an "add" event because LVM2 will only act on
> > those.
> > -
> > -RUN+="@SYSDIR_BIN@/systemd-run --unit=cancel-multipath-wait-$kernel
> > --description 'cancel waiting for multipath siblings of $kernel' --
> > no-block --timer-property DefaultDependencies=no --timer-property
> > Conflicts=shutdown.target --timer-property Before=shutdown.target --
> > timer-property Conflicts=initrd-cleanup.service --timer-property
> > Before=initrd-cleanup.service --timer-property AccuracySec=500ms --
> > property DefaultDependencies=no --property Conflicts=shutdown.target
> > --property Before=shutdown.target --property Conflicts=initrd-
> > cleanup.service --property Before=initrd-cleanup.service --on-
> > active=$env{FIND_MULTIPATHS_WAIT_UNTIL} @SYSDIR_BIN@/udevadm trigger
> > --action=add $sys$devpath"
> > +#
> > +# These units are ordered Before=initrd-cleanup.service so that a
> > trigger that
> > +# is already in flight completes before the initrd switches root.
> > They must
> > +# *not* use Conflicts=initrd-cleanup.service: unlike
> > shutdown.target, whose
> > +# jobs are enqueued irreversibly, initrd-cleanup.service/start is a
> > plain job
> > +# enqueued by initrd-parse-etc.service via OnSuccess=. Starting a
> > unit that
> > +# conflicts with it therefore cancels that job instead of failing to
> > start,
> > +# and OnSuccess= is never retried, so the initrd would hang forever
> > with an
> > +# empty job queue. Stopping these units before switch-root is
> > already
> > +# guaranteed by the isolate to initrd-switch-root.target.
> > +
> > +RUN+="@SYSDIR_BIN@/systemd-run --unit=cancel-multipath-wait-$kernel
> > --description 'cancel waiting for multipath siblings of $kernel' --
> > no-block --timer-property DefaultDependencies=no --timer-property
> > Conflicts=shutdown.target --timer-property Before=shutdown.target --
> > timer-property Before=initrd-cleanup.service --timer-property
> > AccuracySec=500ms --property DefaultDependencies=no --property
> > Conflicts=shutdown.target --property Before=shutdown.target --
> > property Before=initrd-cleanup.service --on-
> > active=$env{FIND_MULTIPATHS_WAIT_UNTIL} @SYSDIR_BIN@/udevadm trigger
> > --action=add $sys$devpath"
> >  
> >  LABEL="pretend_mpath"
> >  ENV{DM_MULTIPATH_DEVICE_PATH}="1"
> 
> -- 
> Dr. Martin Wilck <mwilck@suse.com>
> SUSE Software Solutions Germany GmbH, Frankenstr. 146, 90461 Nürnberg,
> Germany
> Geschäftsführer: Jochen Jaser, Andrew McDonald (HRB 36809,AG Nürnberg)


      reply	other threads:[~2026-08-28 18:00 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-28  9:25 [PATCH] multipath.rules.in: fix race in unit cancel-multipath-wait-$kernel Antonio Alvarez Feijoo
2026-08-28 13:57 ` Martin Wilck
2026-08-28 18:00   ` Benjamin Marzinski [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=apHMpfHQ5zivcVAB@redhat.com \
    --to=bmarzins@redhat.com \
    --cc=antonio.feijoo@suse.com \
    --cc=dm-devel@lists.linux.dev \
    --cc=mwilck@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.