On 10/18/2025 4:11 AM, Ackerley Tng wrote:
[...]

+static int kvm_gmem_init_inode(struct inode *inode, loff_t size, u64 flags)
+{
+       struct gmem_inode *gi = GMEM_I(inode);
+       MA_STATE(mas, &gi->attributes, 0, (size >> PAGE_SHIFT) - 1);
+       u64 attrs;
+       int r;
+
+       inode->i_op = &kvm_gmem_iops;
+       inode->i_mapping->a_ops = &kvm_gmem_aops;
+       inode->i_mode |= S_IFREG;
+       inode->i_size = size;
+       mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
+       mapping_set_inaccessible(inode->i_mapping);
+       /* Unmovable mappings are supposed to be marked unevictable as well. */
AS_UNMOVABLE has been removed and got merged into AS_INACCESSIBLE, not sure if
it's better to use "Inaccessible" instead of "Unmovable"

+       WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));
+
+       gi->flags = flags;
+
+       mt_set_external_lock(&gi->attributes,
+                            &inode->i_mapping->invalidate_lock);
+
+       /*
+        * Store default attributes for the entire gmem instance. Ensuring every
+        * index is represented in the maple tree at all times simplifies the
+        * conversion and merging logic.
+        */
+       attrs = gi->flags & GUEST_MEMFD_FLAG_INIT_SHARED ? 0 : 
KVM_MEMORY_ATTRIBUTE_PRIVATE;
+
+       /*
+        * Acquire the invalidation lock purely to make lockdep happy. There
+        * should be no races at this time since the inode hasn't yet been fully
+        * created.
+        */
+       filemap_invalidate_lock(inode->i_mapping);
+       r = mas_store_gfp(&mas, xa_mk_value(attrs), GFP_KERNEL);
+       filemap_invalidate_unlock(inode->i_mapping);
+
+       return r;
+}
+
[...]
@@ -925,13 +986,39 @@ static struct inode *kvm_gmem_alloc_inode(struct 
super_block *sb)

        mpol_shared_policy_init(&gi->policy, NULL);

+       /*
+        * Memory attributes are protected the filemap invalidation lock, but
                                     ^
                                protected by
+        * the lock structure isn't available at this time.  Immediately mark
+        * maple tree as using external locking so that accessing the tree
+        * before its fully initialized results in NULL pointer dereferences
+        * and not more subtle bugs.
+        */
+       mt_init_flags(&gi->attributes, MT_FLAGS_LOCK_EXTERN);
+
        gi->flags = 0;
        return &gi->vfs_inode;
  }

  static void kvm_gmem_destroy_inode(struct inode *inode)
  {
-       mpol_free_shared_policy(&GMEM_I(inode)->policy);
+       struct gmem_inode *gi = GMEM_I(inode);
+
+       mpol_free_shared_policy(&gi->policy);
+
+       /*
+        * Note!  Checking for an empty tree is functionally necessary to avoid
+        * explosions if the tree hasn't been initialized, i.e. if the inode is

It makes sense to skip __mt_destroy() when mtree is empty.
But what explosions it could trigger if mtree is empty?
It seems __mt_destroy() can handle the case if the external lock is not set.


+        * being destroyed before guest_memfd can set the external lock.
+        */
+       if (!mtree_empty(&gi->attributes)) {
+               /*
+                * Acquire the invalidation lock purely to make lockdep happy,
+                * the inode is unreachable at this point.
+                */
+               filemap_invalidate_lock(inode->i_mapping);
+               __mt_destroy(&gi->attributes);
+               filemap_invalidate_unlock(inode->i_mapping);
+       }
  }

  static void kvm_gmem_free_inode(struct inode *inode)
--
2.51.0.858.gf9c4a03a3a-goog


Reply via email to