From patchwork Wed Mar 1 23:43:19 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tahsin Erdogan X-Patchwork-Id: 9599265 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 4C0DE600CB for ; Thu, 2 Mar 2017 00:08:00 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 33BA128543 for ; Thu, 2 Mar 2017 00:08:00 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2899E2853D; Thu, 2 Mar 2017 00:08:00 +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.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, 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 88E1E2853D for ; Thu, 2 Mar 2017 00:07:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753540AbdCBAH6 (ORCPT ); Wed, 1 Mar 2017 19:07:58 -0500 Received: from mail-pf0-f171.google.com ([209.85.192.171]:32872 "EHLO mail-pf0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751387AbdCBAHy (ORCPT ); Wed, 1 Mar 2017 19:07:54 -0500 Received: by mail-pf0-f171.google.com with SMTP id w189so15644025pfb.0 for ; Wed, 01 Mar 2017 16:07:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=GzwGbeMvsh+gsqOtzGTfJOvjH5fKB9omzuP88WvCe9Y=; b=pxzWj8apSgdtqvx4vZsLbqMTzX3iP3TnvsQJwKX69QeSQIaGrQexDeCMg2hHFsEiSx 8hIl9RW7EHPOUUi6d7IIDNbbg9KlSOhVqtkNEsRdwVL+WP8AZPL3SNjBc200AK2glrt6 NFtQPjcBsgfhbezGaC0fP+sWXPjcb0FFa9Zz36yamlaNfPlw190LcCHH3KheDEpAG8NW wG2t3JABkns1Md2Yfd4QofvM5LHQ6IwYiK+rUJedhkZUd7vwhhSf/tpmuccEVB0qaJBa ejdjEYqBRRk3Z1/H7QNNDPcvSxdPwgkWW8QNVntPnUHDA+AEdCEhAIJNaj2aLCiMFblo 9mTA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=GzwGbeMvsh+gsqOtzGTfJOvjH5fKB9omzuP88WvCe9Y=; b=EfpJdT88xYNwJABze3aq2aZ+m82HYkSV+6jQMfK0Pn6dEW66BO7+u/ZTp1ZI9Gr/T3 5RRJt6qEFmjO5TtbenwIqanI6xbdvV6V/g9WuYtqAeavpVrGaPkE+D4Qj3zp42dKAz1R yahkm47ekRAtOJSTEx7p+sAt9hma9cUn6rPXU19YlNq+W8QHIruapG7vYbRNtLEPdhue VOLsVX8OWhWwEq3aKXcuvunLKgS4//BDqKm1agb4CDdIPzfzyvco48CkmoPa7MS3E9tw RVlbYNMfTHKIbvmLESGjX8fGDtpE5Vx4E4xUIxDerob1JbtoGd63HORTUS7RXZvdSDjo fAeQ== X-Gm-Message-State: AMke39mk1stl2yAJGzQSnbP74sGTu00yWl0Ao6XD4wBuNZ3UjbGxvnuWw0KXAn+qBAGm7erN X-Received: by 10.98.153.25 with SMTP id d25mr12139603pfe.15.1488411822469; Wed, 01 Mar 2017 15:43:42 -0800 (PST) Received: from tahsin1.mtv.corp.google.com ([100.99.140.90]) by smtp.gmail.com with ESMTPSA id a77sm12665889pfj.1.2017.03.01.15.43.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 01 Mar 2017 15:43:41 -0800 (PST) From: Tahsin Erdogan To: Tejun Heo , Jens Axboe Cc: linux-block@vger.kernel.org, David Rientjes , linux-kernel@vger.kernel.org, Tahsin Erdogan Subject: [PATCH v2] blkcg: allocate struct blkcg_gq outside request queue spinlock Date: Wed, 1 Mar 2017 15:43:19 -0800 Message-Id: <20170301234319.29584-1-tahsin@google.com> X-Mailer: git-send-email 2.12.0.rc1.440.g5b76565f74-goog In-Reply-To: <20170301165501.GB3662@htj.duckdns.org> References: <20170301165501.GB3662@htj.duckdns.org> 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 blkg_conf_prep() currently calls blkg_lookup_create() while holding request queue spinlock. This means allocating memory for struct blkcg_gq has to be made non-blocking. This causes occasional -ENOMEM failures in call paths like below: pcpu_alloc+0x68f/0x710 __alloc_percpu_gfp+0xd/0x10 __percpu_counter_init+0x55/0xc0 cfq_pd_alloc+0x3b2/0x4e0 blkg_alloc+0x187/0x230 blkg_create+0x489/0x670 blkg_lookup_create+0x9a/0x230 blkg_conf_prep+0x1fb/0x240 __cfqg_set_weight_device.isra.105+0x5c/0x180 cfq_set_weight_on_dfl+0x69/0xc0 cgroup_file_write+0x39/0x1c0 kernfs_fop_write+0x13f/0x1d0 __vfs_write+0x23/0x120 vfs_write+0xc2/0x1f0 SyS_write+0x44/0xb0 entry_SYSCALL_64_fastpath+0x18/0xad In the code path above, percpu allocator cannot call vmalloc() due to queue spinlock. A failure in this call path gives grief to tools which are trying to configure io weights. We see occasional failures happen shortly after reboots even when system is not under any memory pressure. Machines with a lot of cpus are more vulnerable to this condition. Add a flag to blkg_lookup_create() to indicate whether releasing locks for memory allocation purposes is okay. Suggested-by: Tejun Heo Signed-off-by: Tahsin Erdogan --- v2: Moved blkg creation into blkg_lookup_create() to avoid duplicating blkg_lookup_create() logic. block/blk-cgroup.c | 51 +++++++++++++++++++++++++++++++++++++++------- include/linux/blk-cgroup.h | 4 ++-- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 295e98c2c8cc..afb16e998bf3 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -258,18 +258,22 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, * blkg_lookup_create - lookup blkg, try to create one if not there * @blkcg: blkcg of interest * @q: request_queue of interest + * @wait_ok: whether blocking for memory allocations is okay * * Lookup blkg for the @blkcg - @q pair. If it doesn't exist, try to * create one. blkg creation is performed recursively from blkcg_root such * that all non-root blkg's have access to the parent blkg. This function * should be called under RCU read lock and @q->queue_lock. * + * When @wait_ok is true, rcu and queue locks may be dropped for allocating + * memory. In this case, the locks will be reacquired on return. + * * Returns pointer to the looked up or created blkg on success, ERR_PTR() * value on error. If @q is dead, returns ERR_PTR(-EINVAL). If @q is not * dead and bypassing, returns ERR_PTR(-EBUSY). */ struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg, - struct request_queue *q) + struct request_queue *q, bool wait_ok) { struct blkcg_gq *blkg; @@ -300,7 +304,30 @@ struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg, parent = blkcg_parent(parent); } - blkg = blkg_create(pos, q, NULL); + if (wait_ok) { + struct blkcg_gq *new_blkg; + + spin_unlock_irq(q->queue_lock); + rcu_read_unlock(); + + new_blkg = blkg_alloc(pos, q, GFP_KERNEL); + + rcu_read_lock(); + spin_lock_irq(q->queue_lock); + + if (unlikely(!new_blkg)) + return ERR_PTR(-ENOMEM); + + if (unlikely(blk_queue_bypass(q))) { + blkg_free(new_blkg); + return ERR_PTR(blk_queue_dying(q) ? + -ENODEV : -EBUSY); + } + + blkg = blkg_create(pos, q, new_blkg); + } else + blkg = blkg_create(pos, q, NULL); + if (pos == blkcg || IS_ERR(blkg)) return blkg; } @@ -789,6 +816,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol, { struct gendisk *disk; struct blkcg_gq *blkg; + struct request_queue *q; struct module *owner; unsigned int major, minor; int key_len, part, ret; @@ -812,18 +840,27 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol, return -ENODEV; } + q = disk->queue; + rcu_read_lock(); - spin_lock_irq(disk->queue->queue_lock); + spin_lock_irq(q->queue_lock); - if (blkcg_policy_enabled(disk->queue, pol)) - blkg = blkg_lookup_create(blkcg, disk->queue); - else + if (blkcg_policy_enabled(q, pol)) { + blkg = blkg_lookup_create(blkcg, q, true /* wait_ok */); + + /* + * blkg_lookup_create() may have dropped and reacquired the + * queue lock. Check policy enabled state again. + */ + if (!IS_ERR(blkg) && unlikely(!blkcg_policy_enabled(q, pol))) + blkg = ERR_PTR(-EOPNOTSUPP); + } else blkg = ERR_PTR(-EOPNOTSUPP); if (IS_ERR(blkg)) { ret = PTR_ERR(blkg); rcu_read_unlock(); - spin_unlock_irq(disk->queue->queue_lock); + spin_unlock_irq(q->queue_lock); owner = disk->fops->owner; put_disk(disk); module_put(owner); diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h index 01b62e7bac74..78067dd59c91 100644 --- a/include/linux/blk-cgroup.h +++ b/include/linux/blk-cgroup.h @@ -172,7 +172,7 @@ extern struct cgroup_subsys_state * const blkcg_root_css; struct blkcg_gq *blkg_lookup_slowpath(struct blkcg *blkcg, struct request_queue *q, bool update_hint); struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg, - struct request_queue *q); + struct request_queue *q, bool wait_ok); int blkcg_init_queue(struct request_queue *q); void blkcg_drain_queue(struct request_queue *q); void blkcg_exit_queue(struct request_queue *q); @@ -694,7 +694,7 @@ static inline bool blkcg_bio_issue_check(struct request_queue *q, blkg = blkg_lookup(blkcg, q); if (unlikely(!blkg)) { spin_lock_irq(q->queue_lock); - blkg = blkg_lookup_create(blkcg, q); + blkg = blkg_lookup_create(blkcg, q, false /* wait_ok */); if (IS_ERR(blkg)) blkg = NULL; spin_unlock_irq(q->queue_lock);