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

[Pedro Falcato <pfalcato@suse.de>]

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
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 -
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?

> +
> +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.

> +
> +		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
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.

> +};
> +#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.

> + * @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.")

>  } __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.

> +}
> +
> +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.

_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?
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.

>  } __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.

>  			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?

>  	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.

>  
> +#ifdef CONFIG_SPLIT_I_MMAP
> +static inline void i_mmap_nest_lock(struct address_space *mapping,
> +				struct rw_semaphore *lock)
> +{
> +	int i;
> +
> +	for (i = 0; i < split_tree_num; i++)
> +		down_write_nest_lock(&mapping->i_mmap[i]->rwsem, lock);
> +}
> +#else
> +static inline void i_mmap_nest_lock(struct address_space *mapping,
> +				struct rw_semaphore *lock)
> +{
> +	down_write_nest_lock(&mapping->i_mmap_rwsem, lock);
> +}
> +#endif
> +
>  static void vm_lock_mapping(struct mm_struct *mm, struct address_space *mapping)
>  {
>  	if (!test_bit(AS_MM_ALL_LOCKS, &mapping->flags)) {
> @@ -2178,7 +2211,7 @@ static void vm_lock_mapping(struct mm_struct *mm, struct address_space *mapping)
>  		 */
>  		if (test_and_set_bit(AS_MM_ALL_LOCKS, &mapping->flags))
>  			BUG();
> -		down_write_nest_lock(&mapping->i_mmap_rwsem, &mm->mmap_lock);
> +		i_mmap_nest_lock(mapping, &mm->mmap_lock);
>  	}
>  }
>  
> @@ -2489,6 +2522,7 @@ static int __mmap_new_file_vma(struct mmap_state *map,
>  	int error;
>  
>  	vma->vm_file = map->file;
> +	vma_set_tree_idx(vma);
>  	if (!map->file_doesnt_need_get)
>  		get_file(map->file);
>  
> diff --git a/mm/vma_init.c b/mm/vma_init.c
> index 3c0b65950510..c115e33d4812 100644
> --- a/mm/vma_init.c
> +++ b/mm/vma_init.c
> @@ -72,6 +72,9 @@ static void vm_area_init_from(const struct vm_area_struct *src,
>  #ifdef CONFIG_NUMA
>  	dest->vm_policy = src->vm_policy;
>  #endif
> +#ifdef CONFIG_SPLIT_I_MMAP
> +	dest->tree_idx = src->tree_idx;
> +#endif
>  #ifdef __HAVE_PFNMAP_TRACKING
>  	dest->pfnmap_track_ctx = NULL;
>  #endif

-- 
Pedro