Re: [PATCH v2 3/4] mm/fs: split the file's i_mmap tree

[Huang Shijie <huangsj@hygon.cn>]

On Thu, Jun 11, 2026 at 12:11:27PM +0100, Pedro Falcato wrote:
> Hi,
> 
> On Thu, Jun 11, 2026 at 02:18:59PM +0800, Huang Shijie wrote:
> > In the UnixBench tests, there is a test "execl" which tests
> > the execve system call.
> >   For example, a Hygon's server has 12 NUMA nodes, and 384 CPUs.
> > When we test our server with "./Run -c 384 execl",
> > the test result is not good enough. The i_mmap locks contended heavily on
> > "libc.so" and "ld.so". The i_mmap tree for "libc.so" can be
> > over 6000 VMAs, all the VMAs can be in different NUMA mode. The insert/remove
> > operations do not run quickly enough.
> 
> I _really_ would have appreciated some coordination here, because I said I was
> going to take a look at it. I have something that I think is much simpler
Okay, no problem. 

I waited for more then a month, I thought you are busy at other
things. So I spent more then a week to finish the patch set v2.


> in practice. These patches are also way too complex to be dropped just before
> the merge window.
> 
> Some comments:
> 
> > 
> >  In order to reduce the competition of the i_mmap lock, this patch does
> > following:
> >    1.) Split the single i_mmap tree into several sibling trees:
> >        Each tree has a lock. The CONFIG_SPLIT_I_MMAP is used to
> >        turn on/off this feature.
> 
> There is no need for a config option. This needs to Just Work.
> 
> >    2.) Introduce a new field "tree_idx" for vm_area_struct to save the
> >        sibling tree index for this VMA.
> 
> This is possibly contentious, but there are holes in vm_area_struct.
> So I think this is fine.
> 
> >    3.) Introduce a new field "vma_count" for address_space.
> >        The new mapping_mapped() will use it.
> >    4.) Rewrite the vma_interval_tree_foreach()
> >    5.) Rewrite the lock functions.	
> > 
> >  After this patch, the VMA insert/remove operations will work faster,
> > and we can get over 400% performance improvement with the above test.
> > 
> > Signed-off-by: Huang Shijie <huangsj@hygon.cn>
> > ---
> >  fs/Kconfig               |   8 ++
> >  fs/hugetlbfs/inode.c     |  20 ++++-
> >  fs/inode.c               |  75 ++++++++++++++++-
> >  include/linux/fs.h       | 174 ++++++++++++++++++++++++++++++++++++++-
> >  include/linux/mm.h       |  80 ++++++++++++++++++
> >  include/linux/mm_types.h |   3 +
> >  mm/internal.h            |   3 +-
> >  mm/mmap.c                |  11 ++-
> >  mm/nommu.c               |  23 ++++--
> >  mm/pagewalk.c            |   2 +-
> >  mm/vma.c                 |  72 +++++++++++-----
> >  mm/vma_init.c            |   3 +
> >  12 files changed, 436 insertions(+), 38 deletions(-)
> > 
> > diff --git a/fs/Kconfig b/fs/Kconfig
> > index 43cb06de297f..e24804f70432 100644
> > --- a/fs/Kconfig
> > +++ b/fs/Kconfig
> > @@ -9,6 +9,14 @@ menu "File systems"
> >  config DCACHE_WORD_ACCESS
> >         bool
> >  
> > +config SPLIT_I_MMAP
> > +	bool "Split the file's i_mmap to several trees"
> > +	default n
> > +	help
> > +	  Split the file's i_mmap to several trees, each tree has a separate
> > +	  lock. This will reduce the lock contention of file's i_mmap tree,
> > +	  but it will cost more memory for per inode.
> > +
> >  config VALIDATE_FS_PARSER
> >  	bool "Validate filesystem parameter description"
> >  	help
> > diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> > index da5b41ea5bdd..68d8308418dd 100644
> > --- a/fs/hugetlbfs/inode.c
> > +++ b/fs/hugetlbfs/inode.c
> > @@ -891,6 +891,23 @@ static struct inode *hugetlbfs_get_root(struct super_block *sb,
> >   */
> >  static struct lock_class_key hugetlbfs_i_mmap_rwsem_key;
> >  
> > +#ifdef CONFIG_SPLIT_I_MMAP
> > +static void hugetlbfs_lockdep_set_class(struct address_space *mapping)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < split_tree_num; i++) {
> > +		lockdep_set_class(&mapping->i_mmap[i].rwsem,
> > +				&hugetlbfs_i_mmap_rwsem_key);
> > +	}
> > +}
> > +#else
> > +static void hugetlbfs_lockdep_set_class(struct address_space *mapping)
> > +{
> > +	lockdep_set_class(&mapping->i_mmap_rwsem, &hugetlbfs_i_mmap_rwsem_key);
> > +}
> > +#endif
> > +
> >  static struct inode *hugetlbfs_get_inode(struct super_block *sb,
> >  					struct mnt_idmap *idmap,
> >  					struct inode *dir,
> > @@ -915,8 +932,7 @@ static struct inode *hugetlbfs_get_inode(struct super_block *sb,
> >  
> >  		inode->i_ino = get_next_ino();
> >  		inode_init_owner(idmap, inode, dir, mode);
> > -		lockdep_set_class(&inode->i_mapping->i_mmap_rwsem,
> > -				&hugetlbfs_i_mmap_rwsem_key);
> > +		hugetlbfs_lockdep_set_class(inode->i_mapping);
> >  		inode->i_mapping->a_ops = &hugetlbfs_aops;
> >  		simple_inode_init_ts(inode);
> >  		info->resv_map = resv_map;
> > diff --git a/fs/inode.c b/fs/inode.c
> > index 62c579a0cf7d..cb67ae83f5b3 100644
> > --- a/fs/inode.c
> > +++ b/fs/inode.c
> > @@ -214,6 +214,70 @@ static int no_open(struct inode *inode, struct file *file)
> >  	return -ENXIO;
> >  }
> >  
> > +#ifdef CONFIG_SPLIT_I_MMAP
> > +int split_tree_num;
> > +static int split_tree_align __maybe_unused = 32;
> > +
> > +static void __init init_split_tree_num(void)
> > +{
> > +#ifdef CONFIG_NUMA
> > +	split_tree_num = nr_node_ids;
> > +#else
> > +	split_tree_num = ALIGN(nr_cpu_ids, split_tree_align);
> > +#endif
> > +}
> 
> Again, too configurable. I think you're too stuck up on the NUMA case -

If you do not care about the NUMA. The performance will _NOT_ get improved
in our NUMA server. I had ever tested code which do not care about the NUMA,
and I got a bad performance. Avoid the remote access is a very important
thing for the NUMA server.

> which does not matter for many people - and may actively harm NUMA users. If
> I have a 128 core 2 NUMA node system, what should I shard by?
It is easy to extend the tree number for NUMA. :)

