On Mon, 2005-09-12 at 15:05 -0700, Paul Menage wrote:
Thanks for your prompt response Paul.
> 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.
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 :)
>
> >
> > Also, how is the control getting out of the while loop ?
>
> Eventually we should reach a class in the hierarchy that isn't in the
> process of being deleted. (Actually, do we ever try to destroy the
> default taskclass? If so we'd need to handle a NULL parent at the end
> of the while loop.)
No, we do not. This logic should work.
>
> >
> > 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 ? 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 would set the dead flag in ckrm.c:ckrm_release_core_class(), than in
> > task class specific code.
>
> I agree that would be cleaner. The problem with that is that the flag
> needs to be set before starting to reclassify out the existing tasks,
> which currently happens before calling ckrm_release_core_class().
Valid point.
Thanks again.
chandra
>
> Paul
>
--
----------------------------------------------------------------------
Chandra Seetharaman | Be careful what you choose....
- [EMAIL PROTECTED] | .......you may get it.
----------------------------------------------------------------------
-------------------------------------------------------
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
