[Minios-devel] [UNIKRAFT VFS PATCH v2 1/4] Fix dentry referecing and file closing

[Mihai Pogonaru <pogonarumihai@xxxxxxxxx>]

Increase the refcount of the vfscore_file's dentry when opening it.
This is the OSv behaviour since each vfscore_file has an intrusive_ptr
ref to its dentry, which increases dentry's refcount on creation and
decreases dentry's refcount on deletion. We must increase the dentry's
refcount since we might end up with files pointing to deleted dentries,
if the actual file/directory the dentry refers gets deleted (so the
dentry's refcount hits 0 and it gets freed).

We must also decrease the refcount of the dentry on file closing.

The statement "Dentry stays forever in the dentry cache. Unless the
file/directory it refers to gets deleted/renamed" doesn't hold for
file systems that do not save their structures on permanent mediums
(like LWIP's sockets abstraction) so the dentry refcount is equal to
the number of vfscore_file's pointing at it (so the dentry gets
deleted as soon as the last file is closed).

Right now the vfscore_file is never freed, excepting the LWIP socket's
file which is freed inside sock_net_close along with its dentry and
vnode. This should not be the expected behaviour from a underlying
file system (ramfs provides a good example of expected behaviour).
As such, we must free the vfscore_file when closing it and release its
dentry so the dentry and vnode freeing will be handled by vfs as well.

This patch also makes fdrop return 0 if the file's refcount hits
0 or return 1 otherwise. This is the OSv behaviour and the return
of an error from fdrop is never checked.

Signed-off-by: Mihai Pogonaru <pogonarumihai@xxxxxxxxx>
---
 lib/vfscore/file.c     | 18 ++++++++++++++----
 lib/vfscore/fops.c     |  7 ++++---
 lib/vfscore/syscalls.c |  3 +++
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/lib/vfscore/file.c b/lib/vfscore/file.c
index e56f16dc..e7627e96 100644
--- a/lib/vfscore/file.c
+++ b/lib/vfscore/file.c
@@ -42,15 +42,25 @@
 
 int fdrop(struct vfscore_file *fp)
 {
-       int ret = 0;
        int prev = ukarch_dec(&fp->f_count);
 
        if (prev == 0)
                UK_CRASH("Unbalanced fhold/fdrop");
 
-       if (prev == 1)
-               ret = vfs_close(fp);
-       return ret;
+       if (prev == 1) {
+               /*
+                * we free the file even in case of an error
+                * so release the dentry too
+                */
+               if (vfs_close(fp) != 0)
+                       drele(fp->f_dentry);
+
+               free(fp);
+
+               return 1;
+       }
+
+       return 0;
 }
 void fhold(struct vfscore_file *fp)
 {
diff --git a/lib/vfscore/fops.c b/lib/vfscore/fops.c
index 8f5c2e1b..0cef2c12 100644
--- a/lib/vfscore/fops.c
+++ b/lib/vfscore/fops.c
@@ -53,9 +53,10 @@ int vfs_close(struct vfscore_file *fp)
        if (error)
                return error;
 
-       /* Dentry stays forever in the dentry cache. Unless the
-        * file/directory it refers to gets deleted/renamed */
+       /* Release the dentry */
+       drele(fp->f_dentry);
        fp->f_dentry = NULL;
+
        return 0;
 }
 
@@ -103,7 +104,7 @@ int vfs_write(struct vfscore_file *fp, struct uio *uio, int 
flags)
                ioflags |= IO_SYNC;
 
        if ((flags & FOF_OFFSET) == 0)
-               uio->uio_offset = fp->f_offset;
+               uio->uio_offset = fp->f_offset;
 
        error = VOP_WRITE(vp, uio, ioflags);
        if (!error) {
diff --git a/lib/vfscore/syscalls.c b/lib/vfscore/syscalls.c
index 385d90c0..f9666c4c 100644
--- a/lib/vfscore/syscalls.c
+++ b/lib/vfscore/syscalls.c
@@ -204,6 +204,8 @@ sys_open(char *path, int flags, mode_t mode, struct 
vfscore_file **fpp)
        }
        fp->f_flags = flags;
 
+       // OSv was using a intrusive_ptr which was increasing the refcount
+       dref(dp);
        // change to std::move once dp is a dentry_ref
        fp->f_dentry = dp;
        dp = NULL;
@@ -214,6 +216,7 @@ sys_open(char *path, int flags, mode_t mode, struct 
vfscore_file **fpp)
                // Note direct delete of fp instead of fdrop(fp). fp was never
                // returned so cannot be in use, and because it wasn't opened
                // it cannot be close()ed.
+               drele(fp->f_dentry);
                free(fp);
                return error;
        }
-- 
2.11.0


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel