Page MenuHomeFreeBSD

Throws exception to block redundant devmatch events
Needs ReviewPublic

Authored by iciliaat_gmail.com on Apr 20 2024, 9:59 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jun 24, 7:49 AM
Unknown Object (File)
Sun, Jun 23, 12:13 AM
Unknown Object (File)
Jun 17 2024, 8:40 PM
Unknown Object (File)
Jun 5 2024, 5:38 PM
Unknown Object (File)
Jun 3 2024, 10:45 AM
Unknown Object (File)
Jun 3 2024, 5:35 AM
Unknown Object (File)
Jun 2 2024, 10:56 PM
Unknown Object (File)
Apr 27 2024, 12:25 AM
Subscribers

Details

Reviewers
cperciva
Summary

In devd.cc, certain nomatch events are processed multiple times, impacting the boot time unnecessarily. This patch fixes the issue by the use of a flag, in the form of /var/run/devmatch_ran, that is made every time the system boots, to discern necessary from redundant events.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 57245
Build 54133: arc lint + arc unit

Event Timeline

sbin/devd/devd.cc
870

This seems to be comparing the timestamp on the file to the *current* time. We want to compare it to the time the event was *generated* (which is why we added the time=foo field to events).

871

Why not just return;?

1054

Unnecessary blank line.

So which events and why? I dont think the appraoch outlined here can possibly work.

sbin/devd/devd.cc
870

The events from the kernel don't have timestamps. We add time= in devd, and by then it's too late.

sbin/devd/devd.cc
870

Also, you don't know that this is a valid test: you are still racing devmatch if you add timestamps to the events. The problem is that devmatch looks at the state of the system at a given time. There's no guarantee noting that time will help you filter events that happen between when you take the snapshot and when you note the time.

Also, it bakes in that all events aren't so good... but most events are good.

The only real way to fix this is to move devmatch into devd. Anything else is too racy.

sbin/devd/devd.cc
870

Also, this is early boot, ntpd might step the time.

sbin/devd/devd.cc
870

The events from the kernel don't have timestamps. We add time= in devd, and by then it's too late.

The idea is that we're going to add a timestamp, that's a separate patch.

870

Also, this is early boot, ntpd might step the time.

I was expecting that we would run this before ntpd starts. But yes we need to be aware of that.

sbin/devd/devd.cc
870

It's still too racey even with that as i explained.

And it's solving the problem wrong. The whole issue is we reparse linker.hints for every fork. If devmatch moved entirely into devd we would onlybparse linker hints once and we'd retain knowledge of what we've loaded and we'd know we had all the events to do the freeze thaw properly and more often when we get bursts of new devices.

870

If time steps asynchronously this can't work. Ntpd does that at some random time after it starts.

Rather than prevent, you are much much better off moving devmatch into devd. It's unclear to me if you need to process all the events before calling daemon or if you can defer the module stuff to a thread....

sbin/devd/devd.cc
871

I decided to use an exception because I wanted it to be caught by the function calling it, so that I can then log the incident with devdlog. This allows for visibility into what's going on in the system.