From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-eopbgr730114.outbound.protection.outlook.com ([40.107.73.114]:62688 "EHLO NAM05-DM3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1733084AbeIXUvk (ORCPT ); Mon, 24 Sep 2018 16:51:40 -0400 From: Sasha Levin To: "stable@vger.kernel.org" , "linux-kernel@vger.kernel.org" CC: "Dennis Zhou (Facebook)" , Jiufei Xue , Joseph Qi , Tejun Heo , Jens Axboe , Sasha Levin Subject: [PATCH AUTOSEL 4.18 44/76] Revert "blk-throttle: fix race between blkcg_bio_issue_check() and cgroup_rmdir()" Date: Mon, 24 Sep 2018 14:48:29 +0000 Message-ID: <20180924144751.164410-43-alexander.levin@microsoft.com> References: <20180924144751.164410-1-alexander.levin@microsoft.com> In-Reply-To: <20180924144751.164410-1-alexander.levin@microsoft.com> Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Sender: stable-owner@vger.kernel.org List-ID: From: "Dennis Zhou (Facebook)" [ Upstream commit 6b06546206868f723f2061d703a3c3c378dcbf4c ] This reverts commit 4c6994806f708559c2812b73501406e21ae5dcd0. Destroying blkgs is tricky because of the nature of the relationship. A blkg should go away when either a blkcg or a request_queue goes away. However, blkg's pin the blkcg to ensure they remain valid. To break this cycle, when a blkcg is offlined, blkgs put back their css ref. This eventually lets css_free() get called which frees the blkcg. The above commit (4c6994806f70) breaks this order of events by trying to destroy blkgs in css_free(). As the blkgs still hold references to the blkcg, css_free() is never called. The race between blkcg_bio_issue_check() and cgroup_rmdir() will be addressed in the following patch by delaying destruction of a blkg until all writeback associated with the blkcg has been finished. Fixes: 4c6994806f70 ("blk-throttle: fix race between blkcg_bio_issue_check(= ) and cgroup_rmdir()") Reviewed-by: Josef Bacik Signed-off-by: Dennis Zhou Cc: Jiufei Xue Cc: Joseph Qi Cc: Tejun Heo Cc: Jens Axboe Signed-off-by: Jens Axboe Signed-off-by: Sasha Levin --- block/blk-cgroup.c | 78 ++++++++------------------------------ include/linux/blk-cgroup.h | 1 - 2 files changed, 16 insertions(+), 63 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index eb85cb87c40f..ec868373b11b 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -307,28 +307,11 @@ struct blkcg_gq *blkg_lookup_create(struct blkcg *blk= cg, } } =20 -static void blkg_pd_offline(struct blkcg_gq *blkg) -{ - int i; - - lockdep_assert_held(blkg->q->queue_lock); - lockdep_assert_held(&blkg->blkcg->lock); - - for (i =3D 0; i < BLKCG_MAX_POLS; i++) { - struct blkcg_policy *pol =3D blkcg_policy[i]; - - if (blkg->pd[i] && !blkg->pd[i]->offline && - pol->pd_offline_fn) { - pol->pd_offline_fn(blkg->pd[i]); - blkg->pd[i]->offline =3D true; - } - } -} - static void blkg_destroy(struct blkcg_gq *blkg) { struct blkcg *blkcg =3D blkg->blkcg; struct blkcg_gq *parent =3D blkg->parent; + int i; =20 lockdep_assert_held(blkg->q->queue_lock); lockdep_assert_held(&blkcg->lock); @@ -337,6 +320,13 @@ static void blkg_destroy(struct blkcg_gq *blkg) WARN_ON_ONCE(list_empty(&blkg->q_node)); WARN_ON_ONCE(hlist_unhashed(&blkg->blkcg_node)); =20 + for (i =3D 0; i < BLKCG_MAX_POLS; i++) { + struct blkcg_policy *pol =3D blkcg_policy[i]; + + if (blkg->pd[i] && pol->pd_offline_fn) + pol->pd_offline_fn(blkg->pd[i]); + } + if (parent) { blkg_rwstat_add_aux(&parent->stat_bytes, &blkg->stat_bytes); blkg_rwstat_add_aux(&parent->stat_ios, &blkg->stat_ios); @@ -379,7 +369,6 @@ static void blkg_destroy_all(struct request_queue *q) struct blkcg *blkcg =3D blkg->blkcg; =20 spin_lock(&blkcg->lock); - blkg_pd_offline(blkg); blkg_destroy(blkg); spin_unlock(&blkcg->lock); } @@ -1006,54 +995,21 @@ static struct cftype blkcg_legacy_files[] =3D { * @css: css of interest * * This function is called when @css is about to go away and responsible - * for offlining all blkgs pd and killing all wbs associated with @css. - * blkgs pd offline should be done while holding both q and blkcg locks. - * As blkcg lock is nested inside q lock, this function performs reverse - * double lock dancing. + * for shooting down all blkgs associated with @css. blkgs should be + * removed while holding both q and blkcg locks. As blkcg lock is nested + * inside q lock, this function performs reverse double lock dancing. * * This is the blkcg counterpart of ioc_release_fn(). */ static void blkcg_css_offline(struct cgroup_subsys_state *css) { struct blkcg *blkcg =3D css_to_blkcg(css); - struct blkcg_gq *blkg; =20 spin_lock_irq(&blkcg->lock); =20 - hlist_for_each_entry(blkg, &blkcg->blkg_list, blkcg_node) { - struct request_queue *q =3D blkg->q; - - if (spin_trylock(q->queue_lock)) { - blkg_pd_offline(blkg); - spin_unlock(q->queue_lock); - } else { - spin_unlock_irq(&blkcg->lock); - cpu_relax(); - spin_lock_irq(&blkcg->lock); - } - } - - spin_unlock_irq(&blkcg->lock); - - wb_blkcg_offline(blkcg); -} - -/** - * blkcg_destroy_all_blkgs - destroy all blkgs associated with a blkcg - * @blkcg: blkcg of interest - * - * This function is called when blkcg css is about to free and responsible= for - * destroying all blkgs associated with @blkcg. - * blkgs should be removed while holding both q and blkcg locks. As blkcg = lock - * is nested inside q lock, this function performs reverse double lock dan= cing. - */ -static void blkcg_destroy_all_blkgs(struct blkcg *blkcg) -{ - spin_lock_irq(&blkcg->lock); while (!hlist_empty(&blkcg->blkg_list)) { struct blkcg_gq *blkg =3D hlist_entry(blkcg->blkg_list.first, - struct blkcg_gq, - blkcg_node); + struct blkcg_gq, blkcg_node); struct request_queue *q =3D blkg->q; =20 if (spin_trylock(q->queue_lock)) { @@ -1065,7 +1021,10 @@ static void blkcg_destroy_all_blkgs(struct blkcg *bl= kcg) spin_lock_irq(&blkcg->lock); } } + spin_unlock_irq(&blkcg->lock); + + wb_blkcg_offline(blkcg); } =20 static void blkcg_css_free(struct cgroup_subsys_state *css) @@ -1073,8 +1032,6 @@ static void blkcg_css_free(struct cgroup_subsys_state= *css) struct blkcg *blkcg =3D css_to_blkcg(css); int i; =20 - blkcg_destroy_all_blkgs(blkcg); - mutex_lock(&blkcg_pol_mutex); =20 list_del(&blkcg->all_blkcgs_node); @@ -1412,11 +1369,8 @@ void blkcg_deactivate_policy(struct request_queue *q= , =20 list_for_each_entry(blkg, &q->blkg_list, q_node) { if (blkg->pd[pol->plid]) { - if (!blkg->pd[pol->plid]->offline && - pol->pd_offline_fn) { + if (pol->pd_offline_fn) pol->pd_offline_fn(blkg->pd[pol->plid]); - blkg->pd[pol->plid]->offline =3D true; - } pol->pd_free_fn(blkg->pd[pol->plid]); blkg->pd[pol->plid] =3D NULL; } diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h index 0fce47d5acb1..5d46b83d4820 100644 --- a/include/linux/blk-cgroup.h +++ b/include/linux/blk-cgroup.h @@ -88,7 +88,6 @@ struct blkg_policy_data { /* the blkg and policy id this per-policy data belongs to */ struct blkcg_gq *blkg; int plid; - bool offline; }; =20 /* --=20 2.17.1