Page MenuHomeFreeBSD
Differential D33477

ithread(9): update top-level description
ClosedPublic
Actions

Authored by mhorne on Dec 15 2021, 5:26 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Mar 22, 6:33 AM
Unknown Object (File)
Fri, Mar 21, 2:45 AM
Unknown Object (File)
Fri, Mar 14, 3:07 AM
Unknown Object (File)
Thu, Mar 13, 10:36 AM
Unknown Object (File)
Thu, Mar 13, 8:29 AM
Unknown Object (File)
Thu, Feb 27, 12:21 PM
Unknown Object (File)
Sun, Feb 23, 3:05 PM
Unknown Object (File)
Feb 18 2025, 6:20 PM
View All 99 Files
Subscribers
bcr
imp
pauamma_gundo.com

Details

Reviewers
jhb
ygy
pauamma_gundo.com
Group Reviewers
manpages
Commits
rGda82ab9801fb: intr_event(9): update top-level description
rGdfc91493abc6: intr_event(9): update top-level description
Summary

The ithread has been subsumed by the 'interrupt event' object, so
update the description to reflect this by describing an interrupt event
and its contents. We've also moved on from having a single handler
function to the split filter-and-handler model. Explain the purpose and
constraints of these two handlers.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

mhorne created this revision.Dec 15 2021, 5:26 PM
Herald added a subscriber: imp. · View Herald TranscriptDec 15 2021, 5:27 PM
mhorne requested review of this revision.Dec 15 2021, 5:27 PM
mhorne added a child revision: D33478: ithread(9): update existing function descriptions.
Harbormaster completed remote builds in B43384: Diff 100087.Dec 15 2021, 5:27 PM
mhorne added a comment.Dec 15 2021, 5:31 PM

Looking for content review primarily before wording/grammer/formatting.

