improve pipe implementation to support blocking read/write

kernel/fs/filebase.c

@@ -72,15 +72,14 @@ void vfs_free_nodes(vfs_tnode_t *tnode)
 }
 
 /* Return the node descriptor for a handle */
-vfs_node_desc_t *vfs_handle_to_fd(vfs_handle_t handle)
+vfs_node_desc_t *vfs_handle_to_fd(vfs_handle_t handle, const char *func)
 {
     task_t *t = sched_get_current_task();
     if (t != NULL) {
         vfs_node_desc_t* nd = (vfs_node_desc_t*)ht_search(&(t->open_files_table), handle);
         if (nd != NULL) return nd;
-        klogw("VFS: cannot locate %d (0x%x) in file list of task %d, "
-              "maybe we need to increase hash table's size.\n",
-              handle, handle, t->tid);
+        klogw("VFS: %s() cannot locate %d (0x%x) in file list of task %d\n",
+              func, handle, handle, t->tid);
     }
     return NULL;
 }

kernel/fs/filebase.h

@@ -33,5 +33,5 @@ extern vfs_tnode_t vfs_root;
 vfs_tnode_t* vfs_alloc_tnode(const char* name, vfs_inode_t* inode, vfs_inode_t* parent);
 vfs_inode_t* vfs_alloc_inode(vfs_node_type_t type, uint32_t perms, uint32_t uid, vfs_fsinfo_t* fs, vfs_tnode_t* mnt);
 void vfs_free_nodes(vfs_tnode_t* tnode);
-vfs_node_desc_t* vfs_handle_to_fd(vfs_handle_t handle);
+vfs_node_desc_t* vfs_handle_to_fd(vfs_handle_t handle, const char *func);
 vfs_tnode_t* vfs_path_to_node(const char* path, uint8_t mode, vfs_node_type_t create_type);

kernel/fs/pipefs.c

@@ -29,12 +29,16 @@ vfs_fsinfo_t pipefs = {
     .ioctl = NULL
 };
 
-lock_t pipe_lock;
+lock_t pipe_lock = {0};
+extern lock_t vfs_lock;
+
+char pipe_eof_magic_word[5] = {0xFF, 0x0E, 0x00, 0x0F, 0x00}; 
 
 /* Identifying information for a node */
 typedef struct {
     char    buff[PIPE_BUFFER_SIZE];
     int64_t size;
+    bool    closed;
 } pipefs_ident_t;
 
 static pipefs_ident_t *create_ident()
