Page MenuHomeFreeBSD

rc: Remove rc_fast_and_loose
ClosedPublic

Authored by 0mp on Oct 23 2024, 12:58 PM.
Tags
Referenced Files
Unknown Object (File)
Sat, Nov 30, 11:35 AM
Unknown Object (File)
Fri, Nov 29, 1:00 AM
Unknown Object (File)
Mon, Nov 18, 3:13 PM
Unknown Object (File)
Sun, Nov 17, 3:51 AM
Unknown Object (File)
Sun, Nov 10, 5:31 AM
Unknown Object (File)
Sat, Nov 9, 11:14 PM
Unknown Object (File)
Fri, Nov 8, 11:52 PM
Unknown Object (File)
Wed, Nov 6, 2:02 AM
Subscribers

Diff Detail

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

Event Timeline

0mp requested review of this revision.Oct 23 2024, 12:58 PM
mhorne added a subscriber: mhorne.
mhorne added inline comments.
libexec/rc/rc.subr
1906–1909

Looks like this can go as well.

share/man/man8/rc.subr.8
1039–1040

Arguably, this item can be removed altogether, as it no longer describes a "check" at all.

This revision is now accepted and ready to land.Oct 23 2024, 3:22 PM
brooks added a subscriber: brooks.
brooks added inline comments.
libexec/rc/rc.subr
1799–1801

We could probably drop this case as well (maybe in a seperate commit). People have had >20 years to notice the warning and update their systems.

Update docs and remove an unused boottrace function

This revision now requires review to proceed.Oct 23 2024, 4:17 PM
0mp planned changes to this revision.Oct 23 2024, 4:18 PM
0mp marked 2 inline comments as done.
0mp added inline comments.
libexec/rc/rc.subr
1799–1801

The .sh case, correct?

share/man/man8/rc.subr.8
1039–1040

Good catch!

0mp requested review of this revision.Oct 23 2024, 4:18 PM
0mp marked an inline comment as done.
This revision is now accepted and ready to land.Oct 23 2024, 5:02 PM
libexec/rc/rc.subr
1799–1801

Yes

0mp marked 2 inline comments as done.Oct 23 2024, 7:56 PM
0mp added inline comments.

Approved.

This probably deserves an entry in UPDATING as well as the relnotes tag, since it breaks backward compatibility and applies to those who track main as well as release users.

0mp marked an inline comment as done.Oct 23 2024, 8:35 PM

Approved.

This probably deserves an entry in UPDATING as well as the relnotes tag, since it breaks backward compatibility and applies to those who track main as well as release users.

Thanks! I'll make sure to add an appropriate UPDATING entry.

Approved, plus a minor nit.

share/man/man8/rc.subr.8
1026–1027

No need for the newline here.

share/man/man8/rc.subr.8
1026–1027

Thanks for catching that. The whole sentence should read: However, it might also be ignored by .Ic run_rc_script if.

I've got some reports from the NetBSD community that setting rc_fast_and_loose on NetBSD booting on vax simh makes rc finish in 109 seconds instead of 183 seconds (https://mastodon.sdf.org/@mrgtwentythree/113364676304360235).

At the same time, brooks@ mentioned in Bugzilla (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=282255#c4) that the benefits of rc_fast_and_loose are probably negligible.

In D47264#1078101, @0mp wrote:

I've got some reports from the NetBSD community that setting rc_fast_and_loose on NetBSD booting on vax simh makes rc finish in 109 seconds instead of 183 seconds (https://mastodon.sdf.org/@mrgtwentythree/113364676304360235).

At the same time, brooks@ mentioned in Bugzilla (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=282255#c4) that the benefits of rc_fast_and_loose are probably negligible.

Well, Brooks' comment should probably be taken in the context of "on hardware where FreeBSD runs." With that, I suspect that that claim is still true.

Even if there is a marginal improvement (and there probably is on low-powered hardware or emulators), the option is not obviously worth the complexity, given that:

  • it's broken (on FreeBSD), and not well-known
  • fixing it would require a lot of time
  • it introduces extra constraints for rc scripts, which are hard enough to write correctly already

I'd still vote for removing it.

I'm sure it adds appreciable speedup in some cases, but I can't see us fixing support and keeping it working. Best to stop pretending and focus on lower hanging fruit.

  • Add UPDATING entry
  • Fix typos in rc.subr.8 and do some extra wordsmithing there
This revision now requires review to proceed.Oct 25 2024, 5:44 PM
0mp marked an inline comment as done.Oct 25 2024, 6:51 PM
This revision is now accepted and ready to land.Oct 28 2024, 1:43 PM
This revision was automatically updated to reflect the committed changes.

Thank you all for feedback! :)