Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(RHEL-33815) udev-node: optimize device node symlink creation #307

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

jamacku
Copy link
Member

@jamacku jamacku commented Dec 20, 2024

No description provided.

keszybz and others added 6 commits December 20, 2024 15:30
-1 was used everywhere, but -EBADF or -EBADFD started being used in various
places. Let's make things consistent in the new style.

Note that there are two candidates:
EBADF 9 Bad file descriptor
EBADFD 77 File descriptor in bad state

Since we're initializating the fd, we're just assigning a value that means
"no fd yet", so it's just a bad file descriptor, and the first errno fits
better. If instead we had a valid file descriptor that became invalid because
of some operation or state change, the other errno would fit better.

In some places, initialization is dropped if unnecessary.

Partially backported from commit systemd/systemd@254d131

(cherry picked from commit 254d131)

Related: RHEL-33815
These 2 operations are inseparable.

(cherry picked from commit c9032f9)

Related: RHEL-33815
We likely always want to open the directory via a slink.

There's currently only one caller so it doesn't make any difference in practice
but I think it's still nicer.

No functional change.

(cherry picked from commit 72a459a)

Related: RHEL-33815
That's usually the errno code we return when a device cannot be found because
it's been unplugged.

(cherry picked from commit e8a54a4)

Related: RHEL-33815
And make the new format the one we expect as it should replace the old one
pretty quickly.

(cherry picked from commit 6d90488)

Related: RHEL-33815
If multiple devices requested the same device node symlink with the same
priority, then previously we read O(N^2) of files saved in
/run/udev/links.

This makes if the requested symlink already exists with equal or higher
priority, then the symlink is kept, and skip to read all existing files,
except for one related to the current device node, in /run/udev/links.
Hence, the total amount of file read becomes O(N).

This improves performance of testcase_simultaneous_events_2 added by the
previous commit about 30%.
Before (32.8 sec):
```
 ## 3 iterations start: 11:13:44.690953163
 ## 3 iterations end: 11:14:17.493974927
```
After (23.8 sec):
```
 ## 3 iterations start: 11:17:53.869938387
 ## 3 iterations end: 11:18:17.624268345
```

This is based on the idea and analysis by Franck Bui.

Replaces #25839.

Co-authored-by: Franck Bui <[email protected]>
(cherry picked from commit 331aa7a)

Resolves: RHEL-33815
@jamacku jamacku added this to the RHEL-9.6.0 milestone Dec 20, 2024
@jamacku jamacku requested a review from msekletar December 20, 2024 14:36
@github-actions github-actions bot changed the title udev-node: optimize device node symlink creation (RHEL-33815) udev-node: optimize device node symlink creation Dec 20, 2024
@github-actions github-actions bot added tracker/invalid-product pr/needs-ci Formerly needs-ci pr/needs-review Formerly needs-review labels Dec 20, 2024
Copy link

github-actions bot commented Dec 20, 2024

Commit validation

Tracker - RHEL-33815

The following commits meet all requirements

commit upstream
a6ebdb7 - tree-wide: use -EBADF for fd initialization systemd/systemd@254d131
41f6c53 - udev: merge link_directory_lock() into link_directory_open() systemd/systemd@c9032f9
9d4f18e - udev: let stack_directory_open() convert a slink into a dirname itself… systemd/systemd@72a459a
229f1f2 - udev: return ENODEV if link_directory_read_one() can't find the devnod… systemd/systemd@e8a54a4
cf23250 - udev: simplify a bit stack_directory_find_prioritized_devnode() systemd/systemd@6d90488
623d77b - udev-node: optimize device node symlink creation systemd/systemd@331aa7a
3b3ba0e - udev: update devlink with the newer device node even when priority is … systemd/systemd@7ec5ce5

Follow-up detection

Failed

🔴 Some follow-up commits for this Pull Request were detected in upstream
🔴 Some mentions of commits from this Pull Request were detected in upstream

Follow-ups

commit follow-up
a6ebdb7 - tree-wide: use -EBADF for fd initialization systemd/systemd@906682a

Commit mentions

commit mention
623d77b - udev-node: optimize device node symlink creation systemd/systemd@4ef83d9
systemd/systemd@df1dccd

Tracker validation

Success

🟢 Tracker RHEL-33815 has set desired product: rhel-9.6
🟢 Tracker RHEL-33815 has set desired component: systemd
🟢 Tracker RHEL-33815 has been approved
🟢 Tracker RHEL-33815 has set severity


Pull Request validation

Failed

🔴 Failed or pending checks - build (GCC, openssl)[failure],build (GCC, auto)[failure],build (GCC_ASAN_UBSAN, auto)[failure],build (CLANG_RELEASE, auto)[failure],build (CLANG, gcrypt)[failure],build (CLANG_ASAN_UBSAN_NO_DEPS, auto)[failure],build (CLANG, auto)[failure],build (CLANG_ASAN_UBSAN, auto)[failure] Failed or pending statuses - CentOS CI (CentOS Stream 9)[failure],CentOS CI (CentOS Stream 9 + sanitizers)[failure]
🔴 Review - Missing review from a member (1 required)

… equivalent

Several udev rules depends on the previous behavior, i.e. that udev
replaces the devlink with the newer device node when the priority is
equivalent. Let's relax the optimization done by
331aa7a.

Follow-up for 331aa7a.

Note, the offending commit drops O(N) of file reads per uevent, and this
commit does not change the computational order. So, hopefully the
performance impact of this change is small enough.

Fixes #28141.

(cherry picked from commit 7ec5ce5)

Resolves: RHEL-33815
@jamacku jamacku marked this pull request as draft December 20, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants