[dunfell,1/2] systemd: implement --initialized-match/nomatch arguments

systemd-udev-trigger.service by default triggeres all devices regardless
of whether they were already recognized by systemd-udevd.

There are machines (especially in embedded environments) where
systemd-udev-trigger.service is configured to run at a later stage of
the boot sequence, which can lead to quite a lot of devices being
triggered although they were already recognized by systemd-udevd.

Re-triggering a lot of devices is a relatively expensive operation and
therefore should be avoided if unnecessary.

Therefore this patch introduces --initialized-nomatch, which filters out
devices that are already present in the udev database. For consistance
reasons --initialized-match is implemented as well, which filters out devices
that are *not* already present in the udev database.

Signed-off-by: Pawan Badganchi <badganchipv@gmail.com>
---
 ...-initialized-match-nomatch-arguments.patch | 156 ++++++++++++++++++
 meta/recipes-core/systemd/systemd_244.5.bb    |   1 +
 2 files changed, 157 insertions(+)
 create mode 100644 meta/recipes-core/systemd/systemd/0001-udevadm-trigger-implement-initialized-match-nomatch-arguments.patch

This patch series adds a feature to systemd, which is typically not
allowed in a stable branch.

However I can see that there might be an argument that this is
actually fixing a bug (needless re-triggering of devices)

I'd love to get feedback from others as to whether I should take this patch.

Steve

