Re: [PATCH] media: tw686x: hold dev->lock during streamoff DMA disable

[Shuhao Fu <sfual@cse.ust.hk>]

Hi,

Below is the local KUnit/KCSAN setup I used to reproduce the TW686x
warning.

I do not have a TW686x board or a QEMU device model for this chip, so
this is not a hardware-backed userspace repro. This setup is only my
best-effort local reference for the race.

To stay as close as possible to the real userspace trigger, I moved the
writer side up to the same internal callback reached by
VIDIOC_STREAMOFF: tw686x_stop_streaming(). The reader side stayed the
real tw686x_dma_delay() timer callback, which consumes and clears
pending_dma_en / pending_dma_cmd under dev->lock.

Locally, I used the existing kernel/kcsan/tw686x_dma_race.kunitconfig
setup, but rebuilt it with stricter KCSAN settings:

- CONFIG_KCSAN_STRICT=y
- CONFIG_KCSAN_INTERRUPT_WATCHER=y
- CONFIG_KCSAN_REPORT_ONCE_IN_MS=0
- CONFIG_KCSAN_SKIP_WATCH=100
- CONFIG_KCSAN_SKIP_WATCH_RANDOMIZE=n
- CONFIG_KCSAN_UDELAY_TASK=200
- CONFIG_KCSAN_UDELAY_INTERRUPT=100
- CONFIG_KCSAN_DELAY_RANDOMIZE=n

The build command was:

  ./tools/testing/kunit/kunit.py build \
    --arch=x86_64 \
    --kunitconfig=kernel/kcsan/tw686x_dma_race.kunitconfig \
    --build_dir=../out-tw686x-kcsan-kunit-strict \
    --make_options CC=clang-20 \
    --make_options LD=ld.bfd \
    --make_options AR=llvm-ar-20 \
    --make_options NM=llvm-nm-20 \
    --make_options OBJCOPY=llvm-objcopy-20 \
    --make_options READELF=llvm-readelf-20 \
    --make_options LLVM_IAS=1 \
    --jobs 8

Then I booted only the focused TW686x disable-vs-delay test under QEMU:

  timeout 120 qemu-system-x86_64 \
    -smp 8 \
    -m 2048 \
    -kernel out-tw686x-kcsan-kunit-strict/arch/x86/boot/bzImage \
    -append 'kunit.filter_glob=kcsan.test_tw686x_pending_dma_disable_vs_delay kunit.enable=1 console=ttyS0 kunit_shutdown=reboot' \
    -nographic \
    -no-reboot

The KUnit harness uses a fake tw686x_dev plus a minimal fake
tw686x_video_channel and vb2_queue, so the writer side can call the
real tw686x_stop_streaming() path instead of jumping straight into
tw686x_disable_channel(). The internal call chain is:

- writer side:
  test_kernel_tw686x_stop_streaming()
  -> test_tw686x_kunit_stop_streaming()
  -> tw686x_stop_streaming()
  -> tw686x_disable_channel()

- reader side:
  test_kernel_tw686x_dma_delay()
  -> test_tw686x_kunit_dma_delay()
  -> tw686x_dma_delay()

With that setup I got repeated KCSAN reports of:

  BUG: KCSAN: data-race in test_tw686x_kunit_dma_delay / tw686x_disable_channel

The first clean hit in my local log was:

  read to 0xffff8f55021d7910 of 4 bytes by interrupt on cpu 5:
   test_tw686x_kunit_dma_delay+0x3c/0x110
   test_kernel_tw686x_dma_delay+0x29/0x40
   ...
   tw686x_disable_channel+0x1e8/0x200
   tw686x_stop_streaming+0x81/0x1e0
   test_tw686x_kunit_stop_streaming+0x22/0x30
   test_kernel_tw686x_stop_streaming+0x29/0x40

  write to 0xffff8f55021d7910 of 4 bytes by task 83 on cpu 5:
   tw686x_disable_channel+0x1e8/0x200
   tw686x_stop_streaming+0x81/0x1e0
   test_tw686x_kunit_stop_streaming+0x22/0x30
   test_kernel_tw686x_stop_streaming+0x29/0x40

I then saw the same pair again later in the same run, still with
tw686x_stop_streaming() in the writer stack and
test_tw686x_kunit_dma_delay() on the reader side.

Again, this KUnit result is a local reference and best-effort proof of
the overlap, not a claim of a full hardware-backed userspace repro.

Thanks,
Shuhao

On Thu, Apr 23, 2026 at 09:05:42PM +0800, Shuhao Fu wrote:
> tw686x_dma_delay() consumes and clears dev->pending_dma_en /
> dev->pending_dma_cmd under dev->lock before replaying that deferred DMA
> image to the hardware registers. tw686x_disable_channel() updates the
> same fields. Most call sites serialize tw686x_disable_channel() with
> dev->lock, but tw686x_stop_streaming() is the exception: it checks
> dev->pci_dev under dev->lock, drops the lock, and then calls
> tw686x_disable_channel().
> 
> If dma_delay_timer fires in that window, the timer path can race the
> streamoff disable path and consume stale or partially updated pending
> DMA state. This can leave deferred DMA programming out of sync with the
> requested stream stop, so STREAMOFF may not immediately reflect the
> intended channel-disable state.
> 
> Fix with holding dev->lock across the stop-side pci_dev check and
> tw686x_disable_channel() call, so VIDIOC_STREAMOFF follows the same
> locking contract as the other channel enable/disable paths.