From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from fout-a8-smtp.messagingengine.com (fout-a8-smtp.messagingengine.com [103.168.172.151]) (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 435422248A3 for ; Thu, 5 Mar 2026 17:45:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.151 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772732731; cv=none; b=VZKIdaN3gu1Pve3hAx1QtZoyVsLPyF3hHh7aoHjOE8cbfUkaoMDDBlIoRJU2hd0SahtoaMC0LlTrwXhplcS9rC8En5sw8uSiS8etaVUBoKhpx3A+QOvV89x4GZQqwmraM4r8nLRKdTHFr32Ws6bg5KdoG7dd1cwUBynJchQx6y4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772732731; c=relaxed/simple; bh=pzRUyjSlX2700Hq1rMOWcOaDsOt14wSdLlNd+xT85bY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=T+bCxPwLZdlPnIw1VfGEUNxBzIs5DBfRrebV06xj/G8vNTXPsnAoxfO1zo6wmSqoSIwZDaNorHE+EoMRu/eD+L0cDr3fJowu5Sv6Z9RUWAWcvbCiQUQWCY5A4IgXTJh7Nwi92tAflR801bY7opwKXllMf5xsLapz9T/jMxtlOP0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=bur.io; spf=pass smtp.mailfrom=bur.io; dkim=pass (2048-bit key) header.d=bur.io header.i=@bur.io header.b=LeMsaGg4; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=uiUooHH+; arc=none smtp.client-ip=103.168.172.151 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=bur.io Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bur.io Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bur.io header.i=@bur.io header.b="LeMsaGg4"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="uiUooHH+" Received: from phl-compute-04.internal (phl-compute-04.internal [10.202.2.44]) by mailfout.phl.internal (Postfix) with ESMTP id 7FFECEC0574; Thu, 5 Mar 2026 12:45:29 -0500 (EST) Received: from phl-frontend-04 ([10.202.2.163]) by phl-compute-04.internal (MEProxy); Thu, 05 Mar 2026 12:45:29 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bur.io; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm2; t=1772732729; x=1772819129; bh=hLBgVNsWIP OVeePzwIQYkboGhQzrYTjvXhyciHZ6hLE=; b=LeMsaGg4xCxd16Xpu/xyjywNZS Af75n44d2llL4gMP+W6djQ56BvgmI8Fvh1XuMHSI82h+vzqeYtxaQlA9qiI3+Y4D qiHcefAHh272VTxlGYcrjL8tYPwzMoLco3NCjVXGvxJ9x7ZdPRFAor9vkAABt9if SNQ3WZQdll0toQ0xBzOoy9e7hNFg2Sq+GNLzrnKin4crfDTQBpmROFkgoJv9RGrh 7N9LElOA8VN6+FgQnf1SWuNR+SletTpF/bVwdU1Er/DArruBxntpb7PtRs5UlV8p pURw6PjcEkCyb8GYzKCO1OyCawBnX2qedDV2K4OZjH2gvdQrGfLu4JBqUJUw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t= 1772732729; x=1772819129; bh=hLBgVNsWIPOVeePzwIQYkboGhQzrYTjvXhy ciHZ6hLE=; b=uiUooHH+4+WaK02KNotaJrAX7qdmc8ZeRB37JsPFFgCO37ouPfU c5qko8PpUEvKN7yRUz4eWqI3V6e45su3E7KERZlIqKpNQud5W8VNwNBEognx4MIk WO70rwXGD2QstF8Tym6hX1kiQIsVGnvPkW0LOhqUuf7wxBASwhBKQWqS63nYyAjx KZQN5WuYOTnC9GiC2vYux4GKFR2C0cHYLuo9JZGuBvOvAIyzwSSl6f44+Y2N67Po FfcTMxhs5ug0DQpMR+Ksk6aTuGskf0Aq7fm5u2gbwy8O5SRgaiUuMe542ENcclHa K7zBe7co/XgVfjB/PPAX3gfTgRfoaq9zoTA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefgedrtddtgddvieejtdduucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu rghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf gurhepfffhvfevuffkfhggtggujgesthdtredttddtvdenucfhrhhomhepuehorhhishcu uehurhhkohhvuceosghorhhishessghurhdrihhoqeenucggtffrrghtthgvrhhnpeekvd ekffejleelhfevhedvjeduhfejtdfhvdevieeiiedugfeugfdtjefgfeeljeenucevlhhu shhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegsohhrihhssegsuh hrrdhiohdpnhgspghrtghpthhtohepfedpmhhouggvpehsmhhtphhouhhtpdhrtghpthht ohepughsthgvrhgsrgesshhushgvrdgtiidprhgtphhtthhopeifqhhusehsuhhsvgdrtg homhdprhgtphhtthhopehlihhnuhigqdgsthhrfhhssehvghgvrhdrkhgvrhhnvghlrdho rhhg X-ME-Proxy: Feedback-ID: i083147f8:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 5 Mar 2026 12:45:28 -0500 (EST) Date: Thu, 5 Mar 2026 09:46:09 -0800 From: Boris Burkov To: David Sterba Cc: Qu Wenruo , linux-btrfs@vger.kernel.org Subject: Re: [PATCH RFC] btrfs: get rid of btrfs_(alloc|free)_compr_folio() Message-ID: <20260305174609.GC926642@zen.localdomain> References: <20260305025611.GC5735@twin.jikos.cz> Precedence: bulk X-Mailing-List: linux-btrfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260305025611.GC5735@twin.jikos.cz> On Thu, Mar 05, 2026 at 03:56:11AM +0100, David Sterba wrote: > On Mon, Mar 02, 2026 at 06:30:30PM +1030, Qu Wenruo wrote: > > [GLOBAL POOL] > > Btrfs has maintained a global (per-module) pool of folios for compressed > > read/writes. > > > > That pool is maintained by a LRU list of cached folios, with a shrinker > > to free all cached folios when needed. > > > > The function btrfs_alloc_compr_folio() will try to grab any existing > > folio from that LRU list first, and go regular folio allocation if that > > list is empty. > > > > And btrfs_free_compr_folio() will try to put the folio into the LRU list > > if the current cache level is below the threshold (256 pages). > > > > [EXISTING LIMITS] > > Since the global pool is per-module, we have no way to support different > > folio sizes. This limit is already affecting bs > ps support, so that bs > > > ps cases just by-pass that global pool completely. > > > > Another limit is for bs < ps cases, which is more common. > > In that case we always allocate a full page (can be as large as 64K) for > > the compressed bio, this can be very wasteful if the on-disk extent is > > pretty small. > > > > [POTENTIAL BUGS] > > Recently David is reporting bugs related to compression: > > > > - List corruption on that LRU list > > > > - Folio ref count reaching zero at btrfs_free_compr_folio > > > > Although I haven't yet found a concrete chain of how this happened, I'm > > suspecting the usage of folio->lru is different from page->lru. > > Under most cases a lot of folio members are 1:1 mapped to page members, > > but I have not seen any driver/fs using folio->lru for their internal > > usage. (There are users of page->lru though) > > The pool worked well for pages and it is possible that the ->lru has > different semantics that do not apply the same for folios. With the > support for large folios and bs < ps the pool lost its appeal. > > > [REMOVAL] > > Personally I'm not a huge fan of maintaining a local folio list just for > > compression. > > > > I believe MM has seen much more pressure and can still properly give us > > folios/pages. > > I do not think we are doing it better than MM, so let's just use regular > > folio allocation instead. > > While in principle we should not try to be smarter than the allocator > there's a still a difference when we don't have to use it at all when > there's memory pressure that wants to write out data. The allocation is > a place where it can stall or increase the pressure on the rest of the > system. > > When we cycle through a few pages in the pool we can write lots of data. > Returning pages to allocator does not guarantee we'll get them back. > Placing that just to the compression path was an exception because there > should ideally be no allocation required on the writeout path. As > there's no generic layer that would do that transparently it's btrfs > being nice at a small cost of a few pages. > > So I stand by the reason of the pool but with the evolution of folios > and bs < ps the cost could be too high. I can still see the pool for a > subset of the combinations for some common scenario like 4K block size > on 64K page host (e.g. ARM). > FWIW, we do already see issues with allocation in compressed IO in production, so I am hesitant to support this change. On the one hand, we have the issues so the pool is not completely saving us anyway. On the other hand, removing it seems likely to make this worse in the way that you predict, Dave. If we do move forward with this, I will try to watch such errors closely on the first release of a kernel without the pool. Thanks, Boris > > And hopefully this will address David's recent crash (as usual I'm not > > able to reproduce locally). > > I'll run the test with this patch.