Hi - On 2016-03-25 at 16:14 ron minnich <[email protected]> wrote: > The following changes since commit > 24c7dd20c8a5a6983083c97d7492a994c67355d0: > > Upgrade parlib fp state handling, use proc_global_info (XCC) > (2016-03-22 16:54:53 -0400) > > are available in the git repository at: > > [email protected]:rminnich/akaros killkids
> From c361c5f817d55b54be8a5b4f16c0ac9c42da4110 Mon Sep 17 00:00:00 2001 > From: "Ronald G. Minnich" <[email protected]> > Date: Fri, 25 Mar 2016 08:59:42 -0700 > Subject: Add a control file in #cons to support killing children. > > This is needed for ssh support for ^C. > The ssh server, when it sees a ^C, opens and writes a command > to #cons/killkids. > > Right now the command is ignored, but that might change. Is this a reference to CMkillkid? How does that actually work? As far as I can tell from the code, the CMkillkid case only gets called when there is a write to #cons/reboot (Qreboot). We might not need any of the CMkillkid stuff. > diff --git a/kern/drivers/dev/cons.c b/kern/drivers/dev/cons.c > enum { > @@ -619,6 +625,8 @@ static struct dirtab consdir[] = { > {"config", {Qconfig}, 0, 0444}, > {"cons", {Qcons}, 0, 0660}, > {"consctl", {Qconsctl}, 0, 0220}, > + // FIXME -- we don't have real permissions yet so we set it to 222, not > 220 > + {"killkid", {Qkillkid}, 0, 0220 | /* BOGUS */ 2}, I guess you needed the extra 2 for it to work at runtime? > + > +/* > + * quick depth first search for somone to kill. This is a reasonable > + * standin for "kill most distant child" that is also reasonably fast. > + * No locking, totaly unsafe. yep, this is ugly. Once we have cmux we > + * might not need it. But consider that most alternatives involve > + * walking proc in user mode ... yuck! > + */ > +void killkid(void) > +{ > + struct proc *victim = NULL, *kid; > + > + kid = current; > + while (kid) { > + victim = kid; > + kid = TAILQ_FIRST(&victim->children); > + } > + if (victim) { > + printk("killkid: killing %d\n", victim->pid); > + send_posix_signal(victim, SIGINT); > + } > +} Yeah, this is horribly racy. If a child exits or anything during this, we'll blow up the kernel. The children list is protected by cv_lock(&parent->child_wait); (that's a spinlock internally btw). It protects the children list, which is a source of references for all of a process's children. I think it'd be safe to grab the locks as you went down the list, recursively. We'd be establishing a lock ordering from parent->child, but that's probably okay. Especially if we get rid of this code eventually. Another approach would be to lock briefly and up the refcnt at each step through the parent-child chain. This might be better. Here's why: It's probably safe to hold the locks while signalling. Though if we ever change things, like call proc_destroy() (a more serious 'kill') while holding the locks, then we'll have deadlock issues. If we use refcnts to "hold our place" as we walk down the line, we'll be safe from deadlock. So maybe: find_first_kid: // helper, 0 on failure, incref'd child on success grab my lock kid = TAILQ_FIRST(&me->children); if (kid) proc_incref(kid, 1); unlock my lock return kid kill_kid: // do you want to kill the caller? or only kids? victim = me; /* start the loop with an incref'd victim */ proc_incref(victim, 1); loop: child = find_first_kid(victim) if (!child) break; /* no longer need old victim, we have the ref on child */ proc_decref(victim) victim = child; /* now we have a victim. might be 'me' still. we have a ref on it and we need to decref when done */ if (!(victim == me)) // optional 'me' check send_sig_int proc_decref(victim) /* decref no matter what */ I think that will work and be relatively safe. =) Barret -- You received this message because you are subscribed to the Google Groups "Akaros" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. For more options, visit https://groups.google.com/d/optout.