On Wed, Jun 7, 2023 at 11:52 PM Pawan Badganchi <badganchipv@gmail.com> wrote:
>
> systemd-udev-trigger.service by default triggeres all devices regardless
> of whether they were already recognized by systemd-udevd.
>
> There are machines (especially in embedded environments) where
> systemd-udev-trigger.service is configured to run at a later stage of
> the boot sequence, which can lead to quite a lot of devices being
> triggered although they were already recognized by systemd-udevd.
>
> Re-triggering a lot of devices is a relatively expensive operation and
> therefore should be avoided if unnecessary.
>
> Therefore this patch introduces --initialized-nomatch, which filters out
> devices that are already present in the udev database. For consistance
> reasons --initialized-match is implemented as well, which filters out devices
> that are *not* already present in the udev database.
>
> Signed-off-by: Pawan Badganchi <badganchipv@gmail.com>
> ---
>  ...-initialized-match-nomatch-arguments.patch | 156 ++++++++++++++++++
>  meta/recipes-core/systemd/systemd_244.5.bb    |   1 +
>  2 files changed, 157 insertions(+)
>  create mode 100644 meta/recipes-core/systemd/systemd/0001-udevadm-trigger-implement-initialized-match-nomatch-arguments.patch
>
> diff --git a/meta/recipes-core/systemd/systemd/0001-udevadm-trigger-implement-initialized-match-nomatch-arguments.patch b/meta/recipes-core/systemd/systemd/0001-udevadm-trigger-implement-initialized-match-nomatch-arguments.patch
> new file mode 100644
> index 0000000000..2f7da0aff9
> --- /dev/null
> +++ b/meta/recipes-core/systemd/systemd/0001-udevadm-trigger-implement-initialized-match-nomatch-arguments.patch
> @@ -0,0 +1,156 @@
> +From 678f2b16676cd566c50bdec78350f732a9a3ee41 Mon Sep 17 00:00:00 2001
> +From: Danilo Krummrich <danilo.krummrich@bmw.de>
> +Date: Mon, 14 Jun 2021 17:46:00 +0200
> +Subject: [PATCH] udevadm: trigger: implement --initialized-match/nomatch
> + arguments
> +
> +systemd-udev-trigger.service by default triggeres all devices regardless
> +of whether they were already recognized by systemd-udevd.
> +
> +There are machines (especially in embedded environments) where
> +systemd-udev-trigger.service is configured to run at a later stage of
> +the boot sequence, which can lead to quite a lot of devices being
> +triggered although they were already recognized by systemd-udevd.
> +
> +Re-triggering a lot of devices is a relatively expensive operation and
> +therefore should be avoided if unnecessary.
> +
> +Therefore this patch introduces --initialized-nomatch, which filters out
> +devices that are already present in the udev database. For consistance
> +reasons --initialized-match is implemented as well, which filters out devices
> +that are *not* already present in the udev database.
> +
> +Replaces #19949.
> +
> +Upstream-Status: Backport [https://github.com/systemd/systemd/commit/678f2b16676cd566c50bdec78350f732a9a3ee41.patch]
> +Comment: Refreshed hunk from udevadm, _udevadm and udevadm-trigger.c
> +Signed-off-by: Pawan Badganchi <pawan.badganchi@kpit.com>
> +---
> + man/udevadm.xml               | 29 +++++++++++++++++++++++++++++
> + shell-completion/bash/udevadm |  3 ++-
> + shell-completion/zsh/_udevadm |  2 ++
> + src/udev/udevadm-trigger.c    | 13 +++++++++++++
> + 4 files changed, 46 insertions(+), 1 deletion(-)
> +
> +diff --git a/man/udevadm.xml b/man/udevadm.xml
> +index 24ed4a29c71d..af485711222e 100644
> +--- a/man/udevadm.xml
> ++++ b/man/udevadm.xml
> +@@ -395,6 +395,35 @@
> +             then each matching result is ORed, that is, all children of each specified device are triggered.</para>
> +           </listitem>
> +         </varlistentry>
> ++        <varlistentry>
> ++          <term><option>--initialized-match</option></term>
> ++          <term><option>--initialized-nomatch</option></term>
> ++          <listitem>
> ++            <para>When <option>--initialized-match</option> is specified, trigger events for devices
> ++            that are already initialized by <command>systemd-udevd</command>, and skip devices that
> ++            are not initialized yet.</para>
> ++            <para>When <option>--initialized-nomatch</option> is specified, trigger events for devices
> ++            that are not initialized by <command>systemd-udevd</command> yet, and skip devices that
> ++            are already initialized.</para>
> ++            <para>Here, initialized devices are those for which at least one udev rule already
> ++            completed execution – for any action but <literal>remove</literal> — that set a property
> ++            or other device setting (and thus has an entry in the udev device database). Devices are
> ++            no longer considered initialized if a <literal>remove</literal> action is seen for them
> ++            (which removes their entry in the udev device database). Note that devices that have no
> ++            udev rules are never considered initialized, but might still be announced via the sd-device
> ++            API (or similar). Typically, it is thus essential that applications which intend to use
> ++            such a match, make sure a suitable udev rule is installed that sets at least one property
> ++            on devices that shall be matched.</para>
> ++            <para>WARNING: <option>--initialized-nomatch</option> can potentially save a significant
> ++            amount of time compared to re-triggering all devices in the system and e.g. can be used to
> ++            optimize boot time. However, this is not safe to be used in a boot sequence in general.
> ++            Especially, when udev rules for a device depend on its parent devices (e.g.
> ++            <literal>ATTRS</literal> or <literal>IMPORT{parent}</literal> keys, see
> ++            <citerefentry><refentrytitle>udev</refentrytitle><manvolnum>7</manvolnum></citerefentry>
> ++            for more details), the final state of the device becomes easily unstable with this option.
> ++            </para>
> ++          </listitem>
> ++        </varlistentry>
> +         <varlistentry>
> +           <term><option>-w</option></term>
> +           <term><option>--settle</option></term>
> +diff --git a/shell-completion/bash/udevadm b/shell-completion/bash/udevadm
> +index 4d52597cbfdf..23ce02365c7f 100644
> +--- a/shell-completion/bash/udevadm
> ++++ b/shell-completion/bash/udevadm
> +@@ -51,7 +51,8 @@
> +         [INFO_STANDALONE]='-r --root -a --attribute-walk -x --export -e --export-db -c --cleanup-db
> +                            -w --wait-for-initialization'
> +         [INFO_ARG]='-q --query -p --path -n --name -P --export-prefix -d --device-id-of-file'
> +-        [TRIGGER_STANDALONE]='-v --verbose -n --dry-run -w --settle --wait-daemon'
> ++        [TRIGGER_STANDALONE]='-v --verbose -n --dry-run -w --settle --wait-daemon
> ++                              --initialized-match --initialized-nomatch'
> +         [TRIGGER_ARG]='-t --type -c --action -s --subsystem-match -S --subsystem-nomatch
> +                        -a --attr-match -A --attr-nomatch -p --property-match
> +                        -g --tag-match -y --sysname-match --name-match -b --parent-match'
> +diff --git a/shell-completion/zsh/_udevadm b/shell-completion/zsh/_udevadm
> +index 8a10237e3d66..63df8b7c9ecc 100644
> +--- a/shell-completion/zsh/_udevadm
> ++++ b/shell-completion/zsh/_udevadm
> +@@ -30,7 +30,9 @@
> +         '--property-match=[Trigger events for devices with a matching property value.]' \
> +         '--tag-match=property[Trigger events for devices with a matching tag.]' \
> +         '--sysname-match=[Trigger events for devices with a matching sys device name.]' \
> +-        '--parent-match=[Trigger events for all children of a given device.]'
> ++        '--parent-match=[Trigger events for all children of a given device.]' \
> ++        '--initialized-match[Trigger events for devices that are already initialized.]' \
> ++        '--initialized-nomatch[Trigger events for devices that are not initialized yet.]'
> + }
> +
> + (( $+functions[_udevadm_settle] )) ||
> +diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c
> +index e4b3150e6039..56921e2cc62e 100644
> +--- a/src/udev/udevadm-trigger.c
> ++++ b/src/udev/udevadm-trigger.c
> +@@ -10,6 +10,7 @@
> + #include "device-private.h"
> + #include "fd-util.h"
> + #include "fileio.h"
> ++#include "parse-util.h"
> + #include "path-util.h"
> + #include "process-util.h"
> + #include "set.h"
> +@@ -130,6 +131,8 @@
> +                "  -y --sysname-match=NAME           Trigger devices with this /sys path\n"
> +                "     --name-match=NAME              Trigger devices with this /dev name\n"
> +                "  -b --parent-match=NAME            Trigger devices with that parent device\n"
> ++               "     --initialized-match            Trigger devices that are already initialized\n"
> ++               "     --initialized-nomatch          Trigger devices that are not initialized yet\n"
> +                "  -w --settle                       Wait for the triggered events to complete\n"
> +                "     --wait-daemon[=SECONDS]        Wait for udevd daemon to be initialized\n"
> +                "                                    before triggering uevents\n"
> +@@ -142,6 +145,8 @@
> +         enum {
> +                 ARG_NAME = 0x100,
> +                 ARG_PING,
> ++                ARG_INITIALIZED_MATCH,
> ++                ARG_INITIALIZED_NOMATCH,
> +         };
> +
> +         static const struct option options[] = {
> +@@ -158,6 +163,8 @@
> +                 { "sysname-match",     required_argument, NULL, 'y'      },
> +                 { "name-match",        required_argument, NULL, ARG_NAME },
> +                 { "parent-match",      required_argument, NULL, 'b'      },
> ++                { "initialized-match",     no_argument,       NULL, ARG_INITIALIZED_MATCH     },
> ++                { "initialized-nomatch",   no_argument,       NULL, ARG_INITIALIZED_NOMATCH   },
> +                 { "settle",            no_argument,       NULL, 'w'      },
> +                 { "wait-daemon",       optional_argument, NULL, ARG_PING },
> +                 { "version",           no_argument,       NULL, 'V'      },
> +@@ -301,7 +308,12 @@
> +                         }
> +                         break;
> +                 }
> +-
> ++                case ARG_INITIALIZED_MATCH:
> ++                case ARG_INITIALIZED_NOMATCH:
> ++                        r = device_enumerator_add_match_is_initialized(e, c == ARG_INITIALIZED_MATCH ? MATCH_INITIALIZED_YES : MATCH_INITIALIZED_NO);
> ++                        if (r < 0)
> ++                                return log_error_errno(r, "Failed to set initialized filter: %m");
> ++                        break;
> +                 case 'V':
> +                         return print_version();
> +                 case 'h':
> diff --git a/meta/recipes-core/systemd/systemd_244.5.bb b/meta/recipes-core/systemd/systemd_244.5.bb
> index bd66d82932..a398043308 100644
> --- a/meta/recipes-core/systemd/systemd_244.5.bb
> +++ b/meta/recipes-core/systemd/systemd_244.5.bb
> @@ -39,6 +39,7 @@ SRC_URI += "file://touchscreen.rules \
>             file://CVE-2023-26604-2.patch \
>             file://CVE-2023-26604-3.patch \
>             file://CVE-2023-26604-4.patch \
> +           file://0001-udevadm-trigger-implement-initialized-match-nomatch-arguments.patch \
>             "
>
>  # patches needed by musl
> --
> 2.38.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#182500): https://lists.openembedded.org/g/openembedded-core/message/182500
> Mute This Topic: https://lists.openembedded.org/mt/99403398/3620601
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [steve@sakoman.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Hi,

Could you please take this patch

On Wed, Nov 22, 2023 at 7:45 PM Pawan Badganchi <badganchipv@gmail.com> wrote:
>
> Hi,
>
> Could you please take this patch

Sorry, this seried is a feature change and not a candidate for a stable release.

Steve

> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#191135): https://lists.openembedded.org/g/openembedded-core/message/191135
> Mute This Topic: https://lists.openembedded.org/mt/99403398/3620601
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [steve@sakoman.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>