linux-kernel - Re: [PATCH 3/3] kernel/workqueue: Suppress a false positive lockdep complaint

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:   Thu, 25 Oct 2018 17:34:50 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Bart Van Assche <bvanassche@....org>, 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>,
        "tytso@....edu" <tytso@....edu>
Subject: Re: [PATCH 3/3] kernel/workqueue: Suppress a false positive lockdep
 complaint

On Thu, 2018-10-25 at 15:05 +0000, Bart Van Assche wrote:

> @@ -2889,7 +2893,7 @@ static bool start_flush_work(struct work_struct *work, struct wq_barrier *barr,
>  	 * workqueues the deadlock happens when the rescuer stalls, blocking
>  	 * forward progress.
>  	 */
> -	if (!from_cancel &&
> +	if (!from_cancel && (pwq->wq->flags & __WQ_HAS_BEEN_USED) &&
>  	    (pwq->wq->saved_max_active == 1 || pwq->wq->rescuer)) {
>  		lock_acquire_exclusive(&pwq->wq->lockdep_map, 0, 0, NULL,
>  				       _THIS_IP_);

This also doesn't seem right to me. You shouldn't really care whether or
not the workqueue has been used at this point, lockdep also doesn't do
this for locks.

Any dependency you cause at some point can - at a future time - be taken
into account when checking dependency cycles. Removing one arbitrarily
just because you haven't actually executed anything *yet* just removes
knowledge from lockdep. In the general case, this isn't right. Just
because you haven't executd anything here doesn't mean that it's
*impossible* to have executed something, right?

(Also, still trying to understand patch 2)

johannes