From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from va-2-53.ptr.blmpb.com (va-2-53.ptr.blmpb.com [209.127.231.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D02383B1009 for ; Tue, 4 Aug 2026 15:31:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.127.231.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785857496; cv=none; b=SbxgYsXBj36PdwYKOjLuAS2sgydE8XBd+V5AuZoCO3zs4YxuY1uWLHoLvh0rrR1dh3GgFry93s3LYdyPIeJKSopFXB7F1dzrapdu8p3Az2B7h/FOODd81f37pvgYH4zHab0KkRvgAUVSwbJRzVD8xc4bRi2KNGayGxZBPdLHcPI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785857496; c=relaxed/simple; bh=by/fcGncfc5OCuRW9skgJuC6t1LUujBBm0TnhfUwb/o=; h=From:Content-Type:Mime-Version:In-Reply-To:Cc:Subject:References: To:Date:Message-Id; b=qvdtOLqpVcRKooM3rAtV+AbkUf1O6KklWbA8emKMc1t/9WXPB9f9hR0Ts90oqZK7a/AOlT2ZT2nyoesV5dMfn6M0+bcahakRM18Afo89oNFxdq07Yyomu6TNQK0Aeb05b/zRfWMP6nvFv+u0i0tVFC9PlRF2/0bcDv8HrSUl2jo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=fygo.io; spf=pass smtp.mailfrom=fygo.io; dkim=pass (2048-bit key) header.d=fygo-io.20200929.dkim.larksuite.com header.i=@fygo-io.20200929.dkim.larksuite.com header.b=SAuhuLf8; arc=none smtp.client-ip=209.127.231.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=fygo.io Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=fygo.io Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=fygo-io.20200929.dkim.larksuite.com header.i=@fygo-io.20200929.dkim.larksuite.com header.b="SAuhuLf8" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; s=s1; d=fygo-io.20200929.dkim.larksuite.com; t=1785857479; h=from:subject:mime-version:from:date:message-id:subject:to:cc: reply-to:content-type:mime-version:in-reply-to:message-id; bh=LX/Wg0tks/cc/fTlAyp1t5QsfNTLQnLsZm1iWKs61Hs=; b=SAuhuLf8EeGKGsRB+g4s6HX7NDoAzM6N6u64XjLKfCJO3BuUyEh84mIQBGbDbMwrClQKGN LiznKfJebp/iFwdIWh+vwD/S5Bq8BEFgu3zzc2ggSCJZu9QCGoOMr2xKtfyQ/DK7pd4W6k coIzpoOzRMwBJHB4C4t0BIovVY0EUM0UXvuUQS9s05mVFCVgG1ue95e9tCsQpecQMZTJ8q FYV00Mtv1Evufztxzmr8n+RW4Emn1+0KPKRmh+TSOeWuc1565iRRZzTHUSHHS8sxrLCFfv r4JHNRSZ1HGgSqzwR9LRL+YljEvN9sdXu+1/bAIx4hQgR7VlNlX+BtewZ5VQVg== From: "yu kuai" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Reply-To: yukuai@fygo.io Precedence: bulk X-Mailing-List: linux-bcache@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 X-Original-From: yu kuai In-Reply-To: <20260804133208.GB8078@lst.de> User-Agent: Mozilla Thunderbird Cc: "Yu Kuai" , "Jens Axboe" , "Tejun Heo" , "Johannes Weiner" , =?utf-8?q?Michal_Koutn=C3=BD?= , "Jonathan Corbet" , "Josef Bacik" , "Coly Li" , "Kent Overstreet" , "Alasdair Kergon" , "Mike Snitzer" , "Mikulas Patocka" , "Benjamin Marzinski" , "Song Liu" , "Dan Williams" , "Vishal Verma" , "Dave Jiang" , "Alison Schofield" , "Pankaj Gupta" , "Andreas Gruenbacher" , "Matthew Wilcox" , "Jan Kara" , "Andrew Morton" , "Chris Li" , "Kairui Song" , "Nilay Shroff" , , , , , , , , , , , , Subject: Re: [RFC PATCH v1 2/3] blk-cgroup: store blkcg in bio instead of blkg References: <20260804065313.2092022-1-yukuai@kernel.org> <20260804065313.2092022-3-yukuai@kernel.org> <20260804133208.GB8078@lst.de> Received: from [192.168.1.104] ([39.182.0.181]) by smtp.larksuite.com with ESMTPS; Tue, 04 Aug 2026 15:31:03 +0000 To: "Christoph Hellwig" , "Tao Cui" , "yu kuai" Date: Tue, 4 Aug 2026 23:30:55 +0800 Message-Id: <0fc5dd22-7a8d-43f0-8555-d2ca966efff6@fygo.io> X-Lms-Return-Path: Hi, =E5=9C=A8 2026/8/4 21:32, Christoph Hellwig =E5=86=99=E9=81=93: > On Tue, Aug 04, 2026 at 05:19:24PM +0800, Tao Cui wrote: >> While reading 2/3, one spot in bio_pinned_blkg() made me wonder, so I >> gave it a try =E2=80=94 and the WARN_ON_ONCE triggers every time for me. >> >> I may well be missing something, but my worry is that the bio's ref on >> the blkg keeps the object alive, not its entry in the radix tree. >> blkg_destroy() runs throtl_pd_offline (which only schedules an async >> flush) before radix_tree_delete(), so the queued bio ends up dispatched >> (blk_throtl_dispatch_work_fn -> blk_cgroup_bio_start -> >> bio_pinned_blkg) after the blkg is already gone from the tree, and >> blkg_lookup() returns NULL. >> >> I applied the series and wrote a small reproducer: >> >> - null_blk, cgroup v2, a child cgroup with io.max rbps=3D4096; >> - a read issued in the child cgroup gets throttled and queued, pinnin= g >> the blkg; >> - migrate the reader out and rmdir the cgroup; the queued bio is then >> flushed after the blkg has left the tree. > Can you add this to blktests? > >> Maybe keeping the pinned blkg pointer in the bio would sidestep this, so >> the lookup can't miss? The problem here is that blkg_destroy can be called while blkg is still pin= ned by blkg_get, in this case remove the cgroup directly remove the blkg from r= adix tree, that's why blkg_lookup can't find this blkg anymore, and the extra bl= kg ref is leaked :( > That would grow the bio, which we try hard to avoid. I think the way to > avoid this is to have active/passive refcounts on the blkg, where an > active one keeps it in the radix tree, but a 0 passive one would prevent > the caller from getting a new reference to it. The users who rely on the > pin for the I/O completion path would then just keep the active reference > and use a pure lookup without getting a new passive reference in the > completion path. This would remove the need for BIO_BLKG_REF which > feels a bit kludgy and eats up precious bio flag space. The problem here is that remove a cgroup can also remove the blkg from radi= x tree, even through it still has active refcounts. I think this can be fixed by ch= ecking the cgroup online_pin first, if it's zero, we can search the blkg from the request_queue blkg list, where blkg will not be removed until blkg_free_wor= kfn(). What's better, if we can convert the blkg list to hash table with key as bl= k-cgroup, it will be much better as we can lookup from this table instead of blkcg ra= dix tree. > --=20 Thanks, Kuai