Make fd tables work for files or chans
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 24 Jul 2015 12:30:18 +0000 (08:30 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Mon, 27 Jul 2015 15:53:33 +0000 (11:53 -0400)
The guts of insert_file, {get,put}_file_from_fd are reworked for
fd_tables such that they can handle either files or chans.  Whether you
use files or chans will be choosen mostly by the wrapper function (e.g.
insert_file for the VFS).

The invariant is that if a bit is set in the fdset, then file XOR chan
is set.

For operations like clone and close_fdt (closes all open files), we
don't really need wrappers, since that code is called from
VFS/9ns-independent parts of the kernel.

Note that clone_fdt has the chan incref commented out, but close_fdt
does not.  Since these aren't actually used by 9ns yet, the invariant
mentioned above isn't true yet.  I'll remove that shortly.

kern/include/vfs.h
kern/src/process.c
kern/src/syscall.c
kern/src/vfs.c

index 2e51d2e..59be8a8 100644 (file)
@@ -372,8 +372,10 @@ struct small_fd_set {
 
 /* Describes an open file.  We need this, since the FD flags are supposed to be
  * per file descriptor, not per file (like the file status flags). */
+struct chan;   /* from 9ns */
 struct file_desc {
        struct file                                     *fd_file;
+       struct chan                                     *fd_chan;
        unsigned int                            fd_flags;
 };
 
@@ -487,12 +489,17 @@ ssize_t kread_file(struct file *file, void *buf, size_t sz);
 void *kread_whole_file(struct file *file);
 
 /* Process-related File management functions */
+void *lookup_fd(struct fd_table *fdt, int fd, bool incref, bool vfs);
+int insert_obj_fdt(struct fd_table *fdt, void *obj, int low_fd, int fd_flags,
+                   bool must_use_low, bool vfs);
+bool close_fd(struct fd_table *fdt, int fd);
+void close_fdt(struct fd_table *open_files, bool cloexec);
+void clone_fdt(struct fd_table *src, struct fd_table *dst);
+
 struct file *get_file_from_fd(struct fd_table *open_files, int fd);
-struct file *put_file_from_fd(struct fd_table *open_files, int file_desc);
+void put_file_from_fd(struct fd_table *open_files, int file_desc);
 int insert_file(struct fd_table *open_files, struct file *file, int low_fd,
                 bool must, bool cloexec);
-void close_all_files(struct fd_table *open_files, bool cloexec);
-void clone_files(struct fd_table *src, struct fd_table *dst);
 int do_chdir(struct fs_struct *fs_env, char *path);
 int do_fchdir(struct fs_struct *fs_env, struct file *file);
 char *do_getcwd(struct fs_struct *fs_env, char **kfree_this, size_t cwd_l);
index 59038dc..97359b8 100644 (file)
@@ -379,7 +379,7 @@ error_t proc_alloc(struct proc **pp, struct proc *parent, int flags)
        p->open_files.open_fds = (struct fd_set*)&p->open_files.open_fds_init;
        if (parent) {
                if (flags & PROC_DUP_FGRP)
-                       clone_files(&parent->open_files, &p->open_files);
+                       clone_fdt(&parent->open_files, &p->open_files);
        } else {
                /* no parent, we're created from the kernel */
                int fd;
@@ -886,7 +886,7 @@ void proc_destroy(struct proc *p)
         * Also note that any mmap'd files will still be mmapped.  You can close the
         * file after mmapping, with no effect. */
        close_9ns_files(p, FALSE);
-       close_all_files(&p->open_files, FALSE);
+       close_fdt(&p->open_files, FALSE);
        /* Tell the ksched about our death, and which cores we freed up */
        __sched_proc_destroy(p, pc_arr, nr_cores_revoked);
        /* Tell our parent about our state change (to DYING) */
index dd79e92..b6abff1 100644 (file)
@@ -578,7 +578,7 @@ static int sys_proc_create(struct proc *p, char *path, size_t path_l,
        }
        /* close the CLOEXEC ones, even though this isn't really an exec */
        close_9ns_files(new_p, TRUE);
-       close_all_files(&new_p->open_files, TRUE);
+       close_fdt(&new_p->open_files, TRUE);
        /* Load the elf. */
        if (load_elf(new_p, program, argc, argv, envc, envp)) {
                set_errstr("Failed to load elf");
@@ -843,7 +843,7 @@ static int sys_exec(struct proc *p, char *path, size_t path_l,
        unmap_and_destroy_vmrs(p);
        /* close the CLOEXEC ones */
        close_9ns_files(p, TRUE);
-       close_all_files(&p->open_files, TRUE);
+       close_fdt(&p->open_files, TRUE);
        env_user_mem_free(p, 0, UMAPTOP);
        if (load_elf(p, program, argc, argv, envc, envp)) {
                kref_put(&program->f_kref);
index 0c95af2..1202735 100644 (file)
@@ -16,6 +16,7 @@
 #include <pmap.h>
 #include <umem.h>
 #include <smp.h>
+#include <ns.h>
 
 struct sb_tailq super_blocks = TAILQ_HEAD_INITIALIZER(super_blocks);
 spinlock_t super_blocks_lock = SPINLOCK_INITIALIZER;
@@ -2331,32 +2332,48 @@ void *kread_whole_file(struct file *file)
 
 /* Process-related File management functions */
 
-/* Given any FD, get the appropriate file, 0 o/w */
-struct file *get_file_from_fd(struct fd_table *open_files, int file_desc)
+/* Given any FD, get the appropriate object, 0 o/w.  Set vfs if you're looking
+ * for a file, o/w a chan.  Set incref if you want a reference count (which is a
+ * 9ns thing, you can't use the pointer if you didn't incref). */
+void *lookup_fd(struct fd_table *fdt, int fd, bool incref, bool vfs)
 {
-       struct file *retval = 0;
-       if (file_desc < 0)
+       void *retval = 0;
+       if (fd < 0)
                return 0;
-       spin_lock(&open_files->lock);
-       if (open_files->closed) {
-               spin_unlock(&open_files->lock);
+       spin_lock(&fdt->lock);
+       if (fdt->closed) {
+               spin_unlock(&fdt->lock);
                return 0;
        }
-       if (file_desc < open_files->max_fdset) {
-               if (GET_BITMASK_BIT(open_files->open_fds->fds_bits, file_desc)) {
+       if (fd < fdt->max_fdset) {
+               if (GET_BITMASK_BIT(fdt->open_fds->fds_bits, fd)) {
                        /* while max_files and max_fdset might not line up, we should never
                         * have a valid fdset higher than files */
-                       assert(file_desc < open_files->max_files);
-                       retval = open_files->fd[file_desc].fd_file;
-                       /* 9ns might be using this one, in which case file == 0 */
-                       if (retval)
-                               kref_get(&retval->f_kref, 1);
+                       assert(fd < fdt->max_files);
+                       if (vfs)
+                               retval = fdt->fd[fd].fd_file;
+                       else
+                               retval = fdt->fd[fd].fd_chan;
+                       /* retval could be 0 if we asked for the wrong one (e.g. it's a
+                        * file, but we asked for a chan) */
+                       if (retval && incref) {
+                               if (vfs)
+                                       kref_get(&((struct file*)retval)->f_kref, 1);
+                               else
+                                       chan_incref((struct chan*)retval);
+                       }
                }
        }
-       spin_unlock(&open_files->lock);
+       spin_unlock(&fdt->lock);
        return retval;
 }
 
+/* Given any FD, get the appropriate file, 0 o/w */
+struct file *get_file_from_fd(struct fd_table *open_files, int file_desc)
+{
+       return lookup_fd(open_files, file_desc, TRUE, TRUE);
+}
+
 /* Grow the vfs fd set */
 static int grow_fd_set(struct fd_table *open_files)
 {
@@ -2437,29 +2454,40 @@ int put_fd(struct fd_table *open_files, int file_desc)
        return 0;
 }
 
-/* Remove FD from the open files, if it was there, and return f.  Currently,
- * this decref's f, so the return value is not consumable or even usable.  This
- * hasn't been thought through yet. */
-struct file *put_file_from_fd(struct fd_table *open_files, int file_desc)
+/* If FD is in the group, remove it, decref it, and return TRUE. */
+bool close_fd(struct fd_table *fdt, int fd)
 {
-       struct file *file = 0;
-       if (file_desc < 0)
-               return 0;
-       spin_lock(&open_files->lock);
-       if (file_desc < open_files->max_fdset) {
-               if (GET_BITMASK_BIT(open_files->open_fds->fds_bits, file_desc)) {
+       struct file *file;
+       struct chan *chan;
+       bool ret = FALSE;
+       if (fd < 0)
+               return FALSE;
+       spin_lock(&fdt->lock);
+       if (fd < fdt->max_fdset) {
+               if (GET_BITMASK_BIT(fdt->open_fds->fds_bits, fd)) {
                        /* while max_files and max_fdset might not line up, we should never
                         * have a valid fdset higher than files */
-                       assert(file_desc < open_files->max_files);
-                       file = open_files->fd[file_desc].fd_file;
-                       open_files->fd[file_desc].fd_file = 0;
-                       assert(file);   /* 9ns shouldn't call this put */
-                       kref_put(&file->f_kref);
-                       CLR_BITMASK_BIT(open_files->open_fds->fds_bits, file_desc);
+                       assert(fd < fdt->max_files);
+                       file = fdt->fd[fd].fd_file;
+                       chan = fdt->fd[fd].fd_chan;
+                       if (file) {
+                               kref_put(&file->f_kref);
+                               fdt->fd[fd].fd_file = 0;
+                       } else {
+                               cclose(chan);
+                               fdt->fd[fd].fd_chan = 0;
+                       }
+                       CLR_BITMASK_BIT(fdt->open_fds->fds_bits, fd);
+                       ret = TRUE;
                }
        }
-       spin_unlock(&open_files->lock);
-       return file;
+       spin_unlock(&fdt->lock);
+       return ret;
+}
+
+void put_file_from_fd(struct fd_table *open_files, int file_desc)
+{
+       close_fd(open_files, file_desc);
 }
 
 static int __get_fd(struct fd_table *open_files, int low_fd)
@@ -2544,82 +2572,100 @@ int claim_fd(struct fd_table *open_files, int file_desc)
        return ret;
 }
 
-/* Inserts the file in the fd_table, returning the corresponding new file
- * descriptor, or an error code.  We start looking for open fds from low_fd.
- *
- * Passing cloexec is a bit cheap, since we might want to expand it to support
- * more FD options in the future. */
-int insert_file(struct fd_table *open_files, struct file *file, int low_fd,
-                bool must, bool cloexec)
+/* Insert a file or chan (obj, chosen by vfs) into the fd group with fd_flags.
+ * If must_use_low, then we have to insert at FD = low_fd.  o/w we start looking
+ * for empty slots at low_fd. */
+int insert_obj_fdt(struct fd_table *fdt, void *obj, int low_fd, int fd_flags,
+                   bool must_use_low, bool vfs)
 {
        int slot, ret;
-       spin_lock(&open_files->lock);
-       if (must) {
-               ret = __claim_fd(open_files, low_fd);
+       spin_lock(&fdt->lock);
+       if (must_use_low) {
+               ret = __claim_fd(fdt, low_fd);
                if (ret < 0) {
-                       spin_unlock(&open_files->lock);
+                       spin_unlock(&fdt->lock);
                        return ret;
                }
                assert(!ret);   /* issues with claim_fd returning status, not the fd */
                slot = low_fd;
        } else {
-               slot = __get_fd(open_files, low_fd);
+               slot = __get_fd(fdt, low_fd);
        }
 
        if (slot < 0) {
-               spin_unlock(&open_files->lock);
+               spin_unlock(&fdt->lock);
                return slot;
        }
-       assert(slot < open_files->max_files &&
-              open_files->fd[slot].fd_file == 0);
-       kref_get(&file->f_kref, 1);
-       open_files->fd[slot].fd_file = file;
-       open_files->fd[slot].fd_flags = 0;
-       if (cloexec)
-               open_files->fd[slot].fd_flags |= FD_CLOEXEC;
-       spin_unlock(&open_files->lock);
+       assert(slot < fdt->max_files &&
+              fdt->fd[slot].fd_file == 0);
+       if (vfs) {
+               kref_get(&((struct file*)obj)->f_kref, 1);
+               fdt->fd[slot].fd_file = obj;
+               fdt->fd[slot].fd_chan = 0;
+       } else {
+               chan_incref((struct chan*)obj);
+               fdt->fd[slot].fd_file = 0;
+               fdt->fd[slot].fd_chan = obj;
+       }
+       fdt->fd[slot].fd_flags = fd_flags;
+       spin_unlock(&fdt->lock);
        return slot;
 }
 
+/* Inserts the file in the fd_table, returning the corresponding new file
+ * descriptor, or an error code.  We start looking for open fds from low_fd.
+ *
+ * Passing cloexec is a bit cheap, since we might want to expand it to support
+ * more FD options in the future. */
+int insert_file(struct fd_table *open_files, struct file *file, int low_fd,
+                bool must, bool cloexec)
+{
+       return insert_obj_fdt(open_files, file, low_fd, cloexec ? FD_CLOEXEC : 0,
+                             must, TRUE);
+}
+
 /* Closes all open files.  Mostly just a "put" for all files.  If cloexec, it
  * will only close the FDs with FD_CLOEXEC (opened with O_CLOEXEC or fcntld). */
-void close_all_files(struct fd_table *open_files, bool cloexec)
+void close_fdt(struct fd_table *fdt, bool cloexec)
 {
        struct file *file;
-       spin_lock(&open_files->lock);
-       if (open_files->closed) {
-               spin_unlock(&open_files->lock);
+       struct chan *chan;
+       spin_lock(&fdt->lock);
+       if (fdt->closed) {
+               spin_unlock(&fdt->lock);
                return;
        }
-       for (int i = 0; i < open_files->max_fdset; i++) {
-               if (GET_BITMASK_BIT(open_files->open_fds->fds_bits, i)) {
+       for (int i = 0; i < fdt->max_fdset; i++) {
+               if (GET_BITMASK_BIT(fdt->open_fds->fds_bits, i)) {
                        /* while max_files and max_fdset might not line up, we should never
                         * have a valid fdset higher than files */
-                       assert(i < open_files->max_files);
-                       file = open_files->fd[i].fd_file;
-                       /* no file == 9ns uses the FD.  they will deal with it */
-                       if (!file)
+                       assert(i < fdt->max_files);
+                       if (cloexec && !(fdt->fd[i].fd_flags & FD_CLOEXEC))
                                continue;
-                       if (cloexec && !(open_files->fd[i].fd_flags & FD_CLOEXEC))
-                               continue;
-                       /* Actually close the file */
-                       open_files->fd[i].fd_file = 0;
-                       assert(file);
-                       kref_put(&file->f_kref);
-                       CLR_BITMASK_BIT(open_files->open_fds->fds_bits, i);
+                       file = fdt->fd[i].fd_file;
+                       chan = fdt->fd[i].fd_chan;
+                       if (file) {
+                               fdt->fd[i].fd_file = 0;
+                               kref_put(&file->f_kref);
+                       } else {
+                               fdt->fd[i].fd_chan = 0;
+                               cclose(chan);
+                       }
+                       CLR_BITMASK_BIT(fdt->open_fds->fds_bits, i);
                }
        }
        if (!cloexec) {
-               free_fd_set(open_files);
-               open_files->closed = TRUE;
+               free_fd_set(fdt);
+               fdt->closed = TRUE;
        }
-       spin_unlock(&open_files->lock);
+       spin_unlock(&fdt->lock);
 }
 
 /* Inserts all of the files from src into dst, used by sys_fork(). */
-void clone_files(struct fd_table *src, struct fd_table *dst)
+void clone_fdt(struct fd_table *src, struct fd_table *dst)
 {
        struct file *file;
+       struct chan *chan;
        spin_lock(&src->lock);
        if (src->closed) {
                spin_unlock(&src->lock);
@@ -2638,12 +2684,16 @@ void clone_files(struct fd_table *src, struct fd_table *dst)
                         * have a valid fdset higher than files */
                        assert(i < src->max_files);
                        file = src->fd[i].fd_file;
+                       chan = src->fd[i].fd_chan;
                        assert(i < dst->max_files && dst->fd[i].fd_file == 0);
                        SET_BITMASK_BIT(dst->open_fds->fds_bits, i);
                        dst->fd[i].fd_file = file;
-                       /* no file means 9ns is using it, they clone separately */
+                       dst->fd[i].fd_chan = chan;
+                       /* no file means 9ns is using it, they clone separately (TODO) */
                        if (file)
                                kref_get(&file->f_kref, 1);
+               //      else
+               //              chan_incref(chan);
                        if (i >= dst->next_fd)
                                dst->next_fd = i + 1;
                }