@@ -65,10 +69,20 @@ int64_t pipefs_read(vfs_inode_t* this, uint64_t offset, uint64_t len, void *buff
     pipefs_ident_t *id = this->ident;
     uint64_t rlen = 0;
 
-    if (id->size == 0 || len == 0) return 0;
-
-    klogd("PIPEFS: read %d bytes from 0x%x (PIPE) to 0x%x with %d bytes\n",
-          len, id->buff, buff, id->size);
+    if (len == 0 || id->closed) {
+        klogd("PIPEFS: read %d bytes to 0x%x and return 0 bytes\n",
+              len, buff);
+        return 0;
+    }
+
+    /* According to standard pipe implementation, the task should be blocked
+     * here until there are data available.
+     */
+    while (id->size == 0) {
+        lock_release(&vfs_lock);
+        sched_sleep(0);
+        lock_lock(&vfs_lock);
+    }
 
     /* We do not use offset here */
     (void)offset;
@@ -78,7 +92,7 @@ int64_t pipefs_read(vfs_inode_t* this, uint64_t offset, uint64_t len, void *buff
     rlen = id->size;
     if (rlen > len) rlen = len;
     memcpy(buff, id->buff, rlen);
-        
+
     if (id->size - rlen > 0) {
         char val = ((char*)id->buff)[id->size - 1];
         memcpy(id->buff, &(id->buff[rlen]), id->size - rlen);
@@ -89,10 +103,22 @@ int64_t pipefs_read(vfs_inode_t* this, uint64_t offset, uint64_t len, void *buff
     }
     id->size -= rlen;
 
+    if (rlen >= 4) {
+        if (((char*)buff)[rlen - 4] == pipe_eof_magic_word[0]) {
+            if (   ((char*)buff)[rlen - 3] == pipe_eof_magic_word[1]
+                && ((char*)buff)[rlen - 2] == pipe_eof_magic_word[2]
+                && ((char*)buff)[rlen - 1] == pipe_eof_magic_word[3])
+            {
+                id->closed = true;
+                rlen -= 4;
+            }
+        }
+    }
+
     lock_release(&pipe_lock);
 
-    klogd("PIPEFS: read %d bytes to 0x%x and return %d bytes\n",
-          len, buff, rlen);
+    klogd("PIPEFS: read %d bytes to 0x%x and return %d bytes (%02x)\n",
+          len, buff, rlen, (rlen > 0 ? ((char*)buff)[rlen - 1] : 0));
 
     return rlen;
 }
@@ -102,8 +128,9 @@ int64_t pipefs_write(vfs_inode_t* this, uint64_t offset, uint64_t len,
 {
     pipefs_ident_t *id = this->ident;
 
-    klogd("PIPEFS: write %d bytes from %x (PIPE) to 0x%x with %d bytes\n",
-          len, id->buff, buff, id->size);
+    klogd("PIPEFS: write %d bytes from %x to %x (PIPE) whose size is "
+          "%d bytes and %d refcount\n",
+          len, buff, id->buff, id->size, this->refcount);
 
     lock_lock(&pipe_lock);
 
@@ -112,9 +139,21 @@ int64_t pipefs_write(vfs_inode_t* this, uint64_t offset, uint64_t len,
 
     /* Output to the buffer */
     uint64_t wlen = 0;
-    if (PIPE_BUFFER_SIZE > id->size) {
-        wlen = PIPE_BUFFER_SIZE - id->size;
+
+    /* According to standard pipe implementation, the task should be blocked
+     * here until there are rooms available in the pipe.
+     */
+    while (true) {
+        if (PIPE_BUFFER_SIZE > id->size) {
+            wlen = PIPE_BUFFER_SIZE - id->size;
+        }
+        if (wlen > 0) break;
+
+        lock_release(&vfs_lock);
+        sched_sleep(0);
+        lock_lock(&vfs_lock);
     }
+
     if (wlen > len) wlen = len;
     memcpy(&(id->buff[id->size]), buff, wlen);
 

kernel/fs/vfs.c

@@ -54,6 +54,8 @@ vec_new_static(vfs_fsinfo_t*, vfs_fslist);
 /* New file handle */
 static uint64_t vfs_next_handle = VFS_MIN_HANDLE; 
 
+extern char pipe_eof_magic_word[];
+
 /* Stat structure related function implementations */
 dev_t vfs_new_dev_id(void)
 {
@@ -180,7 +182,7 @@ int64_t vfs_create(char* path, vfs_node_type_t type)
 /* Changes permissions of node */
 int64_t vfs_chmod(vfs_handle_t handle, int32_t newperms)
 {
-    vfs_node_desc_t* fd = vfs_handle_to_fd(handle);
+    vfs_node_desc_t* fd = vfs_handle_to_fd(handle, __func__);
     if (!fd)
         return -1;
 
@@ -199,7 +201,7 @@ int64_t vfs_chmod(vfs_handle_t handle, int32_t newperms)
 
 int64_t vfs_ioctl(vfs_handle_t handle, int64_t request, int64_t arg)
 {
-    vfs_node_desc_t* fd = vfs_handle_to_fd(handle);
+    vfs_node_desc_t* fd = vfs_handle_to_fd(handle, __func__);
     if (!fd)
         return -1; 
 
@@ -257,7 +259,7 @@ int64_t vfs_mount(char* device, char* path, char* fsname)
 /* Get the length of a file */
 uint64_t vfs_tell(vfs_handle_t handle)
 {
-    vfs_node_desc_t* fd = vfs_handle_to_fd(handle);
+    vfs_node_desc_t* fd = vfs_handle_to_fd(handle, __func__);
 
     if (!fd) {
         kloge("VFS: cannot get fd for file %d\n", handle); 
@@ -271,7 +273,7 @@ uint64_t vfs_tell(vfs_handle_t handle)
 /* Read specified number of bytes from a file */
 int64_t vfs_read(vfs_handle_t handle, uint64_t len, void* buff)
 {
-    vfs_node_desc_t* fd = vfs_handle_to_fd(handle);
+    vfs_node_desc_t* fd = vfs_handle_to_fd(handle, __func__);
     if (!fd) {
         return 0;
     }
@@ -350,7 +352,7 @@ int64_t vfs_unlink(char *path)
 /* Write specified number of bytes to file */
 int64_t vfs_write(vfs_handle_t handle, uint64_t len, const void *buff)
 {
-    vfs_node_desc_t *nd = vfs_handle_to_fd(handle);
+    vfs_node_desc_t *nd = vfs_handle_to_fd(handle, __func__);
     if (!nd)
         return 0;
 
@@ -387,7 +389,7 @@ int64_t vfs_write(vfs_handle_t handle, uint64_t len, const void *buff)
 /* Seek to specified position in file */
 int64_t vfs_seek(vfs_handle_t handle, uint64_t pos, int64_t whence)
 {
-    vfs_node_desc_t* fd = vfs_handle_to_fd(handle);
+    vfs_node_desc_t* fd = vfs_handle_to_fd(handle, __func__);
     if (!fd)
         return -1;
 
@@ -553,16 +555,24 @@ vfs_handle_t vfs_open(char *path, vfs_openmode_t mode)
 
 int64_t vfs_close(vfs_handle_t handle)
 {
+    bool istty = false;
+
     lock_lock(&vfs_lock);
 
-    vfs_node_desc_t *nd = vfs_handle_to_fd(handle);
+    vfs_node_desc_t *nd = vfs_handle_to_fd(handle, __func__);
     if (!nd)
         goto fail;
 
-    if (strcmp(nd->path, "/dev/tty") != 0) {
-        klogv("VFS: close file handle %d\n", handle);
+    if (strcmp(nd->path, "/dev/tty") == 0) istty = true;
+    if (strncmp(nd->path, "/dev/pipe", 9) == 0) {
+        if ((nd->mode & VFS_MODE_WRITE) && nd->seek_pos > 0) {
+            klogi("VFS: write EOF to %s with seek position %d\n",
+                  nd->path, nd->seek_pos);
+            lock_release(&vfs_lock);
+            vfs_write(handle, 4, pipe_eof_magic_word);
+            lock_lock(&vfs_lock);
+        }
     }
-
     nd->inode->refcount--;
 
     task_t *t = sched_get_current_task();
@@ -583,6 +593,10 @@ int64_t vfs_close(vfs_handle_t handle)
     kmfree(nd);
 
     lock_release(&vfs_lock);
+
+    if (!istty) {
+        klogv("VFS: close file handle %d\n", handle);
+    }
     return 0;
 fail:
     lock_release(&vfs_lock);
@@ -591,7 +605,7 @@ int64_t vfs_close(vfs_handle_t handle)
 
 int64_t vfs_refresh(vfs_handle_t handle)
 {
-    vfs_node_desc_t *nd = vfs_handle_to_fd(handle);
+    vfs_node_desc_t *nd = vfs_handle_to_fd(handle, __func__);
     if (!nd)
         return -1; 
 
@@ -617,7 +631,7 @@ int64_t vfs_refresh(vfs_handle_t handle)
 /* Get next directory entry */
 int64_t vfs_getdent(vfs_handle_t handle, vfs_dirent_t* dirent) {
     int64_t status;
-    vfs_node_desc_t *nd = vfs_handle_to_fd(handle);
+    vfs_node_desc_t *nd = vfs_handle_to_fd(handle, __func__);
     if (!nd)
         return -1;
 

kernel/proc/sched.c

@@ -327,7 +327,7 @@ void sched_sleep(time_t millis)
         hpet_sleep(millis);
         return;
     }
- 
+
     lock_lock(&sched_lock);
 
     uint16_t cpu_id = cpu->cpu_id;

kernel/proc/syscall.c

@@ -91,19 +91,24 @@ int64_t k_sigprocmask(int64_t how, sigset_t *set, sigset_t *oldset)
     return 0;
 }
 
-int64_t k_sigaction(int64_t signal, sigaction_t *new, sigaction_t *old)
+int64_t k_sigaction(int64_t s, sigaction_t *new, sigaction_t *old)
 {
     task_t *t = sched_get_current_task();
+    int64_t signal = (int64_t)((int32_t)s);
+
     cpu_set_errno(0);
 
     if (signal >= NSIG || signal < 0 || signal == SIGKILL || signal == SIGSTOP
         || t == NULL)
     {
+        klogd("k_sigaction: signal %d from old 0x%x to new 0x%x "
+              "and return -1 for invalid parameters\n",
+              signal, old, new);
         cpu_set_errno(EINVAL);
         return -1;
     }
 
-    sigaction_t newtmp, oldtmp;
+    sigaction_t newtmp = {0}, oldtmp = {0};
     if (new != NULL) {
         memcpy(&newtmp, new, sizeof(sigaction_t));
 
@@ -298,7 +303,7 @@ int get_full_path(int64_t dirfh, const char *path, char *full_path)
         }
     } else if ((int32_t)dirfh >= (int32_t)0) {
         /* Get the parent path name from dirfh */
-        vfs_node_desc_t *tnode = vfs_handle_to_fd((vfs_handle_t)dirfh);
+        vfs_node_desc_t *tnode = vfs_handle_to_fd((vfs_handle_t)dirfh, __func__);
         if (tnode != NULL) {
             if (path[0] == '.') strcpy(full_path, tnode->path);
         } else {
@@ -826,7 +831,7 @@ int64_t k_fstat(int64_t handle, int64_t statbuf)
         return 0;
     }
 
-    vfs_node_desc_t *fd = vfs_handle_to_fd(handle);
+    vfs_node_desc_t *fd = vfs_handle_to_fd(handle, __func__);
     cpu_set_errno(0);
 
     if (fd != NULL) {
@@ -987,7 +992,7 @@ int64_t k_chdir(char *dir)
 int64_t k_readdir(int64_t handle, uint64_t buff)
 {
     dirent_t *de = (dirent_t*)buff;
-    vfs_node_desc_t *fd = vfs_handle_to_fd(handle);
+    vfs_node_desc_t *fd = vfs_handle_to_fd(handle, __func__);
     int64_t errno = 0;
 
     cpu_set_errno(errno);