imp added inline comments.Dec 15 2021, 6:26 PM
share/man/man9/ithread.9
76–78 ↗(On Diff #100087)

line breaks here are weird.

mhorne mentioned this in D33478: ithread(9): update existing function descriptions.Dec 15 2021, 6:53 PM
pauamma_gundo.com added a subscriber: pauamma_gundo.com.Dec 20 2021, 3:24 AM
pauamma_gundo.com added inline comments.
share/man/man9/ithread.9
129 ↗(On Diff #100087)

Typo for "intr"?

mhorne updated this revision to Diff 110233.Sep 6 2022, 8:47 PM
mhorne marked 2 inline comments as done.

Rebase. Handle feedback, rework text describing locks and sleeping, and add subsections to organize the description somewhat.

Harbormaster completed remote builds in B47269: Diff 110233.Sep 6 2022, 8:48 PM
jhb accepted this revision.Sep 30 2022, 9:05 PM
jhb added inline comments.
share/man/man9/intr_event.9
92
107

I think you want to be careful here about the phrase "interrupt handler" to avoid ambiguity. Here I think you mean the threaded handler?

114

Not sure if you want to clarify that you can't call malloc at all in a filter?

This revision is now accepted and ready to land.Sep 30 2022, 9:05 PM
pauamma_gundo.com added a comment.Oct 5 2022, 4:21 PM

Other than these nits (and bumping .Dd), English in this LGTM.

share/man/man9/intr_event.9
81
96
107

Seconded.

114

The clearer, the better.

mhorne updated this revision to Diff 111513.Oct 6 2022, 5:58 PM
mhorne marked 7 inline comments as done.

Update Handler Constraints to address review comments.
Avoid referring to "filter" and "handler" functions as "callbacks".

I discovered that the filter and threaded routines are described in detail
already in bus_setup_intr(9). I think the overlap is acceptable, however.

This revision now requires review to proceed.Oct 6 2022, 5:58 PM
Harbormaster completed remote builds in B47714: Diff 111513.Oct 6 2022, 5:59 PM
mhorne updated this revision to Diff 111520.Oct 6 2022, 8:25 PM

Small further tweaks, mainly, clarify that filters are executed in a critical section.

Harbormaster completed remote builds in B47719: Diff 111520.Oct 6 2022, 8:25 PM
bcr accepted this revision as: manpages.Oct 7 2022, 2:05 PM
bcr added a subscriber: bcr.

OK from manpages.

jhb accepted this revision.Oct 7 2022, 5:58 PM
jhb added inline comments.
share/man/man9/intr_event.9
114

Suggestion: s/disallowed/not permitted/? (Not a strong preference either way though)

121–122

You could maybe just say "non-sleepable locks"

This revision is now accepted and ready to land.Oct 7 2022, 5:58 PM
pauamma_gundo.com requested changes to this revision.Oct 9 2022, 7:36 AM
pauamma_gundo.com added inline comments.
share/man/man9/intr_event.9
114

"forbidden" feels more less stilted than "disallowed" and less likely to be misread by missing "not" than "not permitted".

121–122

Seconded.

124

"should", not "must"?

128

Or "later, outside (mumble) context".

This revision now requires changes to proceed.Oct 9 2022, 7:36 AM
mhorne updated this revision to Diff 111592.Oct 9 2022, 3:16 PM
mhorne marked 5 inline comments as done.

Handle feedback.

Harbormaster completed remote builds in B47755: Diff 111592.Oct 9 2022, 3:17 PM
mhorne marked an inline comment as done.Oct 9 2022, 3:20 PM
pauamma_gundo.com accepted this revision.Oct 9 2022, 7:07 PM

English LGTM. Can't say beyond that.

This revision is now accepted and ready to land.Oct 9 2022, 7:07 PM
jhb accepted this revision.Oct 11 2022, 5:19 PM
Closed by commit rGdfc91493abc6: intr_event(9): update top-level description (authored by mhorne). · Explain WhyOct 15 2022, 6:52 PM
This revision was automatically updated to reflect the committed changes.
mhorne added a commit: rGdfc91493abc6: intr_event(9): update top-level description.
mhorne added a commit: rGda82ab9801fb: intr_event(9): update top-level description.Oct 30 2022, 2:16 PM

Revision Contents
Changeset List

PathSize
share/
man/
man9/
79 lines

Diff 111834

View Options

share/man/man9/intr_event.9

  • This file was added.
.\" Copyright (c) 2021 The FreeBSD Foundation
.\"
.\" This document was written by Ed Maste under sponsorhip from
.\" The FreeBSD Foundation.
.\"
.\" Redistribution and use in source and binary forms, with or without
.\" modification, are permitted provided that the following conditions
.\" are met:
.\" 1. Redistributions of source code must retain the above copyright
.\" notice, this list of conditions and the following disclaimer.
.\" 2. Redistributions in binary form must reproduce the above copyright
.\" notice, this list of conditions and the following disclaimer in the
.\" documentation and/or other materials provided with the distribution.
.\"
.\" THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
.\" IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
.\" OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
.\" IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
.\" INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
.\" NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
.\" DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
.\" THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
.\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
.\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
.\"
.\" $FreeBSD$
.\"
.Dd August 16, 2021
.Dt GONE_IN 9
.Os
.Sh NAME
.Nm gone_in ,
.Nm gone_in_dev
.Nd deprecation notice functions
.Sh SYNOPSIS
.In sys/systm.h
.Ft void
.Fn gone_in "int major" "const char *msg"
.Ft void
.Fn gone_in_dev "device_t dev" "int major" "const char *msg"
.Sh DESCRIPTION
The
.Fn gone_in
functions are used to provide a notice that the kernel is using a driver or
some other functionality that is deprecated, and will be removed in a future
.Fx
release.
The notice is sent to the kernel
.Xr dmesg 8
log and will appear on the console.
The
.Fa major
argument specifies the major version of the
debdrupUnsubmitted
Done
Inline Actions
.Fa major
- argument specifies the major version of the FreeBSD release that will remove
+ argument specifies the major version of the
+ .Fx
+ release that will remove
the deprecated functionality.
debdrup:
.Fx
release that will remove the deprecated functionality.
.Pp
In releases before
.Fa major
the deprecation notice states
.Do
debdrupUnsubmitted
Not Done
Inline Actions
the deprecation notice states
.Do
- Deprecated code (to be removed in FreeBSD
+ Deprecated code (to be removed in
+ .Fx
.Fa major ):
debdrup:
emasteAuthorUnsubmitted
Done
Inline Actions

I think this one probably should not change because it's literal console output.

emaste: I think this one probably should not change because it's literal console output.
Deprecated code (to be removed in FreeBSD
.Fa major Ns ):
emasteAuthorUnsubmitted
Done
Inline Actions

Added Ns here

emaste: Added Ns here
.Fa msg
.Dc .
In releases equal to and after
.Fa major
the notice states
.Do
Obsolete code will be removed soon:
.Fa msg
.Dc .
.Sh EXAMPLES
.Bd -literal -offset indent
void
sample_init(void)
{
/* Initializaiton code omitted. */
impUnsubmitted
Not Done
Inline Actions

11.x ABI compat isn't the headlining reason to retire, necessarily.
In the past this has been more like "Giant locked filesystem" or "Driver not migrated from old API" or simiar

imp: 11.x ABI compat isn't the headlining reason to retire, necessarily. In the past this has been…
emasteAuthorUnsubmitted
Done
Inline Actions

This was copied from an in-tree one, but indeed "Giant locked filesystem" is probably a good canonical example

emaste: This was copied from an in-tree one, but indeed "Giant locked filesystem" is probably a good…
gone_in(14, "Giant-locked filesystem");
}
impUnsubmitted
Done
Inline Actions

int

imp: int
int
example_driver_attach(struct example_driver_softc *sc)
{
impUnsubmitted
Not Done
Inline Actions

This message needs to be more along the lines of "This hardware no longer relevant to FreeBSD" or "This wifi driver doesn't support FooBar crypto" or some other minimum bar.

imp: This message needs to be more along the lines of "This hardware no longer relevant to FreeBSD"…
emasteAuthorUnsubmitted
Done
Inline Actions

Here's some examples from the tree, as they'd appear in dmesg:

Deprecated code (to be removed in FreeBSD 13): pccard removed, an doesn't support modern crypto
Deprecated code (to be removed in FreeBSD 13): pccard removed, an doesn't support modern crypto
Deprecated code (to be removed in FreeBSD 13): amr(4) driver
Deprecated code (to be removed in FreeBSD 13): FreeBSD 11.x ABI compat
Deprecated code (to be removed in FreeBSD 14): FreeBSD 12.x ABI compat
Deprecated code (to be removed in FreeBSD 13): drm2 drivers
Deprecated code (to be removed in FreeBSD 14): esp(4) driver
Deprecated code (to be removed in FreeBSD 14): PC Card to be removed
Deprecated code (to be removed in FreeBSD 14): mly(4) removed
prefixed with the device name/number for gone_in_dev of course

IMO something short like "ISA drivers", "obsolete hardware" etc. is what we should suggest

sys/dev/mly/mly.c
339: gone_in_dev(dev, 13, "");

sys/dev/twa/tw_osl_freebsd.c
416: gone_in_dev(dev, 13, "twa(4) removed");

sys/dev/iir/iir_pci.c
339: gone_in_dev(dev, 13, "iir(4) removed");

emaste: Here's some examples from the tree, as they'd appear in dmesg: Deprecated code (to be removed…
/* Attach code omitted. */
gone_in_dev(sc->dev, 14, "Giant-locked driver");
}
.Ed
.Sh HISTORY
The
impUnsubmitted
Done
Inline Actions

11

imp: 11
.Nm
functions first appeared in
.Fx 11 .