Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <0ad532b2-df5f-331a-ae7f-21460fc62fe2@I-love.SAKURA.ne.jp>
Date:   Fri, 29 Jul 2022 08:18:32 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH] workqueue: don't skip lockdep wq dependency in
 cancel_work_sync()
Content-Language: en-US
To:     Tejun Heo <tj@kernel.org>
Cc:     Lai Jiangshan <jiangshanlai@gmail.com>,
        Johannes Berg <johannes.berg@intel.com>,
        Hillf Danton <hdanton@sina.com>,
        LKML <linux-kernel@vger.kernel.org>
References: <21b9c1ac-64b7-7f4b-1e62-bf2f021fffcd@I-love.SAKURA.ne.jp>
 <YuK78Jiy12BJG/Tp@slm.duckdns.org>
From:   Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
In-Reply-To: <YuK78Jiy12BJG/Tp@slm.duckdns.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2022/07/29 1:40, Tejun Heo wrote:
> Tetsuo, you gotta explain why this is okay w.r.t. the spurious warnings that
> the above commit addressed. You can't just state that there are cases which
> are missed and then revert it.

There are four commits related to this change.

  commit 87915adc3f0acdf03c776df42e308e5a155c19af
  Author: Johannes Berg <johannes.berg@intel.com>
  Date:   Wed Aug 22 11:49:04 2018 +0200

      workqueue: re-add lockdep dependencies for flushing

  commit d6e89786bed977f37f55ffca11e563f6d2b1e3b5
  Author: Johannes Berg <johannes.berg@intel.com>
  Date:   Wed Aug 22 11:49:03 2018 +0200

      workqueue: skip lockdep wq dependency in cancel_work_sync()

  commit fd1a5b04dfb899f84ddeb8acdaea6b98283df1e5
  Author: Byungchul Park <byungchul.park@lge.com>
  Date:   Wed Oct 25 17:56:04 2017 +0900

      workqueue: Remove now redundant lock acquisitions wrt. workqueue flushes

  commit 0976dfc1d0cd80a4e9dfaf87bd8744612bde475a
  Author: Stephen Boyd <sboyd@codeaurora.org>
  Date:   Fri Apr 20 17:28:50 2012 -0700

      workqueue: Catch more locking problems with flush_work()

. Commit 87915adc3f0acdf0 ("workqueue: re-add lockdep dependencies for
flushing") saying

    These were removed after cross-release partially caught these
    problems, but now cross-release was reverted anyway. IMHO the
    removal was erroneous anyway though, since lockdep should be
    able to catch potential problems, not just actual ones, and
    cross-release would only have caught the problem when actually
    invoking wait_for_completion().

is the answer, commit 0976dfc1d0cd80a4 ("workqueue: Catch more locking
problems with flush_work()") saying

    Add a lockdep hint by acquiring and releasing the work item
    lockdep_map in flush_work() so that we always catch this
    potential deadlock scenario.

is what this patch restores.