Sleep on block requests using kthreads
authorBarret Rhoden <brho@cs.berkeley.edu>
Tue, 26 Oct 2010 22:02:30 +0000 (15:02 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:56 +0000 (17:35 -0700)
There's a bunch of complications, and also note that the page cache's
page_lock doesn't sleep yet - this is just the breq itself.

There will likely be issues with where and on who's core we run
kthreads.  For now, it works.  Likewise, there are potential issues with
schedule returning (the old concern was running off the stack) and how
we wait for IO.  Right now, we have a couple ghetto hacks to prevent us
from being in the monitor when an IO (aka timer interrupt) finishes.

Also, note KVM's sense of time with set_core_timer() seems off.  We
shouldn't have to wait 10s of ms to go through a couple function calls.
It might be using the real core's timer instead of the emulated timer.
Whoever works on the time infrastructure ought to take a look at that.

Documentation/kthreads.txt
kern/include/blockdev.h
kern/include/kthread.h
kern/include/process.h
kern/src/blockdev.c
kern/src/ext2fs.c
kern/src/kthread.c
kern/src/manager.c
kern/src/process.c
kern/src/schedule.c
kern/src/smp.c

index 3e8f4fc..f131f8f 100644 (file)
@@ -230,3 +230,25 @@ you'll want to rerun the kthread on the physical core it was suspended on!
 (cache locality, and it might be a legit option to allow processes to say it's
 okay to take their vcore).  Note this may require more bookkeeping in the struct
 kthread.
 (cache locality, and it might be a legit option to allow processes to say it's
 okay to take their vcore).  Note this may require more bookkeeping in the struct
 kthread.
+
+There is another complication: the way we've been talking about kmsgs (starting
+fresh), we are talking about *routine* messages.  One requirement for routine
+messages that do not return is that they handle process state.  The current
+kmsgs, such as __death and __preempt are built so they can handle acting on
+whichever process is currently running.  Likewise, __launch_kthread() needs to
+handle the cases that arise when it runs on a core that was about to run a
+process (as can often happen with proc_restartcore(), which calls
+process_routine_kmsg()).  Basically, if it was a _S, it just yields the process,
+similar to what happens in Linux (call schedule() on the way out, from what I
+recall).  If it was a _M, things are a bit more complicated, since this should
+only happen if the kthread is for that process (and probably a bunch of other
+things - like they said it was okay to interrupt their vcore to finish the
+syscall).
+
+To a certain extent, routine kmsgs don't seem like a nice fit, when we really
+want to be calling schedule().  Though if you think of it as the enactment of a
+previous scheduling decision (like other kmsgs (__death())), then it makes more
+sense.  The scheduling decision (as of now) was made in the interrupt handler
+when it decided to send the kernel msg.  In the future, we could split this into
+having the handler make the kthread active, and have the scheduler called to
+decide where and when to run the kthread.
index 74a1ad2..3be0d10 100644 (file)
@@ -11,6 +11,7 @@
 #include <kref.h>
 #include <slab.h>
 #include <pagemap.h>
 #include <kref.h>
 #include <slab.h>
 #include <pagemap.h>
+#include <kthread.h>
 
 /* All block IO is done assuming a certain size sector, which is the smallest
  * possible unit of transfer between the kernel and the block layer.  This can
 
 /* All block IO is done assuming a certain size sector, which is the smallest
  * possible unit of transfer between the kernel and the block layer.  This can
@@ -79,6 +80,7 @@ struct block_request {
        unsigned int                            flags;
        void                                            (*callback)(struct block_request *breq);
        void                                            *data;
        unsigned int                            flags;
        void                                            (*callback)(struct block_request *breq);
        void                                            *data;
+       struct semaphore                        sem;
        struct buffer_head                      **bhs;                          /* BHs describing the IOs */
        unsigned int                            nr_bhs;
        struct buffer_head                      *local_bhs[NR_INLINE_BH];
        struct buffer_head                      **bhs;                          /* BHs describing the IOs */
        unsigned int                            nr_bhs;
        struct buffer_head                      *local_bhs[NR_INLINE_BH];
