Page MenuHomeFreeBSD

ithread: Allow some ithreads to sleep
AcceptedPublic

Authored by andrew on Thu, Jan 2, 12:28 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jan 12, 7:41 PM
Unknown Object (File)
Sat, Jan 11, 10:28 AM
Unknown Object (File)
Sat, Jan 11, 6:44 AM
Unknown Object (File)
Sat, Jan 11, 6:08 AM
Unknown Object (File)
Sat, Jan 11, 4:33 AM
Unknown Object (File)
Fri, Jan 10, 8:25 AM
Unknown Object (File)
Wed, Jan 8, 7:43 PM
Unknown Object (File)
Fri, Jan 3, 10:50 PM
Subscribers

Details

Reviewers
trasz
markj
avg
jhb
Summary

Some ithreads need to hold a sleep mutex, e.g. when calling ACPI
methods. Allow ithreads to be marked as sleepable when this is known
to be safe.

Diff Detail

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

Event Timeline

markj added inline comments.
sys/kern/kern_intr.c
645

So is the idea here that different sleepable handlers can share an event and thus a thread?

I'm not really sure that we want to permit that - just because a handler is willing to sleep, doesn't mean that it's willing to wait for another handler to finish sleeping before executing the PRE_ITHREAD routine. I don't have a concrete problem to point at, but using a dedicated ithread seems less risky.

646

This line looks to be too long.

sys/sys/bus.h
285

2048 is free.

sys/kern/kern_intr.c
645

I was just ensuring we don't mix sleepable and non-sleepable ithreads. I can tighten the requirement to only one handler when INTR_SLEEPABLE is set

sys/kern/kern_intr.c
1200–1201

Perhaps this comment needs some update.

sys/sys/bus.h
285

And we probably should convert the values to hex, eventually.

sys/kern/kern_intr.c
645

That may well be sufficient, but for now sleepable handlers are unusual enough that having a dedicated thread is ok. It might just be paranoid; I'm hoping someone more knowledgeable than me can comment.

sys/kern/kern_intr.c
645

We could require INTR_SLEEPABLE to also include INTR_EXCL by making it an error if we have the former but not the latter.

sys/kern/kern_intr.c
645

We could require INTR_SLEEPABLE to include INTR_EXCL.

sys/kern/kern_intr.c
645

That sounds sensible on its surface. We never pool ithreads iirc, such that one thread would share multiple hardware sources...

Why not use a taskqueue? That is what every other driver that needs this functionality does.

In D48283#1103786, @jhb wrote:

Why not use a taskqueue? That is what every other driver that needs this functionality does.

There is an option to do that, but see commit be91b4797e2c8f3440f6fe3aac7e246886f9ebca.

  • Document INTR_SLEEPABLE
  • Change the INTR_SLEEPABLE value uo an earlier unused bit
  • Require INTR_EXCL to be set with INTR_SLEEPABLE

@jhb my personal / private interest in sleepable ithreads is "natural" support for interrupt controllers on slow buses.
Originally, it was assumed that an interrupt controller has fast access and they had.
But imagine something like an I/O expander on an I2C bus.
Let's assume that it supports detecting changes on its I/O inputs and also has a dedicated line to an upstream interrupt controller.
We can configure interrupt modes using I2C transactions and we can also query / mask / etc interrupt lines using I2C.
This is a real interrupt controller from all points of view and if we want to avoid polling (for potentially long time) we need to support sleeping / waiting while communicating with such controller.
At least, that's how I see it.

(Just a reminder that the man page date isn't filled out.)

This revision is now accepted and ready to land.Fri, Jan 17, 5:58 PM

I do have some old out-of-tree changes to rework how ithreads worked that did change the grouping, but even in that case we could accommodate this.