From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D58BEC10F27 for ; Sun, 8 Mar 2020 23:13:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AAEFB21741 for ; Sun, 8 Mar 2020 23:13:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726514AbgCHXNA (ORCPT ); Sun, 8 Mar 2020 19:13:00 -0400 Received: from mail105.syd.optusnet.com.au ([211.29.132.249]:46679 "EHLO mail105.syd.optusnet.com.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726354AbgCHXNA (ORCPT ); Sun, 8 Mar 2020 19:13:00 -0400 Received: from dread.disaster.area (pa49-195-202-68.pa.nsw.optusnet.com.au [49.195.202.68]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 7F5A43A1EC1; Mon, 9 Mar 2020 10:12:54 +1100 (AEDT) Received: from dave by dread.disaster.area with local (Exim 4.92.3) (envelope-from ) id 1jB56L-0004tJ-SX; Mon, 09 Mar 2020 10:12:53 +1100 Date: Mon, 9 Mar 2020 10:12:53 +1100 From: Dave Chinner To: Eric Biggers Cc: linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com Subject: Re: [PATCH] fs/direct-io.c: avoid workqueue allocation race Message-ID: <20200308231253.GN10776@dread.disaster.area> References: <20200308055221.1088089-1-ebiggers@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200308055221.1088089-1-ebiggers@kernel.org> User-Agent: Mutt/1.10.1 (2018-07-13) X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.3 cv=X6os11be c=1 sm=1 tr=0 a=mqTaRPt+QsUAtUurwE173Q==:117 a=mqTaRPt+QsUAtUurwE173Q==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=kj9zAlcOel0A:10 a=SS2py6AdgQ4A:10 a=1XWaLZrsAAAA:8 a=7-415B0cAAAA:8 a=sjk0pT0bszA0zqNz9kcA:9 a=CjuIK1q_8ugA:10 a=biEYGPWJfzWAr4FL6Ov7:22 Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On Sat, Mar 07, 2020 at 09:52:21PM -0800, Eric Biggers wrote: > From: Eric Biggers > > When a thread loses the workqueue allocation race in > sb_init_dio_done_wq(), lockdep reports that the call to > destroy_workqueue() can deadlock waiting for work to complete. This is > a false positive since the workqueue is empty. But we shouldn't simply > skip the lockdep check for empty workqueues for everyone. Why not? If the wq is empty, it can't deadlock, so this is a problem with the workqueue lockdep annotations, not a problem with code that is destroying an empty workqueue. > Just avoid this issue by using a mutex to serialize the workqueue > allocation. We still keep the preliminary check for ->s_dio_done_wq, so > this doesn't affect direct I/O performance. > > Also fix the preliminary check for ->s_dio_done_wq to use READ_ONCE(), > since it's a data race. (That part wasn't actually found by syzbot yet, > but it could be detected by KCSAN in the future.) > > Note: the lockdep false positive could alternatively be fixed by > introducing a new function like "destroy_unused_workqueue()" to the > workqueue API as previously suggested. But I think it makes sense to > avoid the double allocation anyway. Fix the infrastructure, don't work around it be placing constraints on how the callers can use the infrastructure to work around problems internal to the infrastructure. Cheers, Dave. -- Dave Chinner david@fromorbit.com