index 6128317..733f454 100644 (file)
@@ -87,5 +87,9 @@ static inline struct kthread *__up_sem(struct semaphore *sem)
 void kthread_init(void);
 void sleep_on(struct semaphore *sem);
 void restart_kthread(struct kthread *kthread);
 void kthread_init(void);
 void sleep_on(struct semaphore *sem);
 void restart_kthread(struct kthread *kthread);
+/* Kmsg handler to launch/run a kthread.  This must be a routine message, since
+ * it does not return. */
+void __launch_kthread(struct trapframe *tf, uint32_t srcid, void *a0, void *a1,
+                         void *a2);
 
 #endif /* ROS_KERN_KTHREAD_H */
 
 #endif /* ROS_KERN_KTHREAD_H */
index 3a40339..b3cff35 100644 (file)
@@ -79,6 +79,7 @@ bool proc_controls(struct proc *SAFE actor, struct proc *SAFE target);
 void proc_run(struct proc *SAFE p);
 void proc_restartcore(struct proc *SAFE p, trapframe_t *SAFE tf);
 void proc_destroy(struct proc *SAFE p);
 void proc_run(struct proc *SAFE p);
 void proc_restartcore(struct proc *SAFE p, trapframe_t *SAFE tf);
 void proc_destroy(struct proc *SAFE p);
+void __proc_yield_s(struct proc *p, struct trapframe *tf);
 void proc_yield(struct proc *SAFE p, bool being_nice);
 void do_notify(struct proc *p, uint32_t vcoreid, unsigned int notif,
                struct notif_event *ne);
 void proc_yield(struct proc *SAFE p, bool being_nice);
 void do_notify(struct proc *p, uint32_t vcoreid, unsigned int notif,
                struct notif_event *ne);
index 5702b52..f6ae459 100644 (file)
@@ -109,18 +109,23 @@ int bdev_submit_request(struct block_device *bdev, struct block_request *breq)
        /* Faking an interrupt.  The handler runs in interrupt context btw */
        void x86_breq_handler(struct trapframe *tf, void *data)
        {
        /* Faking an interrupt.  The handler runs in interrupt context btw */
        void x86_breq_handler(struct trapframe *tf, void *data)
        {
-               /* Re-register the old dumb handler */
+               /* Turn off the interrupt, Re-register the old dumb handler */
+               set_core_timer(0);
                register_interrupt_handler(interrupt_handlers,
                                           LAPIC_TIMER_DEFAULT_VECTOR, timer_interrupt,
                                           NULL);
                register_interrupt_handler(interrupt_handlers,
                                           LAPIC_TIMER_DEFAULT_VECTOR, timer_interrupt,
                                           NULL);
+               /* In the future, we'll need to figure out which breq this was in
+                * response to */
                struct block_request *breq = (struct block_request*)data;
                if (breq->callback)
                        breq->callback(breq);
        }
        register_interrupt_handler(interrupt_handlers, LAPIC_TIMER_DEFAULT_VECTOR,
                                   x86_breq_handler, breq);
                struct block_request *breq = (struct block_request*)data;
                if (breq->callback)
                        breq->callback(breq);
        }
        register_interrupt_handler(interrupt_handlers, LAPIC_TIMER_DEFAULT_VECTOR,
                                   x86_breq_handler, breq);
-       /* Fake a 5ms delay */
-       set_core_timer(5000);
+       /* Fake a 5ms delay.  or 50... TODO for some reason, KVM needs a much larger
+        * wait time so that we can get to sleep before the interrupt arrives.  Set
+        * this to 5000 for "real" use (this is just faking anyway...) */
+       set_core_timer(50000);
 #else
        if (breq->callback)
                breq->callback(breq);
 #else
        if (breq->callback)
                breq->callback(breq);