For the 128 core 2 NUMA, we can extend to more trees, such as:
   Two trees for each NUMA node.

> 
> > +
> > +static void free_mapping_i_mmap(struct address_space *mapping)
> > +{
> > +	int i;
> > +
> > +	if (!mapping->i_mmap)
> > +		return;
> > +
> > +	for (i = 0; i < split_tree_num; i++)
> > +		kfree(mapping->i_mmap[i]);
> > +
> > +	kfree(mapping->i_mmap);
> > +	mapping->i_mmap = NULL;
> > +}
> > +
> > +static int init_mapping_i_mmap(struct address_space *mapping, gfp_t gfp)
> > +{
> > +	struct i_mmap_tree *tree;
> > +	int i;
> > +
> > +	/* The extra one is used as terminator in vma_interval_tree_foreach() */
> > +	mapping->i_mmap = kzalloc(sizeof(tree) * (split_tree_num + 1), gfp);
> > +	if (!mapping->i_mmap)
> > +		return -ENOMEM;
> > +
> > +	for (i = 0; i < split_tree_num; i++) {
> > +		tree = kzalloc_node(sizeof(*tree), gfp, i);
> > +		if (!tree)
> > +			goto nomem;
> > +
> > +		tree->root = RB_ROOT_CACHED;
> > +		init_rwsem(&tree->rwsem);
> 
> This (as-is) should blow up with lockdep + the locking loops down there.
okay, I will check it later.

thanks a lot.
> 
> > +
> > +		mapping->i_mmap[i] = tree;
> > +	}
> > +	return 0;
> > +nomem:
> > +	free_mapping_i_mmap(mapping);
> > +	return -ENOMEM;
> > +}
> 
> Honestly, it's likely that a simple static array in struct address_space
The array size is not fixed, so we cannot add a static array in address_space.

