Merged to master at 3139de080ef0..c406c2620abb (from, to]

You can see the entire diff with 'git diff' or at
https://github.com/brho/akaros/compare/3139de080ef0...c406c2620abb



On 2016-03-29 at 17:27 ron minnich <[email protected]> wrote:
> The following changes since commit
> b3fd8951724d0cdbe1ca38c2b613b8d78fac241a:
> 
>   WIP-pop (2016-03-25 18:17:32 -0400)
> 
> are available in the git repository at:
> 
>   [email protected]:rminnich/akaros procnet
> 
> for you to fetch changes up to
> 6aef40a86b4a61591571d4233464f88704371a90:
> 
>   Add a control file in #cons to support killing children. (2016-03-29
> 10:24:23 -0700)
> 
> ----------------------------------------------------------------
> Ronald G. Minnich (1):
>       Add a control file in #cons to support killing children.
> 
>  kern/drivers/dev/cons.c | 66
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> https://github.com/rminnich/akaros/compare/rminnich:net...procnet
> 
> On Mon, Mar 28, 2016 at 9:20 AM ron minnich <[email protected]>
> wrote:
> 
> > Since this is essentially a walk of the CAR of a list, which should
> > be done in a few pointer walks, is there a giant lock I can just
> > use to lock the pid structs and get 'er done? I think anything more
> > is is overkill.
> >
> > All that we should do is send a SIGINT and get out.
> >
> > WIth luck this all goes away in a little while anyway.
> >
> > But I'm going to redo per your suggestion. I was just not sure of
> > the rules on locking the pid structs, and I knew I'd get the
> > straight info from you quickly :-)
> >
> > ron
> >
> > On Mon, Mar 28, 2016 at 8:46 AM Barret Rhoden <[email protected]>
> > wrote:
> >
> >> 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.
> >>
> >
> 

-- 
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.

Reply via email to