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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6F513C433F5 for ; Tue, 9 Nov 2021 01:18:45 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 065C66120A for ; Tue, 9 Nov 2021 01:18:44 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 065C66120A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=fromorbit.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 7F5F36B0071; Mon, 8 Nov 2021 20:18:44 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7A4706B0072; Mon, 8 Nov 2021 20:18:44 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6BB826B0073; Mon, 8 Nov 2021 20:18:44 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0117.hostedemail.com [216.40.44.117]) by kanga.kvack.org (Postfix) with ESMTP id 5D3276B0071 for ; Mon, 8 Nov 2021 20:18:44 -0500 (EST) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 25B2C18033E53 for ; Tue, 9 Nov 2021 01:18:44 +0000 (UTC) X-FDA: 78787632294.05.2B0FD0F Received: from mail107.syd.optusnet.com.au (mail107.syd.optusnet.com.au [211.29.132.53]) by imf12.hostedemail.com (Postfix) with ESMTP id 8CFA21001500 for ; Tue, 9 Nov 2021 01:18:43 +0000 (UTC) Received: from dread.disaster.area (pa49-195-103-97.pa.nsw.optusnet.com.au [49.195.103.97]) by mail107.syd.optusnet.com.au (Postfix) with ESMTPS id 36ED110498F2; Tue, 9 Nov 2021 12:18:39 +1100 (AEDT) Received: from dave by dread.disaster.area with local (Exim 4.92.3) (envelope-from ) id 1mkFmX-006Yyg-Kx; Tue, 09 Nov 2021 12:18:37 +1100 Date: Tue, 9 Nov 2021 12:18:37 +1100 From: Dave Chinner To: Matthew Wilcox Cc: Mina Almasry , Michal Hocko , Theodore Ts'o , Greg Thelen , Shakeel Butt , Andrew Morton , Hugh Dickins , Roman Gushchin , Johannes Weiner , Tejun Heo , Vladimir Davydov , riel@surriel.com, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, cgroups@vger.kernel.org Subject: Re: [PATCH v1 1/5] mm/shmem: support deterministic charging of tmpfs Message-ID: <20211109011837.GF418105@dread.disaster.area> References: <20211108211959.1750915-1-almasrymina@google.com> <20211108211959.1750915-2-almasrymina@google.com> <20211108221047.GE418105@dread.disaster.area> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.4 cv=e9dl9Yl/ c=1 sm=1 tr=0 ts=6189cc72 a=fP9RlOTWD4uZJjPSFnn6Ew==:117 a=fP9RlOTWD4uZJjPSFnn6Ew==:17 a=kj9zAlcOel0A:10 a=vIxV3rELxO4A:10 a=7-415B0cAAAA:8 a=Om1FtI2GXO6ST26aLOMA:9 a=CjuIK1q_8ugA:10 a=biEYGPWJfzWAr4FL6Ov7:22 X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 8CFA21001500 X-Stat-Signature: 4cg547n4gmed6h17r1w6cat44bwboqc9 Authentication-Results: imf12.hostedemail.com; dkim=none; dmarc=none; spf=none (imf12.hostedemail.com: domain of david@fromorbit.com has no SPF policy when checking 211.29.132.53) smtp.mailfrom=david@fromorbit.com X-HE-Tag: 1636420723-483575 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: On Mon, Nov 08, 2021 at 11:41:51PM +0000, Matthew Wilcox wrote: > On Tue, Nov 09, 2021 at 09:10:47AM +1100, Dave Chinner wrote: > > > + rcu_read_lock(); > > > + memcg = rcu_dereference(mapping->host->i_sb->s_memcg_to_charge); > > > > Anything doing pointer chasing to obtain static, unchanging > > superblock state is poorly implemented. The s_memcg_to_charge value never > > changes, so this code should associate the memcg to charge directly > > on the mapping when the mapping is first initialised by the > > filesystem. We already do this with things like attaching address > > space ops and mapping specific gfp masks (i.e > > mapping_set_gfp_mask()), so this association should be set up that > > way, too (e.g. mapping_set_memcg_to_charge()). > > I'm not a fan of enlarging struct address_space with another pointer > unless it's going to be used by all/most filesystems. If this is > destined to be a shmem-only feature, then it should be in the > shmem_inode instead of the mapping. Neither am I, but I'm also not a fan of the filemap code still having to drill through the mapping to the host inode just to check if it needs to do special stuff for shmem inodes on every call that adds a page to the page cache. This is just as messy and intrusive and the memcg code really has no business digging about in the filesystem specific details of the inode behind the mapping. Hmmm. The mem_cgroup_charge() call in filemap_add_folio() passes a null mm context, so deep in the guts it ends getting the memcg from active_memcg() in get_mem_cgroup_from_mm(). That ends up using current->active_memcg, so maybe a better approach here is to have shmem override current->active_memcg via set_active_memcg() before it enters the generic fs paths and restore it on return... current_fsmemcg()? > If we are to have this for all filesystems, then let's do that properly > and make it generic functionality from its introduction. Fully agree. Cheers, Dave. -- Dave Chinner david@fromorbit.com