PR#23: pid1: Do not updated state for device units being processed by udev - rpms/systemd

rpms / systemd

#23 pid1: Do not updated state for device units being processed by udev

Merged 2 days ago by teknoraver. Opened 2 days ago by ryantimwilson.

rpms/ ryantimwilson/systemd udev-fix-maybe into c10s-sig-hyperscale

pid1: Do not updated state for device units being processed by udev

Ryan Wilson • 2 days ago

14b620

0001-pid1-Do-not-update-state-for-device-units-being-proc.patch

file added

+97

		`@@ -0,0 +1,97 @@`
		`+ From 3a1ea280dd3c8c70b616467f4fd375cf2e4d31fa Mon Sep 17 00:00:00 2001`
		`+ From: Ryan Wilson <ryantimwilson@meta4.com>`
		`+ Date: Sat, 23 Nov 2024 20:30:31 -0800`
		`+ Subject: [PATCH] pid1: Do not update state for device units being processed by`
		`+ udev`
		`+`
		`+ ---`
		`+ src/core/device.c \| 40 ++++++++++++++++++++++++++++++++++++----`
		`+ src/core/device.h \| 1 +`
		`+ 2 files changed, 37 insertions(+), 4 deletions(-)`
		`+`
		`+ diff --git a/src/core/device.c b/src/core/device.c`
		`+ index d8567676a7..d13140759d 100644`
		`+ --- a/src/core/device.c`
		`+ +++ b/src/core/device.c`
		`+ @@ -334,6 +334,11 @@ static int device_coldplug(Unit *u) {`
		`+ static void device_catchup(Unit *u) {`
		`+ Device *d = ASSERT_PTR(DEVICE(u));`
		`+`
		`+ + if (d->enumerated_udev_processing) {`
		`+ + log_unit_debug(u, "Device is being processed by systemd-udevd. Skipping updating state.");`
		`+ + return;`
		`+ + }`
		`+ +`
		`+ /* Second, let's update the state with the enumerated state */`
		`+ device_update_found_one(d, d->enumerated_found, DEVICE_FOUND_MASK);`
		`+ }`
		`+ @@ -742,6 +747,14 @@ static bool device_is_ready(sd_device *dev) {`
		`+`
		`+ assert(dev);`
		`+`
		`+ + r = device_is_processed(dev);`
		`+ + if (r < 0)`
		`+ + log_device_warning_errno(dev, r, "Failed to check if device is processed, assuming device is processed: %m");`
		`+ + if (r == 0) {`
		`+ + log_device_debug(dev, "Device busy: device is currently being processed");`
		`+ + return false;`
		`+ + }`
		`+ +`
		`+ if (device_for_action(dev, SD_DEVICE_REMOVE))`
		`+ return false;`
		`+`
		`+ @@ -1051,16 +1064,35 @@ static void device_enumerate(Manager *m) {`
		`+ _cleanup_set_free_ Set ready_units = NULL, not_ready_units = NULL;`
		`+ Device *d;`
		`+`
		`+ - if (device_is_processed(dev) <= 0)`
		`+ + r = device_setup_units(m, dev, &ready_units, &not_ready_units);`
		`+ + if (r < 0) {`
		`+ + log_device_debug_errno(dev, r, "Failed to set up device units: %m");`
		`+ continue;`
		`+ + }`
		`+`
		`+ - if (device_setup_units(m, dev, &ready_units, &not_ready_units) < 0)`
		`+ + r = device_is_processed(dev);`
		`+ + if (r < 0)`
		`+ + log_device_warning_errno(dev, r, "Failed to determine if device is processed by systemd-udevd, assuming device is processed: %m");`
		`+ + if (r == 0) {`
		`+ + SET_FOREACH(d, ready_units) {`
		`+ + log_unit_warning(UNIT(d), "Device unit is ready but currently processing in systemd-udevd.");`
		`+ + d->enumerated_udev_processing = true;`
		`+ + }`
		`+ + SET_FOREACH(d, not_ready_units) {`
		`+ + log_unit_debug(UNIT(d), "Device unit currently processing in systemd-udevd.");`
		`+ + d->enumerated_udev_processing = true;`
		`+ + }`
		`+ continue;`
		`+ + }`
		`+`
		`+ - SET_FOREACH(d, ready_units)`
		`+ + SET_FOREACH(d, ready_units) {`
		`+ + log_unit_debug(UNIT(d), "Device unit found in systemd-udevd.");`
		`+ device_update_found_one(d, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV);`
		`+ - SET_FOREACH(d, not_ready_units)`
		`+ + }`
		`+ + SET_FOREACH(d, not_ready_units) {`
		`+ + log_unit_debug(UNIT(d), "Device unit not found in systemd-udevd.");`
		`+ device_update_found_one(d, DEVICE_NOT_FOUND, DEVICE_FOUND_UDEV);`
		`+ + }`
		`+ }`
		`+`
		`+ return;`
		`+ diff --git a/src/core/device.h b/src/core/device.h`
		`+ index 9dd6fb57c2..14d5547c7a 100644`
		`+ --- a/src/core/device.h`
		`+ +++ b/src/core/device.h`
		`+ @@ -31,6 +31,7 @@ struct Device {`
		`+ DeviceFound found, deserialized_found, enumerated_found;`
		`+`
		`+ bool bind_mounts;`
		`+ + bool enumerated_udev_processing;`
		`+`
		`+ /* The SYSTEMD_WANTS udev property for this device the last time we saw it */`
		`+ char **wants_property;`
		`+ --`
		`+ 2.43.5`
		`+`

systemd.spec

file modified

+4 -1

		`@@ -44,7 +44,7 @@`
		`# Allow users to specify the version and release when building the rpm by`
		`# setting the %%version_override and %%release_override macros.`
		`Version: %{?version_override}%{!?version_override:256.7}`
		`- Release: %{?release_override}%{!?release_override:1.6}%{?dist}`
		`+ Release: %{?release_override}%{!?release_override:1.7}%{?dist}`

		`%global stable %(c="%version"; [ "$c" = "${c#.}" ]; echo $?)`

		`@@ -151,6 +151,9 @@`
		`# pam_systemd: Make pam_systemd 256 backwards compatible to logind 255`
		`Patch0905: 0001-pam_systemd-Make-pam_systemd-256-backwards-compatibl.patch`

		`+ # pid1: Do not updated state for device units being processed by udev`
		`+ Patch0906: 0001-pid1-Do-not-update-state-for-device-units-being-proc.patch`
		`+`
		`%endif`

		`%ifarch %{ix86} x86_64 aarch64 riscv64`

ryantimwilson commented 2 days ago

This patch is an attempt to fix https://github.com/systemd/systemd/issues/35329

First, we will deploy to Facebook and verify it is fixed in our test clusters. If so, we will upstream this patch and backport to both Centos and Facebook versions.