gfs2: Clean up after gfs2_create_inode rework
authorAndreas Gruenbacher <agruenba@redhat.com>
Sun, 4 Dec 2022 02:48:52 +0000 (03:48 +0100)
committerAndreas Gruenbacher <agruenba@redhat.com>
Tue, 6 Dec 2022 15:06:31 +0000 (16:06 +0100)
Since commit 3d36e57ff768 ("gfs2: gfs2_create_inode rework"),
gfs2_evict_inode() and gfs2_create_inode() / gfs2_inode_lookup() will
synchronize via the inode hash table and we can be certain that once a
new inode is inserted into the inode hash table(), gfs2_evict_inode()
has completely destroyed any previous versions.  We no longer need to
worry about overlapping inode object lifespans.  Update the code and
comments accordingly.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
fs/gfs2/glock.h
fs/gfs2/super.c

index 76cd2fabc668cf55af042051a9a6799c217fa330..d561126cfb477ba7770ef1b632125ef60e9ab4c8 100644 (file)
@@ -322,20 +322,6 @@ static inline void glock_set_object(struct gfs2_glock *gl, void *object)
 /**
  * glock_clear_object - clear the gl_object field of a glock
  * @gl: the glock
- * @object: the object
- *
- * I'd love to similarly add this:
- *     else if (gfs2_assert_warn(gl->gl_sbd, gl->gl_object == object))
- *             gfs2_dump_glock(NULL, gl, true);
- * Unfortunately, that's not possible because as soon as gfs2_delete_inode
- * frees the block in the rgrp, another process can reassign it for an I_NEW
- * inode in gfs2_create_inode because that calls new_inode, not gfs2_iget.
- * That means gfs2_delete_inode may subsequently try to call this function
- * for a glock that's already pointing to a brand new inode. If we clear the
- * new inode's gl_object, we'll introduce metadata corruption. Function
- * gfs2_delete_inode calls clear_inode which calls gfs2_clear_inode which also
- * tries to clear gl_object, so it's more than just gfs2_delete_inode.
- *
  */
 static inline void glock_clear_object(struct gfs2_glock *gl, void *object)
 {
index 075fad8fb1d1e046bd71ec867231c010b9f53594..02f1b5f2d7f227e5e394cf8367167e46d0b1a6b9 100644 (file)
@@ -1304,14 +1304,21 @@ static int evict_unlinked_inode(struct inode *inode)
                        goto out;
        }
 
-       /* We're about to clear the bitmap for the dinode, but as soon as we
-          do, gfs2_create_inode can create another inode at the same block
-          location and try to set gl_object again. We clear gl_object here so
-          that subsequent inode creates don't see an old gl_object. */
-       if (ip->i_gl) {
-               glock_clear_object(ip->i_gl, ip);
+       if (ip->i_gl)
                gfs2_inode_remember_delete(ip->i_gl, ip->i_no_formal_ino);
-       }
+
+       /*
+        * As soon as we clear the bitmap for the dinode, gfs2_create_inode()
+        * can get called to recreate it, or even gfs2_inode_lookup() if the
+        * inode was recreated on another node in the meantime.
+        *
+        * However, inserting the new inode into the inode hash table will not
+        * succeed until the old inode is removed, and that only happens after
+        * ->evict_inode() returns.  The new inode is attached to its inode and
+        *  iopen glocks after inserting it into the inode hash table, so at
+        *  that point we can be sure that both glocks are unused.
+        */
+
        ret = gfs2_dinode_dealloc(ip);
 out:
        return ret;