> suffices. I would not go through the trouble of getting everything very
> tight and NUMA correct.
> 
> > +#else
> > +static int init_mapping_i_mmap(struct address_space *mapping, gfp_t gfp)
> > +{
> > +	mapping->i_mmap = RB_ROOT_CACHED;
> > +	init_rwsem(&mapping->i_mmap_rwsem);
> > +	return 0;
> > +}
> > +
> > +static void free_mapping_i_mmap(struct address_space *mapping) { }
> > +static void __init init_split_tree_num(void) {}
> > +#endif
> > +
> >  /**
> >   * inode_init_always_gfp - perform inode structure initialisation
> >   * @sb: superblock inode belongs to
> > @@ -302,9 +366,14 @@ int inode_init_always_gfp(struct super_block *sb, struct inode *inode, gfp_t gfp
> >  #endif
> >  	inode->i_flctx = NULL;
> >  
> > -	if (unlikely(security_inode_alloc(inode, gfp)))
> > +	if (init_mapping_i_mmap(mapping, gfp))
> >  		return -ENOMEM;
> >  
> > +	if (unlikely(security_inode_alloc(inode, gfp))) {
> > +		free_mapping_i_mmap(mapping);
> > +		return -ENOMEM;
> > +	}
> > +
> >  	this_cpu_inc(nr_inodes);
> >  
> >  	return 0;
> > @@ -380,6 +449,7 @@ void __destroy_inode(struct inode *inode)
> >  	if (inode->i_default_acl && !is_uncached_acl(inode->i_default_acl))
> >  		posix_acl_release(inode->i_default_acl);
> >  #endif
> > +	free_mapping_i_mmap(&inode->i_data);
> >  	this_cpu_dec(nr_inodes);
> >  }
> >  EXPORT_SYMBOL(__destroy_inode);
> > @@ -480,9 +550,7 @@ EXPORT_SYMBOL(inc_nlink);
> >  static void __address_space_init_once(struct address_space *mapping)
> >  {
> >  	xa_init_flags(&mapping->i_pages, XA_FLAGS_LOCK_IRQ | XA_FLAGS_ACCOUNT);
> > -	init_rwsem(&mapping->i_mmap_rwsem);
> >  	spin_lock_init(&mapping->i_private_lock);
> > -	mapping->i_mmap = RB_ROOT_CACHED;
> >  }
> >  
> >  void address_space_init_once(struct address_space *mapping)
> > @@ -2619,6 +2687,7 @@ void __init inode_init(void)
> >  					&i_hash_mask,
> >  					0,
> >  					0);
> > +	init_split_tree_num();
> >  }
> >  
> >  void init_special_inode(struct inode *inode, umode_t mode, dev_t rdev)
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index cd46615b8f53..f4b3645b61df 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -450,6 +450,25 @@ struct mapping_metadata_bhs {
> >  	struct list_head list;	/* The list of bhs (b_assoc_buffers) */
> >  };
> >  
> > +#ifdef CONFIG_SPLIT_I_MMAP
> > +/*
> > + * struct i_mmap_tree - A single sibling tree of the file's split i_mmap.
> > + * @root: The red/black interval tree root.
> > + * @rwsem: Protects insert/remove operations on this sibling tree.
> > + * @vma_count: Number of VMAs in this sibling tree.
> > + *
> > + * When CONFIG_SPLIT_I_MMAP is enabled, the file's single i_mmap tree is
> > + * split into split_tree_num sibling trees, each with its own lock. This
> > + * reduces lock contention by allowing concurrent VMA insert/remove
> > + * operations on different sibling trees.
> > + */
> > +struct i_mmap_tree {
> > +	struct rb_root_cached	root;
> > +	struct rw_semaphore	rwsem;
> > +	atomic_t		vma_count;
> 
> I don't see what you need this vma_count for? I get the one in address_space,
> but this one does not seem useful.
For non-NUMA case, we can use it to determine which tree we should put the new
VMA.
Round-robin is not good enough for a dynamic system.

> 
> > +};
> > +#endif
> > +
> >  /**
> >   * struct address_space - Contents of a cacheable, mappable object.
> >   * @host: Owner, either the inode or the block_device.
> > @@ -461,8 +480,13 @@ struct mapping_metadata_bhs {
> >   * @gfp_mask: Memory allocation flags to use for allocating pages.
> >   * @i_mmap_writable: Number of VM_SHARED, VM_MAYWRITE mappings.
> >   * @nr_thps: Number of THPs in the pagecache (non-shmem only).
> > - * @i_mmap: Tree of private and shared mappings.
> > - * @i_mmap_rwsem: Protects @i_mmap and @i_mmap_writable.
> > + * @i_mmap: Tree of private and shared mappings. When CONFIG_SPLIT_I_MMAP
> > + *   is enabled, this is an array of split_tree_num struct i_mmap_tree
> > + *   pointers (plus a NULL terminator).
> 
> NULL terminator wastes more memory, so I would really strongly avoid it as
> well.
any better idea?

> 
> > + * @vma_count: Total number of VMAs across all sibling trees (only when
> > + *   CONFIG_SPLIT_I_MMAP is enabled). Used by mapping_mapped().
> > + * @i_mmap_rwsem: Protects @i_mmap and @i_mmap_writable (only when
> > + *   CONFIG_SPLIT_I_MMAP is disabled; otherwise per-tree rwsem is used).
> 
> So, there are very good reasons why you still need an i_mmap_rwsem protecting
> state, even with split mmap trees. Which I'll go into later.
> 
> >   * @nrpages: Number of page entries, protected by the i_pages lock.
> >   * @writeback_index: Writeback starts here.
> >   * @a_ops: Methods.
> > @@ -480,14 +504,19 @@ struct address_space {
> >  	/* number of thp, only for non-shmem files */
> >  	atomic_t		nr_thps;
> >  #endif
> > +#ifdef CONFIG_SPLIT_I_MMAP
> > +	struct i_mmap_tree	**i_mmap;
> > +	atomic_t		vma_count;
> > +#else
> >  	struct rb_root_cached	i_mmap;
> > +	struct rw_semaphore	i_mmap_rwsem;
> > +#endif
> >  	unsigned long		nrpages;
> >  	pgoff_t			writeback_index;
> >  	const struct address_space_operations *a_ops;
> >  	unsigned long		flags;
> >  	errseq_t		wb_err;
> >  	spinlock_t		i_private_lock;
> > -	struct rw_semaphore	i_mmap_rwsem;
> 
> See d3b1a9a778e1 ("fs/address_space: move i_mmap_rwsem to mitigate a false sharing with i_mmap.")
Got it.
> 
> >  } __attribute__((aligned(sizeof(long)))) __randomize_layout;
> >  	/*
> >  	 * On most architectures that alignment is already the case; but
> > @@ -508,6 +537,133 @@ static inline bool mapping_tagged(const struct address_space *mapping, xa_mark_t
> >  	return xa_marked(&mapping->i_pages, tag);
> >  }
> >  
> > +#ifdef CONFIG_SPLIT_I_MMAP
> > +static inline int mapping_mapped(const struct address_space *mapping)
> > +{
> > +	return	atomic_read(&mapping->vma_count);
> 
> Now that I think of it, I don't think we need atomic_t, only unsigned long +
> READ_ONCE() suffices. Increments can race just fine, we don't expect any 
> consistency there - if you want consistency you probably hold the i_mmap lock.
> 
okay. I will check it.

> > +}
> > +
> > +static inline void inc_mapping_vma(struct address_space *mapping,
> > +				struct vm_area_struct *vma)
> > +{
> > +	struct i_mmap_tree *tree = mapping->i_mmap[vma->tree_idx];
> > +
> > +	atomic_inc(&tree->vma_count);
> > +	atomic_inc(&mapping->vma_count);
> > +}
> > +
> > +static inline void dec_mapping_vma(struct address_space *mapping,
> > +				struct vm_area_struct *vma)
> > +{
> > +	struct i_mmap_tree *tree = mapping->i_mmap[vma->tree_idx];
> > +
> > +	atomic_dec(&tree->vma_count);
> > +	atomic_dec(&mapping->vma_count);
> > +}
> 
> This probably shouldn't be in linux/fs.h.
> 
> > +
> > +static inline struct rb_root_cached *get_i_mmap_root(struct address_space *mapping)
> > +{
> > +	return (struct rb_root_cached *)mapping->i_mmap;
> > +}
> > +
> > +static inline void i_mmap_tree_lock_write(struct address_space *mapping,
> > +					struct vm_area_struct *vma)
> > +{
> > +	struct i_mmap_tree *tree = mapping->i_mmap[vma->tree_idx];
> > +
> > +	down_write(&tree->rwsem);
> > +}
> > +
> > +static inline void i_mmap_tree_unlock_write(struct address_space *mapping,
> > +					struct vm_area_struct *vma)
> > +{
> > +	struct i_mmap_tree *tree = mapping->i_mmap[vma->tree_idx];
> > +
> > +	up_write(&tree->rwsem);
> > +}
> > +
> > +#define i_mmap_lock_write_prepare(mapping)
> > +#define i_mmap_unlock_write_complete(mapping)
> 
> It's unclear to me why you added write_prepare() and write_complete().
> 
> > +
> > +extern int split_tree_num;
> > +static inline void i_mmap_lock_write(struct address_space *mapping)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < split_tree_num; i++)
> > +		down_write(&mapping->i_mmap[i]->rwsem);
> 
> Oof, this is an incredibly large hammer. This is basically why I think keeping
> i_mmap_rwsem (in a different form) is required. You do not want to take $nr_cpus
> locks (read _or_ write). For my design, I keep i_mmap_rwsem, but I invert its
> meaning - taking it in write = I'm reading from the tree; taking it in read =
> I'm writing to the tree. This provides some lighter-weight exclusion between
> rmap walks and rmap tree manipulation.
okay, it seem your method is better. I am waiting for your patch.

> 
> _Technically_, you shouldn't need to always take a lock when manipulating the
> tree. A pattern like mnt_hold_writers()/mnt_get_write_access() can probably
> work well. But it may be too complex ATM.
> 
> 
> Also, note that you pretty much do not want i_mmap_lock_write() users after
> the conversion is done.
> 
> > +}
> > +
> > +static inline int i_mmap_trylock_write(struct address_space *mapping)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < split_tree_num; i++) {
> > +		if (!down_write_trylock(&mapping->i_mmap[i]->rwsem)) {
> > +			while (i--)
> > +				up_write(&mapping->i_mmap[i]->rwsem);
> > +			return 0;
> > +		}
> > +	}
> > +	return 1;
> > +}
> > +
> > +static inline void i_mmap_unlock_write(struct address_space *mapping)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < split_tree_num; i++)
> > +		up_write(&mapping->i_mmap[i]->rwsem);
> > +}
> > +
> > +static inline int i_mmap_trylock_read(struct address_space *mapping)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < split_tree_num; i++) {
> > +		if (!down_read_trylock(&mapping->i_mmap[i]->rwsem)) {
> > +			while (i--)
> > +				up_read(&mapping->i_mmap[i]->rwsem);
> > +			return 0;
> > +		}
> > +	}
> > +	return 1;
> > +}
> > +
> > +static inline void i_mmap_lock_read(struct address_space *mapping)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < split_tree_num; i++)
> > +		down_read(&mapping->i_mmap[i]->rwsem);
> > +}
> > +
> > +static inline void i_mmap_unlock_read(struct address_space *mapping)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < split_tree_num; i++)
> > +		up_read(&mapping->i_mmap[i]->rwsem);
> > +}
> > +
> > +static inline void i_mmap_assert_locked(struct address_space *mapping)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < split_tree_num; i++)
> > +		lockdep_assert_held(&mapping->i_mmap[i]->rwsem);
> > +}
> > +
> > +static inline void i_mmap_assert_write_locked(struct address_space *mapping)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < split_tree_num; i++)
> > +		lockdep_assert_held_write(&mapping->i_mmap[i]->rwsem);
> > +}
> > +
> > +#else
> > +
> >  static inline void i_mmap_lock_write(struct address_space *mapping)
> >  {
> >  	down_write(&mapping->i_mmap_rwsem);
> > @@ -561,6 +717,18 @@ static inline struct rb_root_cached *get_i_mmap_root(struct address_space *mappi
> >  	return &mapping->i_mmap;
> >  }
> >  
> > +static inline void inc_mapping_vma(struct address_space *mapping,
> > +				struct vm_area_struct *vma) { }
> > +static inline void dec_mapping_vma(struct address_space *mapping,
> > +				struct vm_area_struct *vma) { }
> > +
> > +#define i_mmap_lock_write_prepare(mapping)	i_mmap_lock_write(mapping)
> > +#define i_mmap_unlock_write_complete(mapping)	i_mmap_unlock_write(mapping)
> > +#define i_mmap_tree_lock_write(mapping, vma)
> > +#define i_mmap_tree_unlock_write(mapping, vma)
> > +
> > +#endif
> > +
> >  /*
> >   * Might pages of this file have been modified in userspace?
> >   * Note that i_mmap_writable counts all VM_SHARED, VM_MAYWRITE vmas: do_mmap
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 0a45c6a8b9f2..9aa8119fa9bf 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -4041,11 +4041,91 @@ struct vm_area_struct *vma_interval_tree_iter_first(struct rb_root_cached *root,
> >  struct vm_area_struct *vma_interval_tree_iter_next(struct vm_area_struct *node,
> >  				unsigned long start, unsigned long last);
> >  
> > +#ifdef CONFIG_SPLIT_I_MMAP
> > +extern int split_tree_num;
> > +
> > +static inline int smallest_tree_idx(struct file *file)
> > +{
> > +	struct address_space *mapping = file->f_mapping;
> > +	int tmp = INT_MAX, count;
> > +	int i, j = 0;
> > +
> > +	/*
> > +	 * Since a not 100% accurate value is still okay,
> > +	 * we do not need any lock here.
> > +	 */
> > +	for (i = 0; i < split_tree_num; i++) {
> > +		count = atomic_read(&mapping->i_mmap[i]->vma_count);
> > +		if (count < tmp) {
> > +			j = i;
> > +			tmp = count;
> > +			if (!tmp)
> > +				break;
> > +		}
> > +	}
> 
> Ohh, I see why you want the per-subtree vma_count now. But is this a net-win?
It keep the trees as even as possible.

