Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 1 Mar 2022 18:29:01 +0800
From:   Ming Lei <ming.lei@redhat.com>
To:     Christoph Hellwig <hch@infradead.org>
Cc:     Yu Kuai <yukuai3@huawei.com>, tj@kernel.org, axboe@kernel.dk,
        cgroups@vger.kernel.org, linux-block@vger.kernel.org,
        linux-kernel@vger.kernel.org, yi.zhang@huawei.com
Subject: Re: [PATCH v9] block: cancel all throttled bios in del_gendisk()
Message-ID: <Yh31bQu3gbXoDBuK@T590>
References: <20220210115637.1074927-1-yukuai3@huawei.com>
 <YhuyBgZSS6m/Mwu6@infradead.org>
 <Yhxnkg0AEaj36t+a@T590>
 <YhyYpWHGVhs3J/dk@infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YhyYpWHGVhs3J/dk@infradead.org>
Precedence: bulk

On Mon, Feb 28, 2022 at 01:40:53AM -0800, Christoph Hellwig wrote:
> On Mon, Feb 28, 2022 at 02:11:30PM +0800, Ming Lei wrote:
> > > FYI, this crashed left rigt and center when running xfstests with
> > > traces pointing to throtl_pending_timer_fn.
> > 
> > Can you share the exact xfstests test(fs, test)? Or panic log?
> > 
> > I can't reproduce it when running './check -g auto' on XFS, meantime
> > tracking throtl_pending_timer_fn().
> 
> From a quick run using f2fs:
> 
> generic/081 files ... [  316.487861] run fstests generic/081 at 2022-02-28 09:38:40

Thanks for providing the reproducer.

The reason is that the pending timer is deleted in blkg's release
handler, so the timer can still be live after request queue is released.

The patch of 'block: cancel all throttled bios in del_gendisk()' should just
make it easier to trigger.

After patch of "block: move blkcg initialization/destroy into disk allocation/
release handler" lands, the issue can be fixed easily by:

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index fa063c6c0338..e8d4be5e1de3 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -82,6 +82,7 @@ static void blkg_free(struct blkcg_gq *blkg)
 		if (blkg->pd[i])
 			blkcg_policy[i]->pd_free_fn(blkg->pd[i]);
 
+	blk_put_queue(blkg->q);
 	free_percpu(blkg->iostat_cpu);
 	percpu_ref_exit(&blkg->refcnt);
 	kfree(blkg);
@@ -297,9 +298,10 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg,
 	blkg->online = true;
 	spin_unlock(&blkcg->lock);
 
-	if (!ret)
+	if (!ret && blk_get_queue(q))
 		return blkg;
-
+	else if (!ret)
+		ret = -ENODEV;
 	/* @blkg failed fully initialized, use the usual release path */
 	blkg_put(blkg);
 	return ERR_PTR(ret);


Thanks,
Ming