Page MenuHomeFreeBSD

LinuxKPI: Reimplement irq_work queue on top of fast taskqueue
ClosedPublic

Authored by wulf on Nov 10 2020, 11:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 23, 12:15 AM
Unknown Object (File)
Sun, Dec 22, 10:08 AM
Unknown Object (File)
Fri, Dec 13, 5:30 AM
Unknown Object (File)
Mon, Dec 9, 4:42 AM
Unknown Object (File)
Nov 23 2024, 1:23 AM
Unknown Object (File)
Nov 22 2024, 2:17 AM
Unknown Object (File)
Nov 19 2024, 3:08 AM
Unknown Object (File)
Nov 17 2024, 4:05 PM

Details

Summary

Linux's irq_work queue was created for asynchronous execution of code from contexts where spin_lock's are not available like "hardware interrupt context". FreeBSD's fast taskqueues was created for the same purposes.

Drm-kmod 5.4 uses irq_work_queue() at least in one place to schedule execution of task/work from the critical section that triggers following INVARIANTS-induced panic:

panic: acquiring blockable sleep lock with spinlock or critical section held (sleep mutex) linuxkpi_short_wq @ /usr/src/sys/kern/subr_taskqueue.c:281
cpuid = 6
time = 1605048416
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe006b538c90
vpanic() at vpanic+0x182/frame 0xfffffe006b538ce0
panic() at panic+0x43/frame 0xfffffe006b538d40
witness_checkorder() at witness_checkorder+0xf3e/frame 0xfffffe006b538f00
__mtx_lock_flags() at __mtx_lock_flags+0x94/frame 0xfffffe006b538f50
taskqueue_enqueue() at taskqueue_enqueue+0x42/frame 0xfffffe006b538f70
linux_queue_work_on() at linux_queue_work_on+0xe9/frame 0xfffffe006b538fb0
irq_work_queue() at irq_work_queue+0x21/frame 0xfffffe006b538fd0
semaphore_notify() at semaphore_notify+0xb2/frame 0xfffffe006b539020
__i915_sw_fence_notify() at __i915_sw_fence_notify+0x2e/frame 0xfffffe006b539050
__i915_sw_fence_complete() at __i915_sw_fence_complete+0x63/frame 0xfffffe006b539080
i915_sw_fence_complete() at i915_sw_fence_complete+0x8e/frame 0xfffffe006b5390c0
dma_i915_sw_fence_wake() at dma_i915_sw_fence_wake+0x4f/frame 0xfffffe006b539100
dma_fence_signal_locked() at dma_fence_signal_locked+0x105/frame 0xfffffe006b539180
dma_fence_signal() at dma_fence_signal+0x72/frame 0xfffffe006b5391c0
dma_fence_is_signaled() at dma_fence_is_signaled+0x80/frame 0xfffffe006b539200
dma_resv_add_shared_fence() at dma_resv_add_shared_fence+0xb3/frame 0xfffffe006b539270
i915_vma_move_to_active() at i915_vma_move_to_active+0x18a/frame 0xfffffe006b5392b0
eb_move_to_gpu() at eb_move_to_gpu+0x3ad/frame 0xfffffe006b539320
eb_submit() at eb_submit+0x15/frame 0xfffffe006b539350
i915_gem_do_execbuffer() at i915_gem_do_execbuffer+0x7d4/frame 0xfffffe006b539570
i915_gem_execbuffer2_ioctl() at i915_gem_execbuffer2_ioctl+0x1c1/frame 0xfffffe006b539600
drm_ioctl_kernel() at drm_ioctl_kernel+0xd9/frame 0xfffffe006b539670
drm_ioctl() at drm_ioctl+0x5cd/frame 0xfffffe006b539820
linux_file_ioctl() at linux_file_ioctl+0x323/frame 0xfffffe006b539880
kern_ioctl() at kern_ioctl+0x1f4/frame 0xfffffe006b5398f0
sys_ioctl() at sys_ioctl+0x12a/frame 0xfffffe006b5399c0
amd64_syscall() at amd64_syscall+0x121/frame 0xfffffe006b539af0
fast_syscall_common() at fast_syscall_common+0xf8/frame 0xfffffe006b539af0
--- syscall (54, FreeBSD ELF64, sys_ioctl), rip = 0x800a6f09a, rsp = 0x7fffffffe588, rbp = 0x7fffffffe640 ---
KDB: enter: panic

Here, the dma_resv_add_shared_fence() performs a critical_enter() and following call of schedule_work() from semaphore_notify() triggers 'acquiring blockable sleep lock with spinlock or critical section held' panic.

Switching irq_work implementation to fast taskqueue fixes the panic for me.

Other report with the similar bug: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=247166

Test Plan

Run for the some time with INVARIANTS kernel option enabled.

Diff Detail

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

Event Timeline

wulf requested review of this revision.Nov 10 2020, 11:55 PM

Don't forget to MFC and bump the __FreeBSD_version in sys/sys/param.h

Also check that the irq_work.h header file compiles standalone.

This revision is now accepted and ready to land.Nov 11 2020, 7:17 AM

Why do we need a __FreeBSD_version bump? Especially if it is also slated for MFC?

It causes less binary breakage :-)

Why do we need a __FreeBSD_version bump? Especially if it is also slated for MFC?

Current drm-kmod can not be compiled with this patch applied as it does direct access to func member of struct irq_work. Now it is moved from sub-structure of struct work_struct to struct irq_work itself to match Linux.

One #ifdef __linux__ should be removed from drm-kmod based on __FreeBSD_version to make it compilable again.

If it requires updates to drm-current-kmod and drm-devel-kmod, as well as might cause binary breakage, then it shouldn't be MFCd. We have enough trouble already with drm-kmod breaking between minor releases and in stable.

If it requires updates to drm-current-kmod and drm-devel-kmod, as well as might cause binary breakage, then it shouldn't be MFCd. We have enough trouble already with drm-kmod breaking between minor releases and in stable.

Currently irq_work does not have own KBI, so nothing to break here. I have more worries about KPI change. Anyway, I'll try to test it before possible MFC.

Add static irq_work declarator used in drm-kmod pull-request https://github.com/freebsd/drm-kmod/pull/39/files
No functional changes.

This revision now requires review to proceed.Nov 15 2020, 11:48 AM

@hselasky , @manu
https://github.com/freebsd/drm-kmod/pull/39/files contains some parts which can be moved to LKPI to reduce drm source patching: "SYSINIT initialization of current for irq_work thread" and "asynchronous kfree()" but I do not have strong opinion about their best place: i915kms.ko or linuxkpi.ko? If latter, I can prepare patches.

@wulf: Please prepare the patches. Having stuff inside the LinuxKPI makes life easier :-)

Changes:

  • Add initialization of current in irq_work thread.
  • Add async free() for use in critical sections.
  • Imported linux/llist.h implementation from OpenBSD as dependency for async free().

https://github.com/freebsd/drm-kmod/pull/39/files is updated.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 17 2021, 11:48 AM
This revision was automatically updated to reflect the committed changes.
sys/compat/linuxkpi/common/include/linux/llist.h
1

Could you add a proper license text here?