Page MenuHomeFreeBSD

Drain grouptaskqueue of the gtask before detaching it.
ClosedPublic

Authored by shurd on Oct 23 2018, 8:04 PM.

Details

Summary

taskqgroup_detach() would remove the task even if it was
running or enqueued, which could lead to panics (see D17404). With
this change, taskqgroup_detach() drains the task and sets a new flag
which prevents the task from being scheduled again.

I've added grouptask_block() and grouptask_unblock() to allow control
over the flag from other locations as well.

Test Plan

Test with e1000 devices, and have Intel test with the initial
panicing testcase

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

shurd created this revision.Oct 23 2018, 8:04 PM
dhw added a subscriber: dhw.Oct 24 2018, 1:52 PM

I tested this, as my laptop had panicked after updating from r339583 to r339639. I first updated sources to r339681, then applied the patch (via 'svn patch'); did the normal update, rebooted, and the em0 NIC worked just fine -- certainly no panic.

For reference:
FreeBSD 13.0-CURRENT #148 r339681M/339681: Wed Oct 24 05:48:37 PDT 2018 root@localhost:/common/S4/obj/usr/src/amd64.amd64/sys/CANARY amd64 1300000

@shurd Thanks! That makes more sense than my version. I can't reproduce the ixl panic on my machine but Iets wait for an official verification by Jeff's team.

@dhw Thank you for testing the patch!

shurd updated this revision to Diff 49570.Oct 24 2018, 5:18 PM

Add gtaskqueue_drain_locked() to hold common code for gtaskqueue_drain()
and grouptask_block().

shurd updated this revision to Diff 49571.Oct 24 2018, 5:29 PM

Add missing semi-colon.

dhw added a comment.Oct 25 2018, 11:21 AM

Given the changes, this morning, I:

  • Reverted the previous changes to sys/kern/subr_gtaskqueue.c and sys/sys/gtaskqueue.h
  • Updated sources from r339681to r339706
  • Applied the new diff (again, via 'svn patch')
  • Performed a normal source-based update

No panics or other issues; em0 NIC works.

We cannot reproduce the panic that was resolved with https://reviews.freebsd.org/D17404, so it looks okay to me.

This revision is now accepted and ready to land.Oct 26 2018, 12:49 PM
This revision was automatically updated to reflect the committed changes.