Differential D49363

sockbuf: remove sbflush_internal() and sbrelease_internal() shims
ClosedPublic
Actions

Authored by glebius on Mar 14 2025, 6:06 PM.

Details

Reviewers

markj

Group Reviewers

network

Commits

rG371392bc10f2: sockbuf: remove sbflush_internal() and sbrelease_internal() shims

Summary

This functions serve just one purpose - allow to call sbdestroy() from
sofree() without triggering unlocked mutex assertions. Let's just don't
save on locking with INVARIANTS kernel and this will allow to clean up all
these shims. Should be no functional changes.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

glebius created this revision.Mar 14 2025, 6:06 PM

Herald added subscribers: olce, imp. · View Herald TranscriptMar 14 2025, 6:06 PM

glebius requested review of this revision.Mar 14 2025, 6:06 PM

Harbormaster completed remote builds in B62922: Diff 152270.Mar 14 2025, 6:06 PM

glebius added a child revision: D49364: sockbuf: provide sbunreserve_locked() which is a complement to sbreserve().Mar 14 2025, 6:07 PM

This feels weird. Suppose there is some bug that is prevented by acquiring these mutexes. Now the bug will be absent in INVARIANTS kernels, but present in production kernels, which will making debugging very confusing. I'm not sure that this is worth it just to slightly simplify some call paths?

In D49363#1126167, @markj wrote:

This feels weird. Suppose there is some bug that is prevented by acquiring these mutexes. Now the bug will be absent in INVARIANTS kernels, but present in production kernels, which will making debugging very confusing. I'm not sure that this is worth it just to slightly simplify some call paths?

Nope, it is the opposite! Right now there could be a bug that would call sbrelease_internal() without holding mutex and INVARIANTS won't catch that. Same for sbflush_internal. With the change, the functions start asserting the lock. And the only place that has a legitimate unlocked call is properly marked.

markj accepted this revision as: markj.Mar 23 2025, 10:50 AM

This revision is now accepted and ready to land.Mar 23 2025, 10:50 AM

Closed by commit rG371392bc10f2: sockbuf: remove sbflush_internal() and sbrelease_internal() shims (authored by glebius). · Explain WhyMon, Mar 24, 6:41 AM

This revision was automatically updated to reflect the committed changes.

glebius added a commit: rG371392bc10f2: sockbuf: remove sbflush_internal() and sbrelease_internal() shims.