From patchwork Thu Apr 12 22:40:52 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bart Van Assche X-Patchwork-Id: 10339283 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 717B9600D0 for ; Thu, 12 Apr 2018 22:40:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 76E5E28560 for ; Thu, 12 Apr 2018 22:40:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6BA5C28578; Thu, 12 Apr 2018 22:40:57 +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=-7.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI 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 C177028581 for ; Thu, 12 Apr 2018 22:40:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752414AbeDLWkz (ORCPT ); Thu, 12 Apr 2018 18:40:55 -0400 Received: from esa3.hgst.iphmx.com ([216.71.153.141]:10289 "EHLO esa3.hgst.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751963AbeDLWkz (ORCPT ); Thu, 12 Apr 2018 18:40:55 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=wdc.com; i=@wdc.com; q=dns/txt; s=dkim.wdc.com; t=1523572856; x=1555108856; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=vSmYVOOtoMAfdPl9svW1OhjWKqVwpi+9wKbH9tPhCKk=; b=KswbD8Ydqnx19ttH0VP3c9ruR1L1H5y/XP2lkFIALuQkgpFDxEJ+KIpI YxqaYfnG1OuwVYA23DqZl+EAk7wh+0m5YdQbEKnDsKM7gGP97e3BO0RGF eYs2Ra6sZ5hxAcchM6L00xgshjIJdbX0dyAFFzQpbOuFMEaS2RPstG3a+ Ikzj0jnX+Yel1r1+gDl18HvwnT1Vncy3815DjrEkwMUomWtyVdade28NY uClR3Lie+a0cumhhaBwm7HU2LbqKsAznCTTQj+Hw/LN3PXmKIPWUk+Tx4 r5JmxLVJHVnyAIZBIYL+vEm9cBK9S5IKO1DL1DlSlx98R1v7+ezXHfXko Q==; X-IronPort-AV: E=Sophos;i="5.48,443,1517846400"; d="scan'208";a="76609937" Received: from mail-dm3nam03lp0020.outbound.protection.outlook.com (HELO NAM03-DM3-obe.outbound.protection.outlook.com) ([207.46.163.20]) by ob1.hgst.iphmx.com with ESMTP; 13 Apr 2018 06:40:55 +0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sharedspace.onmicrosoft.com; s=selector1-wdc-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=vSmYVOOtoMAfdPl9svW1OhjWKqVwpi+9wKbH9tPhCKk=; b=ADUXb2u6viand9SrL62VwtkIPf5LhAQyQzB63Yc62vTIU5cqQda8/e2f00QpPm0ZlWqQMnpVUfCL8SA+CM1BfBrIAS2o7bjXrPe59lE7aM3GE8K7FGRIzNAce8qSHn6WnkP7hLI1jpQQaDR7EAEaxhTn3S76E1opJnU/BS+bx1Q= Received: from MWHPR04MB1198.namprd04.prod.outlook.com (10.173.48.151) by MWHPR04MB0494.namprd04.prod.outlook.com (10.173.49.7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.675.11; Thu, 12 Apr 2018 22:40:52 +0000 Received: from MWHPR04MB1198.namprd04.prod.outlook.com ([fe80::bc43:e461:43cb:4d27]) by MWHPR04MB1198.namprd04.prod.outlook.com ([fe80::bc43:e461:43cb:4d27%14]) with mapi id 15.20.0653.014; Thu, 12 Apr 2018 22:40:52 +0000 From: Bart Van Assche To: "tj@kernel.org" CC: "hch@lst.de" , "linux-block@vger.kernel.org" , "00moses.alexander00@gmail.com" <00moses.alexander00@gmail.com>, "axboe@kernel.dk" Subject: Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller Thread-Topic: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller Thread-Index: AQHT0gIj6JehxhuC4UKjkK6RNGNvy6P9JsqAgAAE4YCAABi7AIAAB0WAgAACgYCAAASKgIAAHJeAgAAMl4CAAAOWgIAAOx4A Date: Thu, 12 Apr 2018 22:40:52 +0000 Message-ID: References: <20180412015852.26014-1-bart.vanassche@wdc.com> <20180412135149.GX793541@devbig577.frc2.facebook.com> <20180412153748.GB793541@devbig577.frc2.facebook.com> <20180412161247.GD793541@devbig577.frc2.facebook.com> <20180412181121.GA1911913@devbig577.frc2.facebook.com> <22e0bba319877407a0abccf292ef0b22454db04d.camel@wdc.com> <20180412190915.GB1911913@devbig577.frc2.facebook.com> In-Reply-To: <20180412190915.GB1911913@devbig577.frc2.facebook.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Bart.VanAssche@wdc.com; x-originating-ip: [50.225.201.71] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; MWHPR04MB0494; 7:l5meuxotcdWmsYGNF53s2XabLOHqPxQM3GiTBP22AV6drP/TihcPM0QD2E+vvHow3CojlHkGtFboWLtoEInhcEFj6UmT/d02xazVf6U57WiQgD198BIVuI8WhOxVpN/f1Qxu0soagQ9eL4Whcvuy69Pk8JLsvM4Idut3DnsmDSPIv+DwWIfr6bzW2bM+9J5dc+yjM7OexsKCpBGgMLG3liHJkNNiMh7EWUkSCtpjxW2R7Zo9C9TnerKMrR+ABYIK; 20:QC4IzWck/6hgbvLw+qTzSk9dyZTGlJRBfSGFJqt0+qPX+l5T8do7HcsSPTOn2STU5+TCRazpNZp/FHIqz0klBhKgk+lmuGtDMzhj6qV+Di3m43hXtNKi9JnIK17nNUUM+fz2fsSrzk6iJVzCvyG+XPeSgRp76zHvwj9LL2NeDGQ= x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(5600026)(4534165)(4627221)(201703031133081)(201702281549075)(48565401081)(2017052603328)(7153060)(7193020); SRVR:MWHPR04MB0494; x-ms-traffictypediagnostic: MWHPR04MB0494: wdcipoutbound: EOP-TRUE x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040522)(2401047)(5005006)(8121501046)(3002001)(93006095)(93001095)(3231232)(944501327)(52105095)(10201501046)(6055026)(6041310)(20161123558120)(20161123560045)(20161123562045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(6072148)(201708071742011); SRVR:MWHPR04MB0494; BCL:0; PCL:0; RULEID:; SRVR:MWHPR04MB0494; x-forefront-prvs: 06400060E1 x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(376002)(39860400002)(346002)(366004)(396003)(39380400002)(189003)(199004)(377424004)(6506007)(97736004)(6512007)(5640700003)(118296001)(6916009)(102836004)(59450400001)(186003)(26005)(6436002)(2616005)(476003)(11346002)(446003)(486006)(54906003)(76176011)(93886005)(99286004)(68736007)(106356001)(316002)(86362001)(575784001)(305945005)(7736002)(39060400002)(6486002)(229853002)(81166006)(1730700003)(4326008)(36756003)(8676002)(2351001)(14454004)(478600001)(72206003)(2900100001)(81156014)(66066001)(6246003)(105586002)(2906002)(53936002)(8936002)(3280700002)(2501003)(25786009)(3846002)(6116002)(5250100002)(5660300001)(3660700001); DIR:OUT; SFP:1102; SCL:1; SRVR:MWHPR04MB0494; H:MWHPR04MB1198.namprd04.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; x-microsoft-antispam-message-info: G73teVKkTgk9o2cCdBGeYguMNg+F4j6eR0hqm94XZrRkg2IwO+Rf6aydtMQzP5UHr1ZguzkjUSmSINDrzZPRXWkE6LxElEZFVwkaD4IdeK6wWgvU8PUu3xkfYUPJKShZzT8CCdWREu8FPeeDIIomfwM8jinVJJm411lXCOYraNxyfiggeeRfHgIieyltZs2t spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-ID: <7887A008C4D48E4D85004BCC815794DB@namprd04.prod.outlook.com> MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: 8c57b2d4-1226-4ae8-7f0d-08d5a0c67237 X-OriginatorOrg: wdc.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8c57b2d4-1226-4ae8-7f0d-08d5a0c67237 X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Apr 2018 22:40:52.0657 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86 X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR04MB0494 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 On Thu, 2018-04-12 at 12:09 -0700, tj@kernel.org wrote: > On Thu, Apr 12, 2018 at 06:56:26PM +0000, Bart Van Assche wrote: > > On Thu, 2018-04-12 at 11:11 -0700, tj@kernel.org wrote: > > > * Right now, blk_queue_enter/exit() doesn't have any annotations. > > > IOW, there's no way for paths which need enter locked to actually > > > assert that. If we're gonna protect more things with queue > > > enter/exit, it gotta be annotated. > > > > Hello Tejun, > > > > One possibility is to check the count for the local CPU of q->q_usage_counter > > at runtime, e.g. using WARN_ON_ONCE(). However, that might have a runtime > > overhead. Another possibility is to follow the example of kernfs and to use > > a lockdep map and lockdep_assert_held(). There might be other alternatives I > > have not thought of. > > Oh, I'd just do straight-forward lockdep annotation on > queue_enter/exit functions and provide the necessary assert helpers. Hello Tejun, Is something like the patch below perhaps what you had in mind? Thanks, Bart. Subject: [PATCH] block: Use lockdep to verify whether blk_queue_enter() is used when necessary Suggested-by: Tejun Heo Signed-off-by: Bart Van Assche Cc: Tejun Heo --- block/blk-cgroup.c | 2 ++ block/blk-core.c | 13 ++++++++++++- block/blk.h | 1 + include/linux/blk-cgroup.h | 2 ++ include/linux/blkdev.h | 1 + 5 files changed, 18 insertions(+), 1 deletion(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 1c16694ae145..c34e13e76f90 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -145,6 +145,8 @@ struct blkcg_gq *blkg_lookup_slowpath(struct blkcg *blkcg, { struct blkcg_gq *blkg; + lock_is_held(&q->q_enter_map); + /* * Hint didn't match. Look up from the radix tree. Note that the * hint can only be updated under queue_lock as otherwise @blkg diff --git a/block/blk-core.c b/block/blk-core.c index 39308e874ffa..a61dbe7f24a5 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -931,8 +931,13 @@ int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags) } rcu_read_unlock(); - if (success) + if (success) { + mutex_acquire_nest(&q->q_enter_map, 0, 0, + lock_is_held(&q->q_enter_map) ? + &q->q_enter_map : NULL, + _RET_IP_); return 0; + } if (flags & BLK_MQ_REQ_NOWAIT) return -EBUSY; @@ -959,6 +964,7 @@ int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags) void blk_queue_exit(struct request_queue *q) { + mutex_release(&q->q_enter_map, 0, _RET_IP_); percpu_ref_put(&q->q_usage_counter); } @@ -994,12 +1000,15 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id, spinlock_t *lock) { struct request_queue *q; + static struct lock_class_key __key; q = kmem_cache_alloc_node(blk_requestq_cachep, gfp_mask | __GFP_ZERO, node_id); if (!q) return NULL; + lockdep_init_map(&q->q_enter_map, "q_enter_map", &__key, 0); + q->id = ida_simple_get(&blk_queue_ida, 0, 0, gfp_mask); if (q->id < 0) goto fail_q; @@ -2264,6 +2273,8 @@ generic_make_request_checks(struct bio *bio) goto end_io; } + lock_is_held(&q->q_enter_map); + /* * For a REQ_NOWAIT based request, return -EOPNOTSUPP * if queue is not a request based queue. diff --git a/block/blk.h b/block/blk.h index 7cd64f533a46..26f313331b13 100644 --- a/block/blk.h +++ b/block/blk.h @@ -145,6 +145,7 @@ static inline void blk_queue_enter_live(struct request_queue *q) * been established, further references under that same context * need not check that the queue has been frozen (marked dead). */ + mutex_acquire_nest(&q->q_enter_map, 0, 0, &q->q_enter_map, _RET_IP_); percpu_ref_get(&q->q_usage_counter); } diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h index 6c666fd7de3c..52e2e2d9971e 100644 --- a/include/linux/blk-cgroup.h +++ b/include/linux/blk-cgroup.h @@ -266,6 +266,8 @@ static inline struct blkcg_gq *__blkg_lookup(struct blkcg *blkcg, { struct blkcg_gq *blkg; + lock_is_held(&q->q_enter_map); + if (blkcg == &blkcg_root) return q->root_blkg; diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 0d22f351a74b..a0b1adbd4406 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -631,6 +631,7 @@ struct request_queue { struct rcu_head rcu_head; wait_queue_head_t mq_freeze_wq; struct percpu_ref q_usage_counter; + struct lockdep_map q_enter_map; struct list_head all_q_node; struct blk_mq_tag_set *tag_set;