From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8A828CF319D for ; Wed, 2 Oct 2024 07:41:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1E6886B027C; Wed, 2 Oct 2024 03:41:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 197DC6B027E; Wed, 2 Oct 2024 03:41:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 05F3D6B027F; Wed, 2 Oct 2024 03:41:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id D41B46B027C for ; Wed, 2 Oct 2024 03:41:39 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 7205340A4A for ; Wed, 2 Oct 2024 07:41:39 +0000 (UTC) X-FDA: 82627867518.16.C48E968 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) by imf26.hostedemail.com (Postfix) with ESMTP id 3DF4F14000A for ; Wed, 2 Oct 2024 07:41:36 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=infradead.org header.s=bombadil.20210309 header.b=lzRx7vNX; spf=none (imf26.hostedemail.com: domain of BATV+9f088ec7695a4423afd3+7710+infradead.org+hch@bombadil.srs.infradead.org has no SPF policy when checking 198.137.202.133) smtp.mailfrom=BATV+9f088ec7695a4423afd3+7710+infradead.org+hch@bombadil.srs.infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727854858; h=from:from:sender: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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=RDkEkc0w+gYwp7xSlV1KZ6Rf1mdE1Ttpc76UXPk+EK0=; b=yW0/n4Ab2flZHJa5GMYQTmsJhezCojzaaZGmVlJGppH0AsYzJPHAsOc66NxqfRvuAOPE7T qPslC2i8Pnr0YibkcfdpremK2jHBVqcH0QqbY9IzBBiF5EGzOI9R+if4OfT3wCSTGbk4Bb htrYBFREVoSvU8xZUEECgYlkmyxf86g= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=infradead.org header.s=bombadil.20210309 header.b=lzRx7vNX; spf=none (imf26.hostedemail.com: domain of BATV+9f088ec7695a4423afd3+7710+infradead.org+hch@bombadil.srs.infradead.org has no SPF policy when checking 198.137.202.133) smtp.mailfrom=BATV+9f088ec7695a4423afd3+7710+infradead.org+hch@bombadil.srs.infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727854858; a=rsa-sha256; cv=none; b=4v040YX35jYHTWkiE9oP9vghTDbjYztky7l/t+XS5anB/Y1TiA2gzoDydpIl4frRfzayuo kY0P7kYhX2OqVi0CkwtQTYxVW9m4BcwdDIKYWOmNKyecQCoUGFoL/73dIhA4mcBqZKZhIq YNzyiyvSMonUSaaAeBkQDZLS+kefku0= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=RDkEkc0w+gYwp7xSlV1KZ6Rf1mdE1Ttpc76UXPk+EK0=; b=lzRx7vNXDb9uuvdCBx97PXZQ/A agm1KxT936uEQFHFBu2E+I5SY6LT3G3t4TKMzOJtCHb0iwOTVQlUMIz55jSwjUaUSyMu9SNCBvqa7 pj4cWPSkbdwY3vilUHNPEjIJkDcgABu/8/cXuApHaRC9a1U0rOS9zPGKTZRBEKk0IQ62pLf/AI0op CArZikmzzLX4YeNTmtr9k+uG9WQ+bFCTQRLhb6liFM88u6uyQXST9Cz82UCa+UdZ9/VHWfJ3eWNX8 1XJwIOsNETnJTpRyAZFpdyXXPt71kszy+ZOKzFI09v9OOO4gNfsRX0RtGBiQfNHBqBP8Uthg2WAzh vy7a74+g==; Received: from hch by bombadil.infradead.org with local (Exim 4.98 #2 (Red Hat Linux)) id 1svtzF-000000053Dd-2pDW; Wed, 02 Oct 2024 07:41:29 +0000 Date: Wed, 2 Oct 2024 00:41:29 -0700 From: Christoph Hellwig To: Qu Wenruo Cc: Christoph Hellwig , Qu Wenruo , linux-btrfs@vger.kernel.org, hannes@cmpxchg.org, mhocko@kernel.org, roman.gushchin@linux.dev, shakeel.butt@linux.dev, muchun.song@linux.dev, akpm@linux-foundation.org, cgroups@vger.kernel.org, linux-mm@kvack.org, Michal Hocko , "Vlastimil Babka (SUSE)" Subject: Re: [PATCH] btrfs: root memcgroup for metadata filemap_add_folio() Message-ID: References: <5d3f4dca-f7f3-4228-8645-ad92c7a1e5ac@gmx.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5d3f4dca-f7f3-4228-8645-ad92c7a1e5ac@gmx.com> X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org. See http://www.infradead.org/rpr.html X-Rspam-User: X-Stat-Signature: nk8fk111mt1zjihkk7i1ez1s6839hnan X-Rspamd-Queue-Id: 3DF4F14000A X-Rspamd-Server: rspam11 X-HE-Tag: 1727854896-987541 X-HE-Meta: U2FsdGVkX1/uZgcdGvHf+ztUXM4N3/xnAsO8fUVwOgGTt6BXnsPkVsXMZYy6rcjFmdRZpaW9+HunwMl1r1UuE65TDBwF6/FLQmOyfMkKoH87fXUvf9HqJv3/cx/UEqvkyEK1ZCgWxacs1XjaU5jG9ZOMjb99wpaslOQg/Csk7znZyvKpq8fyXwfa+RDaWwGMPlaAZO+EuR4ts5KDU13/9Fmyy56StG5Rp7yDW1dnNHg5flhMhgP0HaH7hNUrjIqp4twKzaqobiUUzpeJ3hl4E6lShUCijtEArki4R71CY9CV+Z8ZqL83GZDXDM7Uozg9awAPb4G7UXAGw1qBiICG24ezObQcpZi0MpSNhn4L3z6K8e3afug5B80FUTGeFZ4NMY9l9z1IGzkvPyQ3fRWnM/3EK4kJAslqzt8Y/cvFmiGdS5hz50y5JHKUQEZIEStrh347klHzne00aMPdlG7vhiDzOo/PePpXaDQpQ67byAVXsxHl7UiM+8ymS3VHr3pgcrbDRo9mmfByTI+fqCmCTRoU7SGWOkFgSuRmFrbv6kTKpwu3df3pNmjmvAEA88o4Mtrp3VrnQlWc6DbSXShGsHQDP7pil84Vv6dHjQmSgdp0SV1r+h1jOU0ND/4frJ827mLhF5mODxHg82AB7B5U/FqGCA0K922vAqexrors5ZFWz8rWXy1F/768JWLA1yKf3dr2LRW5WmVfIyJfDV04f5gRh6/rf04FfmPUTi6PKsEpgCX6kJxmVcXaXBdtGn0sz3emD91G4rw0GJt3bEWW/MyFjjS3vTT3fKVZSCDgDEYvhH5i1CV1Na4ow1z1LQCYO3hMaRJ50xpt7MaKdiIOJo1rEDRIgmQESAKenTHvlm0L5h1HHqKy6A3g/Fq+p+tWkv9YnHg/yNcHQNGSDBcIZuNpFmVASmwu4Ac6HC5gd4jX1nhf/NuAcILVrZwUXxiUnDCuPDI9uEfF19dcrpK mKYtQgVE W+TOhse/jBbhL7vJvCJmvYVSIPCBN2OwErUGAFWaYxAIHODjpgoKa6RWB9vtH4PaCQ04t759RpQNEZomHShTulExtLVOncoSDx8CkLLgBN55kPuwOwvuAs8mcwgB4LbTNtIPliAOpaUnVuHCGQBQplqLe+tkJCNYW9BM2bJ9OitFzPOtfO9ampwyjP3CoN5NJURj6u0Fw4OxCfikCfSQ5T8GYxJ0HMiKJwNIq7r9SIr8SXrx1V1dDbUB5DmJLgGZZkdqRMZOo1tcHSWfTn/xgspFeT5/VNLyDHPI4W9/YEC6g5z3o+Y1zlvQeXsB0L3H5WaGSeiDxD04gV3th1d6wGidnqr8dDCj6YrQV94XjC6Ur5PcraGmihKpuIQgVnZfWLOHHftPxr9aPeE5WBcaAEBpYA7SG005WwvR0zV+kfyVZj1nZhYt3QY6CreKUb/8GRE7WRlduuz328r3nTpcM++cPjk4Fr8fCPulntdzxW31xvh3UsfTmhe/7f0uUMTlqbH3TT+gosbucm/yRHV1pp+6cng== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue, Oct 01, 2024 at 07:10:07PM +0930, Qu Wenruo wrote: > > This looks pretty ugly. What speaks against a version of > > filemap_add_folio that doesn't charge the memcg? > > > > Because there is so far only one caller has such requirement. That is a good argument to review the reasons for an interface, but not a killer argument. > Furthermore I believe the folio API doesn't prefer too many different > functions doing similar things. > > E.g. the new folio interfaces only provides filemap_get_folio(), > filemap_lock_folio(), and the more generic __filemap_get_folio(). > > Meanwhile there are tons of page based interfaces, find_get_page(), > find_or_create_page(), find_lock_page() and flags version etc. That's a totally different argument, tough. Those functions were trivial wrappers around a more versatile low-level function. While this is about adding clearly defined functionality, and more importantly not exporting totally random low-level data. What I'd propose is something like the patch below, plus proper documentation. Note that this now does the uncharge on the unlocked folio in the error case. From a quick look that should be fine, but someone who actually knows the code needs to confirm that. diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index 68a5f1ff3301c6..70da62cf32f6c3 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -1284,6 +1284,8 @@ int add_to_page_cache_lru(struct page *page, struct address_space *mapping, pgoff_t index, gfp_t gfp); int filemap_add_folio(struct address_space *mapping, struct folio *folio, pgoff_t index, gfp_t gfp); +int filemap_add_folio_nocharge(struct address_space *mapping, + struct folio *folio, pgoff_t index, gfp_t gfp); void filemap_remove_folio(struct folio *folio); void __filemap_remove_folio(struct folio *folio, void *shadow); void replace_page_cache_folio(struct folio *old, struct folio *new); diff --git a/mm/filemap.c b/mm/filemap.c index 36d22968be9a1e..0a1ae841e8c10f 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -958,20 +958,15 @@ noinline int __filemap_add_folio(struct address_space *mapping, } ALLOW_ERROR_INJECTION(__filemap_add_folio, ERRNO); -int filemap_add_folio(struct address_space *mapping, struct folio *folio, - pgoff_t index, gfp_t gfp) +int filemap_add_folio_nocharge(struct address_space *mapping, + struct folio *folio, pgoff_t index, gfp_t gfp) { void *shadow = NULL; int ret; - ret = mem_cgroup_charge(folio, NULL, gfp); - if (ret) - return ret; - __folio_set_locked(folio); ret = __filemap_add_folio(mapping, folio, index, gfp, &shadow); if (unlikely(ret)) { - mem_cgroup_uncharge(folio); __folio_clear_locked(folio); } else { /* @@ -989,6 +984,22 @@ int filemap_add_folio(struct address_space *mapping, struct folio *folio, } return ret; } +EXPORT_SYMBOL_GPL(filemap_add_folio_nocharge); + +int filemap_add_folio(struct address_space *mapping, struct folio *folio, + pgoff_t index, gfp_t gfp) +{ + int ret; + + ret = mem_cgroup_charge(folio, NULL, gfp); + if (ret) + return ret; + + ret = filemap_add_folio_nocharge(mapping, folio, index, gfp); + if (ret) + mem_cgroup_uncharge(folio); + return ret; +} EXPORT_SYMBOL_GPL(filemap_add_folio); #ifdef CONFIG_NUMA