@@ -132,20 +137,34 @@ int bdev_submit_request(struct block_device *bdev, struct block_request *breq)
 /* Helper method, unblocks someone blocked on sleep_on_breq(). */
 void generic_breq_done(struct block_request *breq)
 {
 /* Helper method, unblocks someone blocked on sleep_on_breq(). */
 void generic_breq_done(struct block_request *breq)
 {
-       /* TODO: BLK - unblock the kthread sleeping on this request */
+#ifdef __i386__        /* Sparc can't restart kthreads yet */
+       struct kthread *sleeper = __up_sem(&breq->sem);
+       if (!sleeper) {
+               /* this is odd, but happened a lot with kvm */
+               printk("[kernel] no one waiting on breq %08p\n", breq);
+               return;
+       }
+       /* For lack of anything better, send it to ourselves (so we handle it out of
+        * interrupt context, which we're still in). */
+       send_kernel_message(core_id(), __launch_kthread, (void*)sleeper, 0, 0,
+                           KMSG_ROUTINE);
+       assert(TAILQ_EMPTY(&breq->sem.waiters));
+#else
        breq->data = (void*)1;
        breq->data = (void*)1;
+#endif
 }
 
 /* Helper, pairs with generic_breq_done() */
 void sleep_on_breq(struct block_request *breq)
 {
 }
 
 /* Helper, pairs with generic_breq_done() */
 void sleep_on_breq(struct block_request *breq)
 {
-       /* TODO: BLK Block til we are done: data gets toggled in the completion.
-        * This only works if the completion happened first (for now) */
+       /* Since printk takes a while, this may make you lose the race */
+       printd("Sleeping on breq %08p\n", breq);
        assert(irq_is_enabled());
 #ifdef __i386__        /* Sparc isn't interrupt driven yet */
        assert(irq_is_enabled());
 #ifdef __i386__        /* Sparc isn't interrupt driven yet */
-       while (!breq->data)
-               cpu_relax();
+       sleep_on(&breq->sem);
 #else
 #else
+       /* Sparc can't block yet (TODO).  This only works if the completion happened
+        * first (for now) */
        assert(breq->data);
 #endif
 }
        assert(breq->data);
 #endif
 }
@@ -242,6 +261,7 @@ found:
        breq->flags = BREQ_READ;
        breq->callback = generic_breq_done;
        breq->data = 0;
        breq->flags = BREQ_READ;
        breq->callback = generic_breq_done;
        breq->data = 0;
+       init_sem(&breq->sem, 0);
        breq->bhs = breq->local_bhs;
        breq->bhs[0] = bh;
        breq->nr_bhs = 1;
        breq->bhs = breq->local_bhs;
        breq->bhs[0] = bh;
        breq->nr_bhs = 1;
index 8211e05..ccb47bd 100644 (file)
@@ -715,6 +715,7 @@ int ext2_readpage(struct page_map *pm, struct page *page)
        breq->flags = BREQ_READ;
        breq->callback = generic_breq_done;
        breq->data = 0;
        breq->flags = BREQ_READ;
        breq->callback = generic_breq_done;
        breq->data = 0;
+       init_sem(&breq->sem, 0);
        breq->bhs = breq->local_bhs;
        breq->nr_bhs = 0;
        /* Pack the BH pointers in the block request */
        breq->bhs = breq->local_bhs;
        breq->nr_bhs = 0;
        /* Pack the BH pointers in the block request */
index 2d0f2c5..a1a4213 100644 (file)
@@ -10,6 +10,7 @@
 #include <page_alloc.h>
 #include <pmap.h>
 #include <smp.h>
 #include <page_alloc.h>
 #include <pmap.h>
 #include <smp.h>
+#include <schedule.h>
 
 struct kmem_cache *kthread_kcache;
 
 
 struct kmem_cache *kthread_kcache;
 
