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 X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6E782C433B4 for ; Wed, 5 May 2021 01:13:38 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id DB86B613C4 for ; Wed, 5 May 2021 01:13:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DB86B613C4 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=fromorbit.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 420728D0001; Tue, 4 May 2021 21:13:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3F7638D0003; Tue, 4 May 2021 21:13:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2B73C8D0001; Tue, 4 May 2021 21:13:37 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0059.hostedemail.com [216.40.44.59]) by kanga.kvack.org (Postfix) with ESMTP id 0F1CD8D0001 for ; Tue, 4 May 2021 21:13:37 -0400 (EDT) Received: from smtpin40.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 9F347180AD5C5 for ; Wed, 5 May 2021 01:13:36 +0000 (UTC) X-FDA: 78105404832.40.4C93BA4 Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by imf06.hostedemail.com (Postfix) with ESMTP id 73CD9C0007C3 for ; Wed, 5 May 2021 01:13:37 +0000 (UTC) Received: from dread.disaster.area (pa49-179-143-157.pa.nsw.optusnet.com.au [49.179.143.157]) by mail108.syd.optusnet.com.au (Postfix) with ESMTPS id C72B31AF823; Wed, 5 May 2021 11:13:32 +1000 (AEST) Received: from dave by dread.disaster.area with local (Exim 4.92.3) (envelope-from ) id 1le66V-003pXr-Mo; Wed, 05 May 2021 11:13:31 +1000 Date: Wed, 5 May 2021 11:13:31 +1000 From: Dave Chinner To: Muchun Song Cc: Roman Gushchin , Matthew Wilcox , Andrew Morton , Johannes Weiner , Michal Hocko , Vladimir Davydov , Shakeel Butt , Yang Shi , alexs@kernel.org, Alexander Duyck , Wei Yang , linux-fsdevel , LKML , Linux Memory Management List Subject: Re: [External] Re: [PATCH 0/9] Shrink the list lru size on memory cgroup removal Message-ID: <20210505011331.GM1872259@dread.disaster.area> References: <20210428094949.43579-1-songmuchun@bytedance.com> <20210430004903.GF1872259@dread.disaster.area> <20210430032739.GG1872259@dread.disaster.area> <20210502235843.GJ1872259@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.3 cv=Tu+Yewfh c=1 sm=1 tr=0 a=I9rzhn+0hBG9LkCzAun3+g==:117 a=I9rzhn+0hBG9LkCzAun3+g==:17 a=kj9zAlcOel0A:10 a=5FLXtPjwQuUA:10 a=7-415B0cAAAA:8 a=sqVQysubuN6EyXWE9wQA:9 a=CjuIK1q_8ugA:10 a=biEYGPWJfzWAr4FL6Ov7:22 Authentication-Results: imf06.hostedemail.com; dkim=none; dmarc=none; spf=none (imf06.hostedemail.com: domain of david@fromorbit.com has no SPF policy when checking 211.29.132.59) smtp.mailfrom=david@fromorbit.com X-Stat-Signature: at1hqsrgfb48ixwc1nq5uma68u1nykd8 X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 73CD9C0007C3 Received-SPF: none (fromorbit.com>: No applicable sender policy available) receiver=imf06; identity=mailfrom; envelope-from=""; helo=mail108.syd.optusnet.com.au; client-ip=211.29.132.59 X-HE-DKIM-Result: none/none X-HE-Tag: 1620177217-745146 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, May 03, 2021 at 02:33:21PM +0800, Muchun Song wrote: > On Mon, May 3, 2021 at 7:58 AM Dave Chinner wrote: > > > If the user wants to insert the allocated object to its lru list in > > > the feature. The > > > user should use list_lru_kmem_cache_alloc() instead of kmem_cache_alloc(). > > > I have looked at the code closely. There are 3 different kmem_caches that > > > need to use this new API to allocate memory. They are inode_cachep, > > > dentry_cache and radix_tree_node_cachep. I think that it is easy to migrate. > > > > It might work, but I think you may have overlooked the complexity > > of inode allocation for filesystems. i.e. alloc_inode() calls out > > to filesystem allocation functions more often than it allocates > > directly from the inode_cachep. i.e. Most filesystems provide > > their own ->alloc_inode superblock operation, and they allocate > > inodes out of their own specific slab caches, not the inode_cachep. > > I didn't realize this before. You are right. Most filesystems > have their own kmem_cache instead of inode_cachep. > We need a lot of filesystems special to be changed. > Thanks for your reminder. > > > > > And then you have filesystems like XFS, where alloc_inode() will > > never be called, and implement ->alloc_inode as: > > > > /* Catch misguided souls that try to use this interface on XFS */ > > STATIC struct inode * > > xfs_fs_alloc_inode( > > struct super_block *sb) > > { > > BUG(); > > return NULL; > > } > > > > Because all the inode caching and allocation is internal to XFS and > > VFS inode management interfaces are not used. > > > > So I suspect that an external wrapper function is not the way to go > > here - either internalising the LRU management into the slab > > allocation or adding the memcg code to alloc_inode() and filesystem > > specific routines would make a lot more sense to me. > > Sure. If we introduce kmem_cache_alloc_lru, all filesystems > need to migrate to kmem_cache_alloc_lru. I cannot figure out > an approach that does not need to change filesystems code. Right, I don't think there's a way to avoid changing all the filesystem code if we are touching the cache allocation routines. However, if we hide it all inside the allocation routine, then the changes to each filesystem is effectively just a 1-liner like: - inode = kmem_cache_alloc(inode_cache, GFP_NOFS); + inode = kmem_cache_alloc_lru(inode_cache, sb->s_inode_lru, GFP_NOFS); Or perhaps, define a generic wrapper function like: static inline void * alloc_inode_sb(struct superblock *sb, struct kmem_cache *cache, gfp_flags_t gfp) { return kmem_cache_alloc_lru(cache, sb->s_inode_lru, gfp); } And then each filesystem ends up with: - inode = kmem_cache_alloc(inode_cache, GFP_NOFS); + inode = alloc_inode_sb(sb, inode_cache, GFP_NOFS); so that all the superblock LRU stuff is also hidden from the filesystems... Cheers, Dave. -- Dave Chinner david@fromorbit.com