Page MenuHomeFreeBSD

Make device_busy/unbusy work w/o Giant held
Needs ReviewPublic

Authored by imp on Sep 1 2020, 5:22 PM.

Details

Reviewers
jhb
hselasky
Summary

The vast majority of the busy/unbusy users in the tree don't acquire Giant
before calling device_busy/unbusy. However, if multiple threads are opening a
file, say, that causes the device to busy/unbusy, then we can race to the root
marking things busy. Create a new device_busy_locked and device_unbusy_locked
that are the current implemntations of device_busy and device_unbusy. Make
device_busy and unbusy acquire Giant before calling the _locked versrions. Since
we never sleep in the busy/unbusy path, Giant's single threaded semantics
suffice to keep this safe.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 41364
Build 38253: arc lint + arc unit

Event Timeline

imp requested review of this revision.Sep 1 2020, 5:22 PM
imp created this revision.
hselasky added a subscriber: hselasky.
hselasky added inline comments.
sys/kern/subr_bus.c
144

Use uint32_t as below?

This revision is now accepted and ready to land.Sep 2 2020, 3:38 PM

I'd rather axe device_busy and instead require drivers to use device_quiesce.

However, this does mean that bus drivers need to use a bus_generic_quiesce that recurses to preserve the "device_busy of the parent" semantic.

In D26284#584505, @jhb wrote:

I'd rather axe device_busy and instead require drivers to use device_quiesce.

However, this does mean that bus drivers need to use a bus_generic_quiesce that recurses to preserve the "device_busy of the parent" semantic.

Good point. I'll rework to eliminate this where it's now not needed and see if we can migrate the rest to quiesce.

Rebase and take out the fdc tweaks. Those aren't needed anymore.

This revision now requires review to proceed.Sep 4 2021, 4:34 AM

There is a spelling mistake in the description:

versrions -> versions

sys/kern/subr_bus.c
3039

Shouldn't you use a refcount function to read the busy value? I mean atomic_xxx