Re: [PATCH] Revert "workqueue: re-add lockdep dependencies for flushing"

From: Bart Van Assche
Date: Mon Oct 22 2018 - 16:54:12 EST


On Mon, 2018-10-22 at 22:28 +-0200, Johannes Berg wrote:
+AD4 The lockdep report even more or less tells you what's going on. Perhaps
+AD4 we need to find a way to make lockdep not print +ACI-lock()+ACI but +ACI-start()+ACI
+AD4 or +ACI-flush()+ACI for work items ... but if you read it this way, you see:
+AD4
+AD4 CPU0 CPU1
+AD4
+AD4 lock(i+AF8-mutex+AF8-key)
+AD4 start(dio-+AD4-complete+AF8-work)
+AD4 lock(i+AF8-mutex+AF8-key)
+AD4 flush(wq dio/...)
+AD4
+AD4 which is +ACo-clearly+ACo a problem.

Your patch made lockdep report that the direct I/O code deadlocks although it
clearly doesn't deadlock. So where do you think the bug is? In the direct I/O
code or in your patch?

The code in the column with label +ACI-CPU0+ACI is code called by do+AF8-blockdev+AF8-direct+AF8-IO().
>From the body of that function:

/+ACo will be released by direct+AF8-io+AF8-worker +ACo-/
inode+AF8-lock(inode)+ADs

I think that is sufficient evidence that the direct I/O code is fine and that
your patch caused lockdep to produce an incorrect deadlock report.

Bart.