From patchwork Wed Feb 7 20:07:24 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tejun Heo X-Patchwork-Id: 10205933 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 5E4976020F for ; Wed, 7 Feb 2018 20:07:34 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4A88828B72 for ; Wed, 7 Feb 2018 20:07:34 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3C03028BEE; Wed, 7 Feb 2018 20:07:34 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BDD1228B72 for ; Wed, 7 Feb 2018 20:07:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754127AbeBGUHd (ORCPT ); Wed, 7 Feb 2018 15:07:33 -0500 Received: from mail-qk0-f196.google.com ([209.85.220.196]:42514 "EHLO mail-qk0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754087AbeBGUHc (ORCPT ); Wed, 7 Feb 2018 15:07:32 -0500 Received: by mail-qk0-f196.google.com with SMTP id f68so2753559qke.9 for ; Wed, 07 Feb 2018 12:07:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=po3t/e344MDZJiZzoz8g+vj9xDdzSLiWfe/uHJOMYec=; b=cJyEtzrtOL8pG8HWxWCII0W55z4uqyWEGh8XvKorVALb6J1EATApetR/L+sNa6jpti uKkjzjAm6B+eehBVu7Kq6ZSR9XpEksl2gxupFx70tf88pu1gncTPbd45onsE4MTISh+D E7mTPbrCtAlOvZgzXDeAz+IA8qhUidPrWxRJEU5v4jpyvbAzYTLrDCba2xVDSPwrqkFH JaKgtwG2vOjOgxXyXbF5wXYRolWxwWmrOjrSAzKwLjBekqQcJLBIIypUueKnyusFNbtI 16zl+bg92T5hC09Y4VjpowCuFTXcLefVzH9RIqgz0xVTaQL7tQ4O5txElchP85Wj+keP Kdrg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=po3t/e344MDZJiZzoz8g+vj9xDdzSLiWfe/uHJOMYec=; b=WO+azVwY5nNxeAfT1Lm878tHNMieBJSKkuoQtQ39euuzzoduKG6JB+K6wXCG/lBe5Q lPIamFIi0mAag1yBihHX0Wf2g1OboLiYkSXzM5MOjeB6cfZ7nzVlI+1WSgTebdLB9rH9 StpuqxnU5Bt1eveZeAyp5tLPkWH9p6Kh6uI4GlbalQZGBnr3es2IXEjF+t8n3b7sHKK/ K2AL/+FAwK4NMQZZWx++u7PuzwavUmDI5fYAhNsrmk8EZ8xzoXUTNxRLa6P0ebm68qYf kQ8pw6Hh2fQ8m3eJra+ujkXQiSSideaWDkSP7stM01PN/IFwqz2mKeqfb4NR1ZXODUPy l7GA== X-Gm-Message-State: APf1xPD9JzWbNPy1L4xxa3fGqjMuuHVqSKaxNNg5EA0jvLHQvxM6/bjD vo8TPkmDLBi0g6dFZRoIa4Y= X-Google-Smtp-Source: AH8x225UJdC8He9a1zYhHlqLmJm6d6gIAAW1i6bNrEV6o3R0waeW7WR+UhjON9wDp5P4n85Cot9/Dg== X-Received: by 10.55.33.156 with SMTP id f28mr10424739qki.5.1518034051641; Wed, 07 Feb 2018 12:07:31 -0800 (PST) Received: from localhost (dhcp-ec-8-6b-ed-7a-cf.cpe.echoes.net. [72.28.5.223]) by smtp.gmail.com with ESMTPSA id y30sm1605951qtm.50.2018.02.07.12.07.30 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 07 Feb 2018 12:07:31 -0800 (PST) Date: Wed, 7 Feb 2018 12:07:24 -0800 From: "tj@kernel.org" To: Bart Van Assche Cc: "hch@lst.de" , "linux-block@vger.kernel.org" , "axboe@kernel.dk" Subject: Re: [PATCH v2] blk-mq: Fix race between resetting the timer and completion handling Message-ID: <20180207200724.GD695913@devbig577.frc2.facebook.com> References: <20180207011133.25957-1-bart.vanassche@wdc.com> <20180207170612.GB695913@devbig577.frc2.facebook.com> <1518024428.2870.35.camel@wdc.com> <20180207173531.GC695913@devbig577.frc2.facebook.com> <1518027251.2870.53.camel@wdc.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1518027251.2870.53.camel@wdc.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hello, Bart. On Wed, Feb 07, 2018 at 06:14:13PM +0000, Bart Van Assche wrote: > When I wrote my comment I was not sure whether or not non-reentrancy is > guaranteed for work queue items. However, according to what I found in the > workqueue implementation I think that is guaranteed. So it shouldn't be > possible that the timer activated by blk_add_timer() gets handled before > aborted_gstate is reset. But since the timeout handler and completion Yeah, we're basically single threaded in the timeout path. > handlers can be executed by different CPUs, shouldn't a memory barrier be > inserted between the blk_add_timer() call and resetting aborted_gsync to > guarantee that a completion cannot occur before blk_add_timer() has reset > RQF_MQ_TIMEOUT_EXPIRED? Ah, you're right. u64_stat_sync doesn't imply barriers, so we want something like the following. diff --git a/block/blk-mq.c b/block/blk-mq.c index df93102..d6edf3b 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -593,7 +593,7 @@ static void blk_mq_rq_update_aborted_gstate(struct request *rq, u64 gstate) */ local_irq_save(flags); u64_stats_update_begin(&rq->aborted_gstate_sync); - rq->aborted_gstate = gstate; + smp_store_release(&rq->aborted_gstate, gstate); u64_stats_update_end(&rq->aborted_gstate_sync); local_irq_restore(flags); } @@ -605,7 +605,7 @@ static u64 blk_mq_rq_aborted_gstate(struct request *rq) do { start = u64_stats_fetch_begin(&rq->aborted_gstate_sync); - aborted_gstate = rq->aborted_gstate; + aborted_gstate = smp_load_acquire(&rq->aborted_gstate); } while (u64_stats_fetch_retry(&rq->aborted_gstate_sync, start)); return aborted_gstate; @@ -836,8 +836,8 @@ static void blk_mq_rq_timed_out(struct request *req, bool reserved) * ->aborted_gstate is set, this may lead to ignored * completions and further spurious timeouts. */ - blk_mq_rq_update_aborted_gstate(req, 0); blk_add_timer(req); + blk_mq_rq_update_aborted_gstate(req, 0); break; case BLK_EH_NOT_HANDLED: break;