> I think doing something like vma-pointer-hashing or just smp_processor_id()
> would work a-ok.
> 
> > +	return j;
> > +}
> > +
> > +static inline void vma_set_tree_idx(struct vm_area_struct *vma)
> > +{
> > +#ifdef CONFIG_NUMA
> > +	vma->tree_idx = numa_node_id();
> > +#else
> > +	vma->tree_idx = smallest_tree_idx(vma->vm_file);
> > +#endif
> > +}
> > +
> > +static inline struct rb_root_cached *get_rb_root(struct vm_area_struct *vma,
> > +					struct address_space *mapping)
> > +{
> > +	return &mapping->i_mmap[vma->tree_idx]->root;
> > +}
> > +
> > +/* Find the first valid VMA in the sibling trees */
> > +static inline struct vm_area_struct *first_vma(struct i_mmap_tree ***__r,
> > +				unsigned long start, unsigned long last)
> > +{
> > +	struct vm_area_struct *vma = NULL;
> > +	struct i_mmap_tree **tree = *__r;
> > +	struct rb_root_cached *root;
> > +
> > +	while (*tree) {
> > +		root = &(*tree)->root;
> > +		tree++;
> > +		vma = vma_interval_tree_iter_first(root, start, last);
> > +		if (vma)
> > +			break;
> > +	}
> > +
> > +	/* Save for the next loop */
> > +	*__r = tree;
> > +	return vma;
> > +}
> > +
> > +/*
> > + * Please use get_i_mmap_root() to get the @root.
> > + * @_tmp is referenced to avoid unused variable warning.
> > + */
> > +#define vma_interval_tree_foreach(vma, root, start, last)		\
> > +	for (struct i_mmap_tree **_r = (struct i_mmap_tree **)(root),	\
> > +		**_tmp = (vma = first_vma(&_r, start, last)) ? _r : NULL;\
> > +	     ((_tmp && vma) || (vma = first_vma(&_r, start, last)));	\
> > +		vma = vma_interval_tree_iter_next(vma, start, last))
> > +#else
> >  /* Please use get_i_mmap_root() to get the @root */
> >  #define vma_interval_tree_foreach(vma, root, start, last)		\
> >  	for (vma = vma_interval_tree_iter_first(root, start, last);	\
> >  	     vma; vma = vma_interval_tree_iter_next(vma, start, last))
> >  
> > +static inline void vma_set_tree_idx(struct vm_area_struct *vma) { }
> > +
> > +static inline struct rb_root_cached *get_rb_root(struct vm_area_struct *vma,
> > +					struct address_space *mapping)
> > +{
> > +	return &mapping->i_mmap;
> > +}
> > +#endif
> > +
> >  void anon_vma_interval_tree_insert(struct anon_vma_chain *node,
> >  				   struct rb_root_cached *root);
> >  void anon_vma_interval_tree_remove(struct anon_vma_chain *node,
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index a308e2c23b82..8d6aab3346ce 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -1072,6 +1072,9 @@ struct vm_area_struct {
> >  #ifdef __HAVE_PFNMAP_TRACKING
> >  	struct pfnmap_track_ctx *pfnmap_track_ctx;
> >  #endif
> > +#ifdef CONFIG_SPLIT_I_MMAP
> > +	int tree_idx;			/* The sibling tree index for the VMA */
> > +#endif
> 
> FTR the struct hole isn't here, but right after vm_lock_seq or vm_refcnt in
> most configs.
okay, thanks.
I did not notice the struct hole issue.
> 
> >  } __randomize_layout;
> >  
> >  /* Clears all bits in the VMA flags bitmap, non-atomically. */
> > diff --git a/mm/internal.h b/mm/internal.h
> > index 5a2ddcf68e0b..2d35cacffd19 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -1888,7 +1888,8 @@ static inline void maybe_rmap_unlock_action(struct vm_area_struct *vma,
> >  
> >  	VM_WARN_ON_ONCE(vma_is_anonymous(vma));
> >  	file = vma->vm_file;
> > -	i_mmap_unlock_write(file->f_mapping);
> > +	i_mmap_tree_unlock_write(file->f_mapping, vma);
> > +	i_mmap_unlock_write_complete(file->f_mapping);
> >  	action->hide_from_rmap_until_complete = false;
> >  }
> >  
> > diff --git a/mm/mmap.c b/mm/mmap.c
> > index d714fdb357e5..70036ec9dcaa 100644
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -1825,15 +1825,20 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
> >  			struct address_space *mapping = file->f_mapping;
> >  
> >  			get_file(file);
> > -			i_mmap_lock_write(mapping);
> > +			i_mmap_lock_write_prepare(mapping);
> > +			i_mmap_tree_lock_write(mapping, mpnt);
> > +
> >  			if (vma_is_shared_maywrite(tmp))
> >  				mapping_allow_writable(mapping);
> >  			flush_dcache_mmap_lock(mapping);
> >  			/* insert tmp into the share list, just after mpnt */
> >  			vma_interval_tree_insert_after(tmp, mpnt,
> > -					get_i_mmap_root(mapping));
> > +					get_rb_root(mpnt, mapping));
> > +			inc_mapping_vma(mapping, tmp);
> 
> Honestly, would prefer to hide all of these details from mmap.
yes, we can. 

