From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-180.mta0.migadu.com (out-180.mta0.migadu.com [91.218.175.180]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 499CC266581 for ; Wed, 5 Aug 2026 00:59:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.180 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785891545; cv=none; b=DOrzCuZfWhIm+KRKtyRoB6elRRKuRdm03Jz0OB+HdUNvbDht7WI5yHeyYFSZ2Moc1+xz57KKqZ2Xk3eeumsBK6ly8Xj+N4swgcCmHucW7rfntDQZSab/eSkA1Vc1pYYgZhzVF6qnmCGzAM6Uk8A+4V07mRAczSWrO8G9mze930s= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785891545; c=relaxed/simple; bh=QXq+No4NCvplH5iP+cVU/DLqFEsEiO2ABlKqbYf8N0U=; h=Message-ID:Date:MIME-Version:Cc:Subject:To:References:From: In-Reply-To:Content-Type; b=kqIpXP3C7i6Il3J96zQA2fgG46csQ5RfvI1c2puhoXuAu1ZyOa5ZQYqGA7Hr9udxNmq/kx2+WZ8AYwJAxQdJ33bhuSqJYuYJkbEPliaySy8S3iLRaDO/g+4vRPT76nsGh/4muZyQwd9jAaWLhIqhNJSWvv8QolM5spB/qsNdLDI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=UKD+o0Q9; arc=none smtp.client-ip=91.218.175.180 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="UKD+o0Q9" Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1785891530; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=kXCZx29YWK0KuSZTNZlcIo3NjcMPu1Ep/GSGxvMq9+g=; b=UKD+o0Q9O0GP/b1+xVPt8DW6AJyHnKdGgtR0kNlepREPLrvPQUsgmz6uhTn2BgsnwwgsW7 6FERSa+9CQu0aEvQy/RHASDL9rD1MgLiRdBkudQn0F5tsVVNDL9YEXbEVMFS961KB/SsYE mzNcNjONJWNh0bFcrOTll82xxRN0PX0= Date: Wed, 5 Aug 2026 08:58:09 +0800 Precedence: bulk X-Mailing-List: gfs2@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Cc: cui.tao@linux.dev, 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 , cgroups@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-block@vger.kernel.org, linux-bcache@vger.kernel.org, dm-devel@lists.linux.dev, linux-raid@vger.kernel.org, nvdimm@lists.linux.dev, virtualization@lists.linux.dev, gfs2@lists.linux.dev, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [RFC PATCH v1 2/3] blk-cgroup: store blkcg in bio instead of blkg To: yukuai@fygo.io, Christoph Hellwig References: <20260804065313.2092022-1-yukuai@kernel.org> <20260804065313.2092022-3-yukuai@kernel.org> <20260804133208.GB8078@lst.de> <0fc5dd22-7a8d-43f0-8555-d2ca966efff6@fygo.io> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Tao Cui In-Reply-To: <0fc5dd22-7a8d-43f0-8555-d2ca966efff6@fygo.io> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT Hi Kuai,Christoph, 在 2026/8/4 23:30, yu kuai 写道: > Hi, > > 在 2026/8/4 21:32, Christoph Hellwig 写道: >> 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 — 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=4096; >>> - a read issued in the child cgroup gets throttled and queued, pinning >>> 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? >> yes, I'll turn the reproducer into a blktests case and send it out. >>> 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 pinned > by blkg_get, in this case remove the cgroup directly remove the blkg from radix > tree, that's why blkg_lookup can't find this blkg anymore, and the extra blkg 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 radix tree, > even through it still has active refcounts. I think this can be fixed by checking > 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_workfn(). > What's better, if we can convert the blkg list to hash table with key as blk-cgroup, > it will be much better as we can lookup from this table instead of blkcg radix tree. > Both of your approaches go further than my store-the-pointer idea; looking forward to the next version. Thanks, Tao >>