kernel: start cleaning up VfsRequest

* I'm being more strict about the linked list state to hopefully ensure
  I'm not leaking any references.
* vfsreq_create was renamed to vfsreq_dispatchcopy as that name feels more
  clear. It copies its argument, and dispatches it.
* Requests for user backends are now handled more like requests for kernel
  backends - there's an accept() function that accepts a request.

5 files changed, 102 insertions, 53 deletions

diff --git a/src/kernel/arch/amd64/driver/util.c b/src/kernel/arch/amd64/driver/util.c
index 0f0ee34..2b33849 100644
--- a/src/kernel/arch/amd64/driver/util.c
+++ b/src/kernel/arch/amd64/driver/util.c
@@ -26,6 +26,7 @@ bool postqueue_pop(VfsReq **queue, void (*accept)(VfsReq *)) {
 	VfsReq *req = *queue;
 	if (req == NULL) return false;
 	*queue = req->postqueue_next;
+	req->postqueue_next = NULL;
 	accept(req);
 	return true;
 }
@@ -45,6 +46,7 @@ void postqueue_ringreadall(VfsReq **queue, ring_t *r) {
 	while (*queue) {
 		req = *queue;
 		*queue = req->postqueue_next;
+		req->postqueue_next = NULL;
 
 		size_t ret = min(mlen, req->output.len);
 		assert(req->type == VFSOP_READ);
diff --git a/src/kernel/handle.c b/src/kernel/handle.c
index f216c13..06a810c 100644
--- a/src/kernel/handle.c
+++ b/src/kernel/handle.c
@@ -1,3 +1,4 @@
+#include <camellia/errno.h>
 #include <kernel/handle.h>
 #include <kernel/malloc.h>
 #include <kernel/panic.h>
@@ -22,7 +23,7 @@ void handle_close(Handle *h) {
 		if (h->base) {
 			handle_close(h->base);
 		} else {
-			vfsreq_create((VfsReq) {
+			vfsreq_dispatchcopy((VfsReq) {
 				.type = VFSOP_CLOSE,
 				.id = h->file_id,
 				.caller = NULL,
@@ -36,7 +37,7 @@ void handle_close(Handle *h) {
 			h->pipe.sister->pipe.sister = NULL;
 		}
 	} else if (h->type == HANDLE_FS_REQ) {
-		if (h->req) vfsreq_finish_short(h->req, -1);
+		if (h->req) vfsreq_finish_short(h->req, -EPIPE);
 	}
 
 	if (h->backend) {
diff --git a/src/kernel/syscalls.c b/src/kernel/syscalls.c
index 9b0746a..5771b6a 100644
--- a/src/kernel/syscalls.c
+++ b/src/kernel/syscalls.c
@@ -85,7 +85,7 @@ hid_t _sys_open(const char __user *path, long len, int flags) {
 		memcpy(path_buf, path_buf + mount->prefix_len, len);
 	}
 
-	vfsreq_create((VfsReq) {
+	vfsreq_dispatchcopy((VfsReq) {
 		.type = VFSOP_OPEN,
 		.input = {
 			.kern = true,
@@ -198,7 +198,7 @@ static long simple_vfsop(
 			req.input.buf = buf;
 			req.input.len = len;
 		}
-		vfsreq_create(req);
+		vfsreq_dispatchcopy(req);
 	} else if (h->type == HANDLE_PIPE) {
 		if (vfsop == VFSOP_READ || vfsop == VFSOP_WRITE) {
 			/* already checked if this is the correct pipe end */
@@ -238,7 +238,7 @@ long _sys_remove(hid_t hid) {
 		hs_close(proc_cur->hs, hid);
 		SYSCALL_RETURN(-EACCES);
 	}
-	vfsreq_create((VfsReq) {
+	vfsreq_dispatchcopy((VfsReq) {
 		.type = VFSOP_REMOVE,
 		.id = h->file_id,
 		.caller = proc_cur,
@@ -271,7 +271,14 @@ hid_t _sys_fs_wait(char __user *buf, long max_len, struct ufs_request __user *re
 	proc_cur->awaited_req.max_len = max_len;
 	proc_cur->awaited_req.res     = res;
 
-	vfs_backend_tryaccept(backend); // sets return value
+	// TODO maybe i should use the postqueue stuff here
+	if (backend->queue) {
+		VfsReq *req = backend->queue;
+		backend->queue = req->queue_next;
+		req->queue_next = NULL;
+		vfsback_useraccept(req);
+	}
+
 	return -1; // dummy
 }
 
diff --git a/src/kernel/vfs/request.c b/src/kernel/vfs/request.c
index 8fe0114..4489f5a 100644
--- a/src/kernel/vfs/request.c
+++ b/src/kernel/vfs/request.c
@@ -6,45 +6,75 @@
 #include <kernel/vfs/request.h>
 #include <shared/mem.h>
 
-static void vfs_backend_user_accept(VfsReq *req);
+/** Checks if the request flags don't contradict each other.
+ * While this could be done by caller of vfsreq_dispatchcopy, as currently
+ * there's only one of them per operation, doing this here means that any
+ * potential future callers can't forget those checks.
+ * Doing this so late is kinda inefficient but I don't really care. */
+static bool
+vfsreq_isvalid(VfsReq *r)
+{
+	/* Notable omissions:
+	 * - Simplifying the path for open(). Already done when resolving the mount.
+	 * - Checking the handle permissions. We don't have access to it.
+	 * */
+	if (r->type == VFSOP_OPEN && !(r->flags & OPEN_WRITE) && (r->flags & OPEN_CREATE)) {
+		return false;
+	}
+
+	// XXX if i add a handle field to vfs_request, check ->readable ->writeable here
+	return true;
+}
 
-void vfsreq_create(VfsReq req_) {
+void
+vfsreq_dispatchcopy(VfsReq tmpl)
+{
 	VfsReq *req;
-	if (req_.caller) {
-		proc_setstate(req_.caller, PS_WAITS4FS);
-		if (!req_.caller->reqslot)
-			req_.caller->reqslot = kmalloc(sizeof *req, "reqslot");
-		req = req_.caller->reqslot;
+	VfsBackend *backend;
+
+	/* allocate memory for the request and move it there */
+	if (tmpl.caller) {
+		// TODO: ensure we aren't overriding an existing request
+		// TODO: wouldn't it be better to just optimize malloc?
+		//       see commit afa28cfc7069f7649632f7b33daac47ee56d3819
+		proc_setstate(tmpl.caller, PS_WAITS4FS);
+		if (!tmpl.caller->reqslot)
+			tmpl.caller->reqslot = kmalloc(sizeof *req, "reqslot");
+		req = tmpl.caller->reqslot;
 		/* (re)using a single allocation for all request a process makes */
 	} else {
 		req = kmalloc(sizeof *req, "reqanon");
 	}
-	memcpy(req, &req_, sizeof *req);
-	if (req->backend) {
-		assert(req->backend->usehcnt);
-		req->backend->usehcnt++;
+
+	memcpy(req, &tmpl, sizeof *req);
+	backend = req->backend;
+	if (backend) {
+		assert(backend->usehcnt > 0);
+		backend->usehcnt++;
 	}
 
-	if (req->type == VFSOP_OPEN && !(req->flags & OPEN_WRITE) && (req->flags & OPEN_CREATE)) {
+	/* Check request validity. Doing this after the memcpy means that it
+	 * doesn't need to be special cased in vfsreq_finish. */
+	if (!vfsreq_isvalid(req)) {
 		vfsreq_finish_short(req, -EINVAL);
 		return;
 	}
-
-	// TODO if i add a handle field to vfs_request, check ->readable ->writeable here
-
-	if (req->backend && req->backend->provhcnt) {
-		VfsReq **iter = &req->backend->queue;
-		while (*iter != NULL) // find free spot in queue
-			iter = &(*iter)->queue_next;
-		*iter = req;
-		vfs_backend_tryaccept(req->backend);
-	} else {
-		vfsreq_finish_short(req, -1);
+	assert(req->queue_next == NULL);
+	assert(req->postqueue_next == NULL);
+
+	if (backend == NULL) {
+		/* null mount - probably never had a real backend */
+		vfsreq_finish_short(req, -ENOENT);
+	} else if (backend->is_user) {
+		vfsback_useraccept(req);
+	} else { /* kernel backend */
+		assert(backend->kern.accept);
+		backend->kern.accept(req);
 	}
 }
 
-void vfsreq_finish(VfsReq *req, char __user *stored, long ret,
-		int flags, Proc *handler)
+void
+vfsreq_finish(VfsReq *req, char __user *stored, long ret, int flags, Proc *handler)
 {
 	if (req->type == VFSOP_OPEN && ret >= 0) {
 		Handle *h;
@@ -88,6 +118,9 @@ void vfsreq_finish(VfsReq *req, char __user *stored, long ret,
 		vfsback_userdown(req->backend);
 	}
 
+	assert(req->queue_next == NULL);
+	assert(req->postqueue_next == NULL);
+
 	if (req->caller) {
 		assert(req->caller->state == PS_WAITS4FS);
 		regs_savereturn(&req->caller->regs, ret);
@@ -97,27 +130,33 @@ void vfsreq_finish(VfsReq *req, char __user *stored, long ret,
 	}
 }
 
-void vfs_backend_tryaccept(VfsBackend *backend) {
-	VfsReq *req = backend->queue;
-	if (!req) return;
-	if (backend->is_user && !backend->user.handler) return;
-
-	backend->queue = req->queue_next;
-	if (backend->is_user) {
-		vfs_backend_user_accept(req);
-	} else {
-		assert(backend->kern.accept);
-		backend->kern.accept(req);
-	}
-}
-
-static void vfs_backend_user_accept(VfsReq *req) {
+void
+vfsback_useraccept(VfsReq *req)
+{
+	VfsBackend *backend;
 	Proc *handler;
 	struct ufs_request res = {0};
 	int len;
 
-	assert(req && req->backend && req->backend->user.handler);
-	handler = req->backend->user.handler;
+	assert(req != NULL);
+	backend = req->backend;
+	assert(backend);
+	assert(backend->is_user);
+	if (backend->provhcnt == 0) {
+		vfsreq_finish_short(req, -EPIPE);
+		return;
+	} else if (backend->user.handler == NULL) {
+		/* queue the request up */
+		// TODO use postqueue
+		VfsReq **it = &backend->queue;
+		while (*it != NULL) { /* find a free spot in queue */
+			it = &(*it)->queue_next;
+		}
+		*it = req;
+		return;
+	}
+
+	handler = backend->user.handler;
 	assert(handler->state == PS_WAITS4REQUEST);
 
 	// the pcpy calls aren't present in all kernel backends
@@ -158,7 +197,6 @@ static void vfs_backend_user_accept(VfsReq *req) {
 	proc_setstate(handler, PS_RUNNING);
 	regs_savereturn(&handler->regs, hid);
 	req->backend->user.handler = NULL;
-	return;
 }
 
 static void
@@ -204,7 +242,8 @@ vfsback_provdown(VfsBackend *b)
 		VfsReq *q = b->queue;
 		while (q) {
 			VfsReq *q2 = q->queue_next;
-			vfsreq_finish_short(q, -1);
+			q->queue_next = NULL;
+			vfsreq_finish_short(q, -EPIPE);
 			q = q2;
 		}
 		b->queue = NULL;
diff --git a/src/kernel/vfs/request.h b/src/kernel/vfs/request.h
index 2bd8c61..0b93491 100644
--- a/src/kernel/vfs/request.h
+++ b/src/kernel/vfs/request.h
@@ -3,7 +3,6 @@
 #include <stdbool.h>
 #include <stddef.h>
 
-/* describes something which can act as an access function */
 struct VfsBackend {
 	/* amount of using references
 	 *  VfsMount
@@ -14,6 +13,7 @@ struct VfsBackend {
 	/* amount of providing references
 	 *  Proc
 	 * 0 - orphaned, will never increase */
+	// TODO move this into .user
 	size_t provhcnt;
 
 	VfsReq *queue;
@@ -64,7 +64,7 @@ struct VfsReq {
 };
 
 /** Assigns the vfs_request to the caller, and dispatches the call */
-void vfsreq_create(VfsReq);
+void vfsreq_dispatchcopy(VfsReq);
 void vfsreq_finish(VfsReq*, char __user *stored, long ret, int flags, Proc *handler);
 
 static inline void vfsreq_finish_short(VfsReq *req, long ret) {
@@ -72,7 +72,7 @@ static inline void vfsreq_finish_short(VfsReq *req, long ret) {
 }
 
 /** Try to accept an enqueued request */
-void vfs_backend_tryaccept(VfsBackend *);
+void vfsback_useraccept(VfsReq *);
 
 /** Decrements the "user" reference count. */
 void vfsback_userdown(VfsBackend *);