On 9/12/05, Chandra Seetharaman <[EMAIL PROTECTED]> wrote:
> > On 9/12/05, Chandra Seetharaman <[EMAIL PROTECTED]> wrote:
> > > > /* put into new class */
> > > > class_lock(class_core(newcls));
> > > >+ while (class_core(newcls)->dead) {
> > > >+ /* If this class is dead, we need to move up the
> > > >hierarchy */
> > > >+ struct ckrm_core_class *parent;
> > > >+ parent = class_core(newcls)->hnode.parent;
> > > >+ ckrm_core_grab(parent);
> > > >+ class_unlock(class_core(newcls));
> > > >+ printk(KERN_INFO "Redirecting task: %s -> %s\n",
> > > class_core(newcls)->name, parent->name);
> > > >+ ckrm_core_drop(class_core(newcls));
> > > >+ class_lock(parent);
> > > >+ newcls = class_type(struct ckrm_task_class, parent);
> > > >+ }
> > >
> > > I do not understand what this section of code is doing.
> >
> > It's basically:
> >
> > while newclass->dead:
> > newclass = newclass->parent
> >
> > but with appropriate locking and reference counting.
>
> Now i see it :)...
>
> One slight problem... In other places locking is done top down(i.e if
> both parent's and child's lock need to be held, we first acquire parent
> and then the child). It is opposite here, which might lead to a ABBA
> deadlock.
My code never tries to hold the lock on two classes simultaneously,
though - the logic is
grab parent
unlock child
drop child
lock parent
which should be safe in the situation that you describe.
>
> I think it is ok to assign the task to the default class if the parent
> is also dying. comments anybody ? (it will also remove the fear of the
> recursive loop :)
I'd much rather have the task go to the parent if the destination
class is dying.
I want to use CKRM for job tracking; all processes in a job will be in
a CKRM class, and each of these classes will be children of a single
parent class. I need to be able to kill all the processes in a job;
this is inherently racy, since a process could be forking at the point
when I kill the processes and delete the class.
If such forked processes classified into the parent, then I can safely
say that any processes listed in the parent class are stray processes
belonging to dead jobs, and I can kill them. If they're reclassified
to the default class in that situation then I have no way of
identifying them.
>
> >
> > >
> > > Any logic to use a different newcls should be placed before the block of
> > > code that sets the task class(not after).
> > >
> >
> > It is - the code that sets the task class in the general case is
> > directly after the while loop that I added.
>
> In my mind, newcls is the appropriate new class before the block :)
> -----------
> /* Take out of old class and drop the oldcore. */
> if ((drop_old_cls = (curcls != NULL))) {
> class_lock(class_core(curcls));
> :
> :
> }
> -----------
>
> but I agree that functionally you are right.
>
> Did you do it for holding the lock ?
Yes.
> If so, it can be simplified by
> making 'dead' an atomic variable, it will also help solve the ABBA
> problem i 've mentioned above.
I don't think 'dead' can be a simple atomic variable - the "check
dead/add task" needs to be atomic with respect to the "mark dead"
otherwise we can have the race:
A checks "dead" on class C
B sets "dead" on class C
B reclassifies tasks from class C (maybe none?)
A adds self to C
Paul
-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
ckrm-tech mailing list
https://lists.sourceforge.net/lists/listinfo/ckrm-tech
