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.5 required=3.0 tests=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 73A2EC2BA2B for ; Wed, 15 Apr 2020 06:16:53 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3B1B8206D9 for ; Wed, 15 Apr 2020 06:16:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3B1B8206D9 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B95278E0007; Wed, 15 Apr 2020 02:16:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B1FD08E0001; Wed, 15 Apr 2020 02:16:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9E6418E0007; Wed, 15 Apr 2020 02:16:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0056.hostedemail.com [216.40.44.56]) by kanga.kvack.org (Postfix) with ESMTP id 8369A8E0001 for ; Wed, 15 Apr 2020 02:16:52 -0400 (EDT) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 44CF1582B for ; Wed, 15 Apr 2020 06:16:52 +0000 (UTC) X-FDA: 76709081064.01.seat93_3bf3bee27a21b X-HE-Tag: seat93_3bf3bee27a21b X-Filterd-Recvd-Size: 5192 Received: from mail-pj1-f66.google.com (mail-pj1-f66.google.com [209.85.216.66]) by imf18.hostedemail.com (Postfix) with ESMTP for ; Wed, 15 Apr 2020 06:16:51 +0000 (UTC) Received: by mail-pj1-f66.google.com with SMTP id o1so5414692pjs.4 for ; Tue, 14 Apr 2020 23:16:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=kKwt+k2pDUu9qpX8GZzWlBWNHQkyl61044lYkr3xgoo=; b=n4GEAmuicLjgECPPAoAcMvDmv6eiUd+jefNCD/5VLNUlVhMG58wzx9/BCAtDr/Fe/g YFlLssXFjZMoBf7beAu/CcaS/p3hZ3+gBc/IeZgpxDTXb/t0TajtunCmj3RBisVytmjF Htp3CR1Fb1yU3FJo6DLBhzbcWt8bWlLE11W7QPoIMgx17GodgJEWw865rIj6/egRWyQv 1oty72Swu3gjNT1zAquxZYLQTN6srUmoRMqK9latFAmKDSM9oXBKV4JuGmJW/95Tp4iY RhxhKFfX1o9oQ3edt/ZBOuYyNbOFdKwmpN/Pf5ptVxUACvHFdRsB2Bi4KQdBBHTPKnUM HnLw== X-Gm-Message-State: AGi0PubWGm+KtSa4hlNDNpSfy56UNvqAhk0dvAlQdu29uLkCP2OmrwfG I8gv+jXCuHLlk8LuSVMHGbY= X-Google-Smtp-Source: APiQypLUnBYTt2JPAvzaO1zJS12kV1LMzAoWxzi3yM07dJiAaH0luQJiHmQw3RCIjIBzSRfPCV0Vfw== X-Received: by 2002:a17:902:7616:: with SMTP id k22mr2578548pll.39.1586931411005; Tue, 14 Apr 2020 23:16:51 -0700 (PDT) Received: from 42.do-not-panic.com (42.do-not-panic.com. [157.230.128.187]) by smtp.gmail.com with ESMTPSA id z16sm3078792pfa.3.2020.04.14.23.16.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Apr 2020 23:16:49 -0700 (PDT) Received: by 42.do-not-panic.com (Postfix, from userid 1000) id 2980940277; Wed, 15 Apr 2020 06:16:49 +0000 (UTC) Date: Wed, 15 Apr 2020 06:16:49 +0000 From: Luis Chamberlain To: Christoph Hellwig Cc: axboe@kernel.dk, viro@zeniv.linux.org.uk, bvanassche@acm.org, gregkh@linuxfoundation.org, rostedt@goodmis.org, mingo@redhat.com, jack@suse.cz, ming.lei@redhat.com, nstange@suse.de, akpm@linux-foundation.org, mhocko@suse.com, yukuai3@huawei.com, linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Omar Sandoval , Hannes Reinecke , Michal Hocko Subject: Re: [PATCH 3/5] blktrace: refcount the request_queue during ioctl Message-ID: <20200415061649.GS11244@42.do-not-panic.com> References: <20200414041902.16769-1-mcgrof@kernel.org> <20200414041902.16769-4-mcgrof@kernel.org> <20200414154044.GB25765@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200414154044.GB25765@infradead.org> User-Agent: Mutt/1.10.1 (2018-07-13) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Apr 14, 2020 at 08:40:44AM -0700, Christoph Hellwig wrote: > On Tue, Apr 14, 2020 at 04:19:00AM +0000, Luis Chamberlain wrote: > > Ensure that the request_queue is refcounted during its full > > ioctl cycle. This avoids possible races against removal, given > > blk_get_queue() also checks to ensure the queue is not dying. > > > > This small race is possible if you defer removal of the request_queue > > and userspace fires off an ioctl for the device in the meantime. > > Hmm, where exactly does the race come in so that it can only happen > after where you take the reference, but not before it? I'm probably > missing something, but that just means it needs to be explained a little > better :) >From the trace on patch 2/5: BLKTRACE_SETUP(loop0) #2 [ 13.933961] == blk_trace_ioctl(2, BLKTRACESETUP) start [ 13.936758] === do_blk_trace_setup(2) start [ 13.938944] === do_blk_trace_setup(2) creating directory [ 13.941029] === do_blk_trace_setup(2) using what debugfs_lookup() gave ---> From LOOP_CTL_DEL(loop0) #2 [ 13.971046] === blk_trace_cleanup(7) end [ 13.973175] == __blk_trace_remove(7) end [ 13.975352] == blk_trace_shutdown(7) end [ 13.977415] = __blk_release_queue(7) calling blk_mq_debugfs_unregister() [ 13.980645] ==== blk_mq_debugfs_unregister(7) begin [ 13.980696] ==== blk_mq_debugfs_unregister(7) debugfs_remove_recursive(q->debugfs_dir) [ 13.983118] ==== blk_mq_debugfs_unregister(7) end q->debugfs_dir is NULL [ 13.986945] = __blk_release_queue(7) blk_mq_debugfs_unregister() end [ 13.993155] = __blk_release_queue(7) end ---> From BLKTRACE_SETUP(loop0) #2 [ 13.995928] === do_blk_trace_setup(2) end with ret: 0 [ 13.997623] == blk_trace_ioctl(2, BLKTRACESETUP) end The BLKTRACESETUP above works on request_queue which later LOOP_CTL_DEL races on and sweeps the debugfs dir underneath us. If you use this commit alone though, this doesn't fix the race issue however, and that's because of both still the debugfs_lookup() use and that we're still using asynchronous removal at this point. refcounting will just ensure we don't take the request_queue underneath our noses. Should I just add this to the commit log? Luis