ソースを参照

[LibOS] Fix get_new_dentry reference count semantics

If `get_new_dentry` returns successfully, it increases returned dentry's
reference counter by one, which matches other similar functions' behavior.
borysp 4 年 前
コミット
289ac3af4d

+ 1 - 1
LibOS/shim/include/shim_fs.h

@@ -502,7 +502,7 @@ const char * dentry_get_name (struct shim_dentry * dent)
  * If hashptr is passed (as an optimization), this is a hash
  * of the name.
  *
- * If parent is non-null, the ref count is 1; else it is zero.
+ * If parent is non-null, the ref count is 2; else it is 1.
  *
  * This function also sets up both a name and a relative path
  */

+ 3 - 1
LibOS/shim/src/fs/shim_dcache.c

@@ -151,7 +151,7 @@ void put_dentry (struct shim_dentry * dent) {
  * If hashptr is passed (as an optimization), this is a hash
  * of the name.
  *
- * If parent is non-null, the ref count is 1; else it is zero.
+ * If parent is non-null, the ref count is 2; else it is 1.
  *
  * This function also sets up both a name and a relative path
  */
@@ -166,6 +166,8 @@ struct shim_dentry * get_new_dentry (struct shim_mount *mount,
     if (!dent)
         return NULL;
 
+    get_dentry(dent);
+
     if (hashptr) {
 #ifdef DEBUG
         // For debug builds, assert that the hash passed in is correct.

+ 1 - 5
LibOS/shim/src/fs/shim_fs.c

@@ -293,10 +293,7 @@ int __mount_fs (struct shim_mount * mount, struct shim_dentry * dent)
         if (ret < 0) {
             /* Try getting rid of ESKIPPED case */
             assert (ret != -ESKIPPED);
-            // TODO: `mount_root` leaks here, but fixing this would require
-            // fixing `get_new_dentry` semantics (its result has sometimes
-            // refcount set to 0).
-            // put_dentry(mount_root);
+            put_dentry(mount_root);
             return ret;
         }
         mount->root = mount_root;
@@ -457,7 +454,6 @@ int mount_fs (const char * type, const char * uri, const char * mount_point,
 
         if (!dent) {
             dent = get_new_dentry(mount, parent, last, last_len, NULL);
-            get_dentry(dent);
         }
     }
 

+ 15 - 8
LibOS/shim/src/fs/shim_namei.c

@@ -169,17 +169,17 @@ int lookup_dentry (struct shim_dentry * parent, const char * name, int namelen,
              * + name strings (see get_new_dentry), make sure it fits into qstr */
             if (parent->rel_path.len + 1 + namelen >= STR_SIZE) {  /* +1 for '/' */
                 debug("Relative path exceeds the limit %d\n", STR_SIZE);
-                return -ENAMETOOLONG;
+                err = -ENAMETOOLONG;
+                goto out;
             }
         }
 
         dent = get_new_dentry(fs, parent, name, namelen, NULL);
-        if (!dent)
-            return -ENOMEM;
+        if (!dent) {
+            err = -ENOMEM;
+            goto out;
+        }
         do_fs_lookup = 1;
-        // In the case we make a new dentry, go ahead and increment the
-        // ref count; in other cases, __lookup_dcache does this
-        get_dentry(dent);
     } else {
         if (!(dent->state & DENTRY_VALID))
             do_fs_lookup = 1;
@@ -206,7 +206,7 @@ int lookup_dentry (struct shim_dentry * parent, const char * name, int namelen,
 
                 /* Trying to weed out ESKIPPED */
                 assert(err != -ESKIPPED);
-                return err;
+                goto out;
             }
         }
         dent->state |= DENTRY_VALID;
@@ -220,8 +220,15 @@ int lookup_dentry (struct shim_dentry * parent, const char * name, int namelen,
     if (dent->state & DENTRY_NEGATIVE)
         err = -ENOENT;
 
-    if (new)
+    if (new) {
+        get_dentry(dent);
         *new = dent;
+    }
+
+out:
+    if (dent) {
+        put_dentry(dent);
+    }
 
     return err;
 }