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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 22 Oct 2018 13:54:06 -0700
From:   Bart Van Assche <bvanassche@....org>
To:     Johannes Berg <johannes@...solutions.net>,
        Tejun Heo <tj@...nel.org>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Christoph Hellwig <hch@....de>,
        Sagi Grimberg <sagi@...mberg.me>,
        "linux-nvme @ lists . infradead . org" 
        <linux-nvme@...ts.infradead.org>
Subject: Re: [PATCH] Revert "workqueue: re-add lockdep dependencies for
 flushing"

On Mon, 2018-10-22 at 22:28 +0200, Johannes Berg wrote:
> The lockdep report even more or less tells you what's going on. Perhaps
> we need to find a way to make lockdep not print "lock()" but "start()"
> or "flush()" for work items ... but if you read it this way, you see:
> 
> CPU0			CPU1
> 
> lock(i_mutex_key)
> 			start(dio->complete_work)
> 			lock(i_mutex_key)
> flush(wq dio/...)
> 
> which is *clearly* 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 "CPU0" is code called by do_blockdev_direct_IO().
>From the body of that function:

			/* will be released by direct_io_worker */
			inode_lock(inode);

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.