But we need to change the functions in mm/interval_tree.c

> 
> >  			flush_dcache_mmap_unlock(mapping);
> > -			i_mmap_unlock_write(mapping);
> > +
> > +			i_mmap_tree_unlock_write(mapping, mpnt);
> > +			i_mmap_unlock_write_complete(mapping);
> >  		}
> >  
> >  		if (!(tmp->vm_flags & VM_WIPEONFORK))
> > diff --git a/mm/nommu.c b/mm/nommu.c
> > index 0f18ffc658e9..1f2c60a220f6 100644
> > --- a/mm/nommu.c
> > +++ b/mm/nommu.c
> > @@ -567,11 +567,16 @@ static void setup_vma_to_mm(struct vm_area_struct *vma, struct mm_struct *mm)
> >  	if (vma->vm_file) {
> >  		struct address_space *mapping = vma->vm_file->f_mapping;
> >  
> > -		i_mmap_lock_write(mapping);
> > +		i_mmap_lock_write_prepare(mapping);
> > +		i_mmap_tree_lock_write(mapping, vma);
> > +
> >  		flush_dcache_mmap_lock(mapping);
> > -		vma_interval_tree_insert(vma, get_i_mmap_root(mapping));
> > +		vma_interval_tree_insert(vma, get_rb_root(vma, mapping));
> > +		inc_mapping_vma(mapping, vma);
> >  		flush_dcache_mmap_unlock(mapping);
> > -		i_mmap_unlock_write(mapping);
> > +
> > +		i_mmap_tree_unlock_write(mapping, vma);
> > +		i_mmap_unlock_write_complete(mapping);
> >  	}
> >  }
> >  
> > @@ -583,11 +588,16 @@ static void cleanup_vma_from_mm(struct vm_area_struct *vma)
> >  		struct address_space *mapping;
> >  		mapping = vma->vm_file->f_mapping;
> >  
> > -		i_mmap_lock_write(mapping);
> > +		i_mmap_lock_write_prepare(mapping);
> > +		i_mmap_tree_lock_write(mapping, vma);
> > +
> >  		flush_dcache_mmap_lock(mapping);
> > -		vma_interval_tree_remove(vma, get_i_mmap_root(mapping));
> > +		vma_interval_tree_remove(vma, get_rb_root(vma, mapping));
> > +		dec_mapping_vma(mapping, vma);
> >  		flush_dcache_mmap_unlock(mapping);
> > -		i_mmap_unlock_write(mapping);
> > +
> > +		i_mmap_tree_unlock_write(mapping, vma);
> > +		i_mmap_unlock_write_complete(mapping);
> >  	}
> >  }
> >  
> > @@ -1063,6 +1073,7 @@ unsigned long do_mmap(struct file *file,
> >  	if (file) {
> >  		region->vm_file = get_file(file);
> >  		vma->vm_file = get_file(file);
> > +		vma_set_tree_idx(vma);
> 
> This is unrelated, shouldn't be done here.
> 
> >  	}
> >  
> >  	down_write(&nommu_region_sem);
> > diff --git a/mm/pagewalk.c b/mm/pagewalk.c
> > index 8df1b5077951..d5745519d95a 100644
> > --- a/mm/pagewalk.c
> > +++ b/mm/pagewalk.c
> > @@ -809,7 +809,7 @@ int walk_page_mapping(struct address_space *mapping, pgoff_t first_index,
> >  	if (!check_ops_safe(ops))
> >  		return -EINVAL;
> >  
> > -	lockdep_assert_held(&mapping->i_mmap_rwsem);
> > +	i_mmap_assert_locked(mapping);
> 
> This kind of conversion should be done in a separate step.
> 
> >  	vma_interval_tree_foreach(vma, get_i_mmap_root(mapping), first_index,
> >  				  first_index + nr - 1) {
> >  		/* Clip to the vma */
> > diff --git a/mm/vma.c b/mm/vma.c
> > index 6159650c1b42..2055758064a9 100644
> > --- a/mm/vma.c
> > +++ b/mm/vma.c
> > @@ -234,22 +234,23 @@ static void __vma_link_file(struct vm_area_struct *vma,
> >  		mapping_allow_writable(mapping);
> >  
> >  	flush_dcache_mmap_lock(mapping);
> > -	vma_interval_tree_insert(vma, get_i_mmap_root(mapping));
> > +	vma_interval_tree_insert(vma, get_rb_root(vma, mapping));
> > +	inc_mapping_vma(mapping, vma);
> 
> inc_mapping_vma() should probably be done implicitly by insertion?
Yes, we can. 
It is more grace to hide it in vma_interval_tree_insert.

> 
> >  	flush_dcache_mmap_unlock(mapping);
> >  }
> >  
> > -/*
> > - * Requires inode->i_mapping->i_mmap_rwsem
> > - */
> >  static void __remove_shared_vm_struct(struct vm_area_struct *vma,
> >  				      struct address_space *mapping)
> >  {
> > +	i_mmap_tree_lock_write(mapping, vma);
> >  	if (vma_is_shared_maywrite(vma))
> >  		mapping_unmap_writable(mapping);
> >  
> >  	flush_dcache_mmap_lock(mapping);
> > -	vma_interval_tree_remove(vma, get_i_mmap_root(mapping));
> > +	vma_interval_tree_remove(vma, get_rb_root(vma, mapping));
> > +	dec_mapping_vma(mapping, vma);
> >  	flush_dcache_mmap_unlock(mapping);
> > +	i_mmap_tree_unlock_write(mapping, vma);
> >  }
> >  
> >  /*
> > @@ -297,8 +298,9 @@ static void vma_prepare(struct vma_prepare *vp)
> >  			uprobe_munmap(vp->adj_next, vp->adj_next->vm_start,
> >  				      vp->adj_next->vm_end);
> >  
> > -		i_mmap_lock_write(vp->mapping);
> > +		i_mmap_lock_write_prepare(vp->mapping);
> >  		if (vp->insert && vp->insert->vm_file) {
> > +			i_mmap_tree_lock_write(vp->mapping, vp->insert);
> >  			/*
> >  			 * Put into interval tree now, so instantiated pages
> >  			 * are visible to arm/parisc __flush_dcache_page
> > @@ -307,6 +309,7 @@ static void vma_prepare(struct vma_prepare *vp)
> >  			 */
> >  			__vma_link_file(vp->insert,
> >  					vp->insert->vm_file->f_mapping);
> > +			i_mmap_tree_unlock_write(vp->mapping, vp->insert);
> >  		}
> >  	}
> >  
> > @@ -318,12 +321,17 @@ static void vma_prepare(struct vma_prepare *vp)
> >  	}
> >  
> >  	if (vp->file) {
> > +		i_mmap_tree_lock_write(vp->mapping, vp->vma);
> >  		flush_dcache_mmap_lock(vp->mapping);
> >  		vma_interval_tree_remove(vp->vma,
> > -					get_i_mmap_root(vp->mapping));
> > -		if (vp->adj_next)
> > +					get_rb_root(vp->vma, vp->mapping));
> > +		dec_mapping_vma(vp->mapping, vp->vma);
> > +		if (vp->adj_next) {
> > +			i_mmap_tree_lock_write(vp->mapping, vp->adj_next);
> >  			vma_interval_tree_remove(vp->adj_next,
> > -					get_i_mmap_root(vp->mapping));
> > +					get_rb_root(vp->adj_next, vp->mapping));
> > +			dec_mapping_vma(vp->mapping, vp->adj_next);
> > +		}
> >  	}
> >  
> >  }
> > @@ -340,12 +348,17 @@ static void vma_complete(struct vma_prepare *vp, struct vma_iterator *vmi,
> >  			 struct mm_struct *mm)
> >  {
> >  	if (vp->file) {
> > -		if (vp->adj_next)
> > +		if (vp->adj_next) {
> >  			vma_interval_tree_insert(vp->adj_next,
> > -					get_i_mmap_root(vp->mapping));
> > +					get_rb_root(vp->adj_next, vp->mapping));
> > +			inc_mapping_vma(vp->mapping, vp->adj_next);
> > +			i_mmap_tree_unlock_write(vp->mapping, vp->adj_next);
> > +		}
> >  		vma_interval_tree_insert(vp->vma,
> > -					get_i_mmap_root(vp->mapping));
> > +					get_rb_root(vp->vma, vp->mapping));
> > +		inc_mapping_vma(vp->mapping, vp->vma);
> >  		flush_dcache_mmap_unlock(vp->mapping);
> > +		i_mmap_tree_unlock_write(vp->mapping, vp->vma);
> >  	}
> >  
> >  	if (vp->remove && vp->file) {
> > @@ -370,7 +383,7 @@ static void vma_complete(struct vma_prepare *vp, struct vma_iterator *vmi,
> >  	}
> >  
> >  	if (vp->file) {
> > -		i_mmap_unlock_write(vp->mapping);
> > +		i_mmap_unlock_write_complete(vp->mapping);
> >  
> >  		if (!vp->skip_vma_uprobe) {
> >  			uprobe_mmap(vp->vma);
> > @@ -1799,12 +1812,12 @@ static void unlink_file_vma_batch_process(struct unlink_vma_file_batch *vb)
> >  	int i;
> >  
> >  	mapping = vb->vmas[0]->vm_file->f_mapping;
> > -	i_mmap_lock_write(mapping);
> > +	i_mmap_lock_write_prepare(mapping);
> >  	for (i = 0; i < vb->count; i++) {
> >  		VM_WARN_ON_ONCE(vb->vmas[i]->vm_file->f_mapping != mapping);
> >  		__remove_shared_vm_struct(vb->vmas[i], mapping);
> >  	}
> > -	i_mmap_unlock_write(mapping);
> > +	i_mmap_unlock_write_complete(mapping);
> >  
> >  	unlink_file_vma_batch_init(vb);
> >  }
> > @@ -1836,10 +1849,13 @@ static void vma_link_file(struct vm_area_struct *vma, bool hold_rmap_lock)
> >  
> >  	if (file) {
> >  		mapping = file->f_mapping;
> > -		i_mmap_lock_write(mapping);
> > +		i_mmap_lock_write_prepare(mapping);
> > +		i_mmap_tree_lock_write(mapping, vma);
> >  		__vma_link_file(vma, mapping);
> > -		if (!hold_rmap_lock)
> > -			i_mmap_unlock_write(mapping);
> > +		if (!hold_rmap_lock) {
> > +			i_mmap_tree_unlock_write(mapping, vma);
> > +			i_mmap_unlock_write_complete(mapping);
> > +		}
> >  	}
> >  }
> >  
> > @@ -2164,6 +2180,23 @@ static void vm_lock_anon_vma(struct mm_struct *mm, struct anon_vma *anon_vma)
> >  	}
> >  }
> 
> I can but hope that all of the above is quite simplified before we get to the
> "making file rmap more complicated" bit.
:(
If we can do not care about the ARM device, we can make it simple.

Thanks
Huang Shijie