@@ -86,7 +87,7 @@ unwind_sleep_prep:
        /* We get here if we should not sleep on sem (the signal beat the sleep).
         * Note we are not optimizing for cases where the signal won. */
        spin_unlock(&sem->lock);
        /* We get here if we should not sleep on sem (the signal beat the sleep).
         * Note we are not optimizing for cases where the signal won. */
        spin_unlock(&sem->lock);
-       printk("Didn't sleep, unwinding...\n");
+       printd("Didn't sleep, unwinding...\n");
        /* Restore the core's current and default stacktop */
        current = kthread->proc;                        /* arguably unnecessary */
        if (kthread->proc)
        /* Restore the core's current and default stacktop */
        current = kthread->proc;                        /* arguably unnecessary */
        if (kthread->proc)
@@ -137,6 +138,39 @@ void restart_kthread(struct kthread *kthread)
        if (current)
                kref_put(&current->kref);
        current = kthread->proc;
        if (current)
                kref_put(&current->kref);
        current = kthread->proc;
+       if (kthread->proc_tf)
+               set_current_tf(kthread->proc_tf);
        /* Finally, restart our thread */
        pop_kernel_tf(&kthread->context);
 }
        /* Finally, restart our thread */
        pop_kernel_tf(&kthread->context);
 }
+
+/* Kmsg handler to launch/run a kthread.  This must be a routine message, since
+ * it does not return.  Furthermore, like all routine kmsgs that don't return,
+ * this needs to handle the fact that it won't return to the given TF (which is
+ * a proc's TF, since this was routine). */
+void __launch_kthread(struct trapframe *tf, uint32_t srcid, void *a0, void *a1,
+                         void *a2)
+{
+       struct kthread *kthread = (struct kthread*)a0;
+       struct proc *cur_proc = current;
+       /* If there is no proc running, don't worry about not returning. */
+       if (cur_proc) {
+               if (cur_proc != kthread->proc) {
+                       /* we're running the kthread from a different proc.  For now, we
+                        * can't be _M, since that would be taking away someone's vcore to
+                        * process another process's work. */
+                       assert(cur_proc->state == PROC_RUNNING_S);
+                       spin_lock(&cur_proc->proc_lock);
+                       /* Wrap up / yield the current _S proc, which calls schedule_proc */
+                       __proc_yield_s(cur_proc, tf);
+                       spin_unlock(&cur_proc->proc_lock);
+               } else {
+                       assert(cur_proc->state == PROC_RUNNING_M);
+                       /* TODO: might need to do something here, though it will depend on
+                        * how we handle async local calls. */
+               }
+
+       }
+       restart_kthread(kthread);
+       assert(0);
+}
index 160129d..efcca2e 100644 (file)
@@ -115,7 +115,13 @@ void manager_brho(void)
 
        /* I usually want this */
        schedule();
 
        /* I usually want this */
        schedule();
-       printk("No work to do (schedule returned)\n");
+       process_routine_kmsg(); /* maybe do this before schedule() */
+
+       enable_irq();
+       /* this ghetto hack really wants to wait for an interrupt, but time out */
+       udelay(60000);  /* wait for IO when there really is nothing to do */
+       process_routine_kmsg();
+       printd("No work to do (schedule returned)\n");
        monitor(0);
 
        // for testing taking cores, check in case 1 for usage
        monitor(0);
 
        // for testing taking cores, check in case 1 for usage
index 8240be7..692ae16 100644 (file)
@@ -569,9 +569,10 @@ static void __proc_startcore(struct proc *p, trapframe_t *tf)
  * returning from local traps and such. */
 void proc_restartcore(struct proc *p, trapframe_t *tf)
 {
  * returning from local traps and such. */
 void proc_restartcore(struct proc *p, trapframe_t *tf)
 {
-       // TODO: proc_restartcore shouldn't ever be called with tf != current_tf,
-       // so the parameter should probably be removed outright.
-       assert(current_tf == tf);
+       if (current_tf != tf) {
+               printk("Current_tf: %08p, tf: %08p\n", current_tf, tf);
+               panic("Current TF is jacked...");
+       }
 
        /* Need ints disabled when we return from processing (race) */
        disable_irq();
 
        /* Need ints disabled when we return from processing (race) */
        disable_irq();
@@ -722,6 +723,16 @@ static uint32_t get_pcoreid(struct proc *p, uint32_t vcoreid)
        return p->procinfo->vcoremap[vcoreid].pcoreid;
 }
 
        return p->procinfo->vcoremap[vcoreid].pcoreid;
 }
 
