Page MenuHomeFreeBSD

destroy_dev_sched*: Don't hold Giant for all deferred destroy_dev.
ClosedPublic

Authored by jhb on Apr 14 2022, 5:21 PM.
Tags
None
Referenced Files
F102907417: D34915.diff
Mon, Nov 18, 2:37 PM
Unknown Object (File)
Sun, Oct 20, 8:21 PM
Unknown Object (File)
Oct 9 2024, 3:06 AM
Unknown Object (File)
Oct 2 2024, 5:12 AM
Unknown Object (File)
Oct 2 2024, 4:49 AM
Unknown Object (File)
Sep 19 2024, 3:25 PM
Unknown Object (File)
Sep 17 2024, 3:35 AM
Unknown Object (File)
Sep 6 2024, 3:25 AM
Subscribers

Details

Summary

Rather than using taskqueue_swi_giant which holds Giant for all
deferred destroy_dev calls, create a separate queue for destroyed
devices with D_NEEDGIANT set in the corresponding cdevsw. The task
for this queue holds Giant whild destroying deferred devices while the
task for the default queue does not hold Giant.

In addition, switch to taskqueue_thread for destroy_dev_sched.
Deferred destroy_dev requests don't need to run at an SWI priority.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 45169
Build 42057: arc lint + arc unit

Event Timeline

jhb requested review of this revision.Apr 14 2022, 5:21 PM
This revision is now accepted and ready to land.Apr 14 2022, 5:28 PM
sys/kern/kern_conf.c
1454

Does Giant need to be held across destroy_devl() too? That function invokes d_purge (seems to be mostly unused) and cdevpriv destructors (not sure if D_NEEDGIANT implies that they're synchronized by Giant too).

sys/kern/kern_conf.c
1454

Oh, hmm. Looks like d_purge isn't handled by the gianttrick hack, and yeah, cdevpriv probably needs Giant as well.

Hold Giant over destroy_devl as well.

This revision now requires review to proceed.Apr 14 2022, 5:58 PM
sys/kern/kern_conf.c
1452

Sigh, so this is a LOR with devmtx in dev_lock I think.

I would need to push Giant down into destroy_devl() instead along with the previous version of this patch I think.

sys/kern/kern_conf.c
1452

That doesn't work out so well because dev_lock is still held when d_purge is invoked.

I guess fixing this for real would require separate global lists and tasks.

jhb retitled this revision from destroy_dev_sched*: Don't hold Giant for deferred destroy_dev. to destroy_dev_sched*: Don't hold Giant for all deferred destroy_dev..Apr 14 2022, 6:34 PM
jhb edited the summary of this revision. (Show Details)
  • Switch to a separate queue + task.
This revision is now accepted and ready to land.Apr 18 2022, 1:15 PM