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. If we are to have this for all filesystems, then let's do that properly and make it generic functionality from its introduction.