+/* Helper function: yields / wraps up current_tf and schedules the _S */
+void __proc_yield_s(struct proc *p, struct trapframe *tf)
+{
+       assert(p->state == PROC_RUNNING_S);
+       p->env_tf= *tf;
+       env_push_ancillary_state(p);                    /* TODO: (HSS) */
+       __proc_set_state(p, PROC_RUNNABLE_S);
+       schedule_proc(p);
+}
+
 /* Yields the calling core.  Must be called locally (not async) for now.
  * - If RUNNING_S, you just give up your time slice and will eventually return.
  * - If RUNNING_M, you give up the current vcore (which never returns), and
 /* Yields the calling core.  Must be called locally (not async) for now.
  * - If RUNNING_S, you just give up your time slice and will eventually return.
  * - If RUNNING_M, you give up the current vcore (which never returns), and
@@ -762,10 +773,7 @@ void proc_yield(struct proc *SAFE p, bool being_nice)
 
        switch (p->state) {
                case (PROC_RUNNING_S):
 
        switch (p->state) {
                case (PROC_RUNNING_S):
-                       p->env_tf= *current_tf;
-                       env_push_ancillary_state(p); // TODO: (HSS)
-                       __proc_set_state(p, PROC_RUNNABLE_S);
-                       schedule_proc(p);
+                       __proc_yield_s(p, current_tf);
                        break;
                case (PROC_RUNNING_M):
                        printd("[K] Process %d (%p) is yielding on vcore %d\n", p->pid, p,
                        break;
                case (PROC_RUNNING_M):
                        printd("[K] Process %d (%p) is yielding on vcore %d\n", p->pid, p,
index f49426e..ad6a094 100644 (file)
@@ -76,12 +76,6 @@ void schedule(void)
                kref_put(&p->kref);
        } else {
                spin_unlock_irqsave(&runnablelist_lock);
                kref_put(&p->kref);
        } else {
                spin_unlock_irqsave(&runnablelist_lock);
-               /* while this is problematic, we really don't have anything to
-         * do and could run off the end of the kernel stack if we just
-         * smp_idle -> manager -> schedule -> smp_idle. */
-               printk("No processes to schedule, enjoy the Monitor!\n");
-               while (1)
-                       monitor(NULL);
        }
        return;
 }
        }
        return;
 }
index 8ccb931..c14771a 100644 (file)
@@ -50,12 +50,12 @@ void smp_idle(void)
                        cpu_halt();
                }
        } else {
                        cpu_halt();
                }
        } else {
-               /* techincally, this check is arch dependent.  i want to know if it
-                * happens.  the enabling/disabling could be interesting. */
                enable_irqsave(&state);
                enable_irqsave(&state);
-               if (!STAILQ_EMPTY(&myinfo->immed_amsgs) ||
-                       !STAILQ_EMPTY(&myinfo->routine_amsgs)) 
-                       printk("[kernel] kmsgs in smp_idle() on a management core.\n");
+               /* this makes us wait to enter the manager til any IO is done (totally
+                * arbitrary 10ms), so we can handle the routine message that we
+                * currently use to do the completion.  Note this also causes us to wait
+                * 10ms regardless of how long the IO takes.  This all needs work. */
+               //udelay(10000); /* done in the manager for now */
                process_routine_kmsg();
                disable_irqsave(&state);
                manager();
                process_routine_kmsg();
                disable_irqsave(&state);
                manager();