Fixes insert_file()
[akaros.git] / kern / src / vfs.c
index 1a999d9..bc43859 100644 (file)
@@ -463,6 +463,8 @@ int path_lookup(char *path, int flags, struct nameidata *nd)
        int retval;
        printd("Path lookup for %s\n", path);
        /* we allow absolute lookups with no process context */
+       /* TODO: RCU read lock on pwd or kref_not_zero in a loop.  concurrent chdir
+        * could decref nd->dentry before we get to incref it below. */
        if (path[0] == '/') {                   /* absolute lookup */
                if (!current)
                        nd->dentry = default_ns.root->mnt_root;
@@ -958,6 +960,7 @@ void load_inode(struct dentry *dentry, unsigned long ino)
  * note we don't pass this an nd, like Linux does... */
 static struct inode *create_inode(struct dentry *dentry, int mode)
 {
+       uint64_t now = epoch_seconds();
        /* note it is the i_ino that uniquely identifies a file in the specific
         * filesystem.  there's a diff between creating an inode (even for an in-use
         * ino) and then filling it in, and vs creating a brand new one.
@@ -970,10 +973,10 @@ static struct inode *create_inode(struct dentry *dentry, int mode)
        inode->i_nlink = 1;
        inode->i_size = 0;
        inode->i_blocks = 0;
-       inode->i_atime.tv_sec = 0;              /* TODO: now! */
-       inode->i_ctime.tv_sec = 0;
-       inode->i_mtime.tv_sec = 0;
-       inode->i_atime.tv_nsec = 0;             /* are these supposed to be the extra ns? */
+       inode->i_atime.tv_sec = now;
+       inode->i_ctime.tv_sec = now;
+       inode->i_mtime.tv_sec = now;
+       inode->i_atime.tv_nsec = 0;
        inode->i_ctime.tv_nsec = 0;
        inode->i_mtime.tv_nsec = 0;
        inode->i_bdev = inode->i_sb->s_bdev;
@@ -1156,20 +1159,23 @@ ssize_t generic_file_read(struct file *file, char *buf, size_t count,
        unsigned long first_idx, last_idx;
        size_t copy_amt;
        char *buf_end;
+       /* read in offset, in case of a concurrent reader/writer, so we don't screw
+        * up our math for count, the idxs, etc. */
+       off64_t orig_off = ACCESS_ONCE(*offset);
 
        /* Consider pushing some error checking higher in the VFS */
        if (!count)
                return 0;
-       if (*offset >= file->f_dentry->d_inode->i_size)
+       if (orig_off >= file->f_dentry->d_inode->i_size)
                return 0; /* EOF */
        /* Make sure we don't go past the end of the file */
-       if (*offset + count > file->f_dentry->d_inode->i_size) {
-               count = file->f_dentry->d_inode->i_size - *offset;
+       if (orig_off + count > file->f_dentry->d_inode->i_size) {
+               count = file->f_dentry->d_inode->i_size - orig_off;
        }
        assert((long)count > 0);
-       page_off = *offset & (PGSIZE - 1);
-       first_idx = *offset >> PGSHIFT;
-       last_idx = (*offset + count) >> PGSHIFT;
+       page_off = orig_off & (PGSIZE - 1);
+       first_idx = orig_off >> PGSHIFT;
+       last_idx = (orig_off + count) >> PGSHIFT;
        buf_end = buf + count;
        /* For each file page, make sure it's in the page cache, then copy it out.
         * TODO: will probably need to consider concurrently truncated files here.*/
@@ -1192,7 +1198,10 @@ ssize_t generic_file_read(struct file *file, char *buf, size_t count,
                pm_put_page(page);      /* it's still in the cache, we just don't need it */
        }
        assert(buf == buf_end);
-       *offset += count;
+       /* could have concurrent file ops that screw with offset, so userspace isn't
+        * safe.  but at least it'll be a value that one of the concurrent ops could
+        * have produced (compared to *offset_changed_concurrently += count. */
+       *offset = orig_off + count;
        return count;
 }
 
@@ -1212,17 +1221,18 @@ ssize_t generic_file_write(struct file *file, const char *buf, size_t count,
        unsigned long first_idx, last_idx;
        size_t copy_amt;
        const char *buf_end;
+       off64_t orig_off = ACCESS_ONCE(*offset);
 
        /* Consider pushing some error checking higher in the VFS */
        if (!count)
                return 0;
        /* Extend the file.  Should put more checks in here, and maybe do this per
         * page in the for loop below. */
-       if (*offset + count > file->f_dentry->d_inode->i_size)
-               file->f_dentry->d_inode->i_size = *offset + count;
-       page_off = *offset & (PGSIZE - 1);
-       first_idx = *offset >> PGSHIFT;
-       last_idx = (*offset + count) >> PGSHIFT;
+       if (orig_off + count > file->f_dentry->d_inode->i_size)
+               file->f_dentry->d_inode->i_size = orig_off + count;
+       page_off = orig_off & (PGSIZE - 1);
+       first_idx = orig_off >> PGSHIFT;
+       last_idx = (orig_off + count) >> PGSHIFT;
        buf_end = buf + count;
        /* For each file page, make sure it's in the page cache, then write it.*/
        for (int i = first_idx; i <= last_idx; i++) {
@@ -1243,7 +1253,7 @@ ssize_t generic_file_write(struct file *file, const char *buf, size_t count,
                pm_put_page(page);      /* it's still in the cache, we just don't need it */
        }
        assert(buf == buf_end);
-       *offset += count;
+       *offset = orig_off + count;
        return count;
 }
 
@@ -1320,7 +1330,16 @@ struct file *do_file_open(char *path, int flags, int mode)
        nd->intent = LOOKUP_OPEN;
        error = path_lookup(path, LOOKUP_FOLLOW, nd);
        if (!error) {
-               /* Still need to make sure we didn't want to O_EXCL create */
+               /* If this is a directory, make sure we are opening with O_RDONLY.
+                * Unfortunately we can't just check for O_RDONLY directly because its
+                * value is 0x0.  We instead have to make sure it's not O_WRONLY and
+                * not O_RDWR explicitly. */
+               if (S_ISDIR(nd->dentry->d_inode->i_mode) &&
+                   ((flags & O_WRONLY) || (flags & O_RDWR))) {
+                       set_errno(EISDIR);
+                       goto out_path_only;
+               }
+               /* Also need to make sure we didn't want to O_EXCL create */
                if ((flags & O_CREAT) && (flags & O_EXCL)) {
                        set_errno(EEXIST);
                        goto out_path_only;
@@ -2207,7 +2226,8 @@ static int __claim_fd(struct files_struct *open_files, int file_desc)
                return -ENFILE; /* Should never really happen. Here to catch bugs. */
 
        SET_BITMASK_BIT(open_files->open_fds->fds_bits, file_desc);
-       assert(file_desc < open_files->max_files && open_files->fd[0].fd_file == 0);
+       assert(file_desc < open_files->max_files &&
+              open_files->fd[file_desc].fd_file == 0);
        if (file_desc >= open_files->next_fd)
                open_files->next_fd = file_desc + 1;
        return 0;
@@ -2225,11 +2245,23 @@ int claim_fd(struct files_struct *open_files, int file_desc)
 
 /* Inserts the file in the files_struct, returning the corresponding new file
  * descriptor, or an error code.  We start looking for open fds from low_fd. */
-int insert_file(struct files_struct *open_files, struct file *file, int low_fd)
+int insert_file(struct files_struct *open_files, struct file *file, int low_fd,
+                bool must)
 {
-       int slot;
+       int slot, ret;
        spin_lock(&open_files->lock);
-       slot = __get_fd(open_files, low_fd);
+       if (must) {
+               ret = __claim_fd(open_files, low_fd);
+               if (ret < 0) {
+                       spin_unlock(&open_files->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);
+       }
+
        if (slot < 0) {
                spin_unlock(&open_files->lock);
                return slot;
@@ -2314,6 +2346,20 @@ void clone_files(struct files_struct *src, struct files_struct *dst)
        spin_unlock(&src->lock);
 }
 
+static void __chpwd(struct fs_struct *fs_env, struct dentry *new_pwd)
+{
+       struct dentry *old_pwd;
+       kref_get(&new_pwd->d_kref, 1);
+       /* writer lock, make sure we replace pwd with ours.  could also CAS.
+        * readers don't lock at all, so they need to either loop, or we need to
+        * delay releasing old_pwd til an RCU grace period. */
+       spin_lock(&fs_env->lock);
+       old_pwd = fs_env->pwd;
+       fs_env->pwd = new_pwd;
+       spin_unlock(&fs_env->lock);
+       kref_put(&old_pwd->d_kref);
+}
+
 /* Change the working directory of the given fs env (one per process, at this
  * point).  Returns 0 for success, -ERROR for whatever error. */
 int do_chdir(struct fs_struct *fs_env, char *path)
@@ -2323,14 +2369,22 @@ int do_chdir(struct fs_struct *fs_env, char *path)
        retval = path_lookup(path, LOOKUP_DIRECTORY, nd);
        if (!retval) {
                /* nd->dentry is the place we want our PWD to be */
-               kref_get(&nd->dentry->d_kref, 1);
-               kref_put(&fs_env->pwd->d_kref);
-               fs_env->pwd = nd->dentry;
+               __chpwd(fs_env, nd->dentry);
        }
        path_release(nd);
        return retval;
 }
 
+int do_fchdir(struct fs_struct *fs_env, struct file *file)
+{
+       if ((file->f_dentry->d_inode->i_mode & __S_IFMT) != __S_IFDIR) {
+               set_errno(ENOTDIR);
+               return -1;
+       }
+       __chpwd(fs_env, file->f_dentry);
+       return 0;
+}
+
 /* Returns a null-terminated string of up to length cwd_l containing the
  * absolute path of fs_env, (up to fs_env's root).  Be sure to kfree the char*
  * "kfree_this" when you are done with it.  We do this since it's easier to