From c38fa859a6fb3e9fce736ead57ee776fe433a0d0 Mon Sep 17 00:00:00 2001
From: dzwdz
Date: Mon, 2 May 2022 18:58:57 +0200
Subject: kernel/vfs: always separately allocate the request object

---
 src/kernel/proc.c        |  2 ++
 src/kernel/proc.h        |  4 ++--
 src/kernel/vfs/request.c | 58 +++++++++++++++++++-----------------------------
 src/kernel/vfs/root.c    |  4 ++--
 4 files changed, 29 insertions(+), 39 deletions(-)

(limited to 'src/kernel')

diff --git a/src/kernel/proc.c b/src/kernel/proc.c
index 39f9e1f..838d520 100644
--- a/src/kernel/proc.c
+++ b/src/kernel/proc.c
@@ -288,6 +288,8 @@ void process_kill(struct process *p, int ret) {
 			 *
 			 * we also reparent it to process_deadparent because we don't want
 			 * dead processes to have any alive children */
+			/* TODO: because requests are no longer owned by the parent, we can safely kill it.
+			 * this whole deathbed thing (and, by extension, freeing after killing) is unnecessary */
 			// TODO process_reparent?
 			p->deathbed = true;
 			process_forget(p);
diff --git a/src/kernel/proc.h b/src/kernel/proc.h
index 80cb4a3..2e7f14d 100644
--- a/src/kernel/proc.h
+++ b/src/kernel/proc.h
@@ -35,7 +35,7 @@ struct process {
 	union {
 		int death_msg; // PS_DEAD
 		struct {
-			struct vfs_request req;
+			struct vfs_request *req;
 		} waits4fs; // PS_WAITS4FS
 		struct {
 			char __user *buf;
@@ -43,7 +43,7 @@ struct process {
 			struct fs_wait_response __user *res;
 		} awaited_req; // PS_WAITS4REQUEST
 		struct {
-			struct vfs_request req;
+			struct vfs_request *req;
 			bool (*ready)();
 			void (*callback)(struct process *);
 		} waits4irq;
diff --git a/src/kernel/vfs/request.c b/src/kernel/vfs/request.c
index 99d2e2c..1f3193f 100644
--- a/src/kernel/vfs/request.c
+++ b/src/kernel/vfs/request.c
@@ -7,22 +7,13 @@
 #include <shared/mem.h>
 
 int vfs_request_create(struct vfs_request req_) {
-	struct vfs_request *req;
-
-	if (req_.caller) {
-		/* if the request has an explicit caller (isn't a close() call)
-		 * it's owned by said caller
-		 *
-		 * it would be simpler if all requests were owned by the backend.
-		 * i might end up changing that eventually. TODO? */
-		process_transition(req_.caller, PS_WAITS4FS);
-		req_.caller->waits4fs.req = req_;
-		req = &req_.caller->waits4fs.req;
-	} else {
-		/* requests without explicit callers (close() calls) are owned by the
-		 * backend, and freed in vfs_request_finish or vfs_request_cancel */
-		req = kmalloc(sizeof *req);
-		memcpy(req, &req_, sizeof *req);
+	struct vfs_request *req = kmalloc(sizeof *req);
+	memcpy(req, &req_, sizeof *req);
+	/* freed in vfs_request_finish or vfs_request_cancel */
+
+	if (req->caller) {
+		process_transition(req->caller, PS_WAITS4FS);
+		req->caller->waits4fs.req = req;
 	}
 
 	if (!req->backend || !req->backend->potential_handlers)
@@ -104,31 +95,28 @@ int vfs_request_finish(struct vfs_request *req, int ret) {
 	if (req->input.kern)
 		kfree(req->input.buf_kern);
 
-	if (!req->caller) {
-		kfree(req); // ok, this stinks. see comment at top of file
-		return 0;
+	if (req->caller) {
+		assert(req->caller->state == PS_WAITS4FS || req->caller->state == PS_WAITS4IRQ);
+		regs_savereturn(&req->caller->regs, ret);
+		process_transition(req->caller, PS_RUNNING);
 	}
 
-	assert(req->caller->state == PS_WAITS4FS || req->caller->state == PS_WAITS4IRQ);
-	regs_savereturn(&req->caller->regs, ret);
-	process_transition(req->caller, PS_RUNNING);
+	kfree(req);
 	return ret;
 }
 
 void vfs_request_cancel(struct vfs_request *req, int ret) {
-	if (!req->caller) {
-		kfree(req);
-		return;
-	}
-
-	assert(req->caller->state == PS_WAITS4FS);
+	if (req->caller) {
+		assert(req->caller->state == PS_WAITS4FS);
 
-	if (req->input.kern)
-		kfree(req->input.buf_kern);
+		if (req->input.kern)
+			kfree(req->input.buf_kern);
 
-	// ret must always be negative, so it won't be confused with a success
-	if (ret > 0) ret = -ret;
-	if (ret == 0) ret = -1;
-	regs_savereturn(&req->caller->regs, ret);
-	process_transition(req->caller, PS_RUNNING);
+		// ret must always be negative, so it won't be confused with a success
+		if (ret > 0) ret = -ret;
+		if (ret == 0) ret = -1;
+		regs_savereturn(&req->caller->regs, ret);
+		process_transition(req->caller, PS_RUNNING);
+	}
+	kfree(req);
 }
diff --git a/src/kernel/vfs/root.c b/src/kernel/vfs/root.c
index 36e9da1..4a1148e 100644
--- a/src/kernel/vfs/root.c
+++ b/src/kernel/vfs/root.c
@@ -50,14 +50,14 @@ static void req_preprocess(struct vfs_request *req, size_t max_len) {
 
 
 static void wait_callback(struct process *proc) {
-	vfs_root_handler(&proc->waits4irq.req);
+	vfs_root_handler(proc->waits4irq.req);
 }
 
 static bool wait_setup(struct vfs_request *req, bool *ready, bool (*ready_fn)()) {
 	if (!ready_fn()) {
 		*ready = false;
 		process_transition(req->caller, PS_WAITS4IRQ);
-		req->caller->waits4irq.req = *req;
+		req->caller->waits4irq.req = req;
 		req->caller->waits4irq.ready = ready_fn;
 		req->caller->waits4irq.callback = wait_callback;
 		return true;
-- 
cgit v1.2.3