lkml.org 
[lkml]   [2011]   [Nov]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [RFC] cgroups: freezer -- Allow to attach a task to a frozen cgroup
    From
    >        void (*cancel_attach)(struct cgroup_subsys *ss, struct cgroup *cgrp,
    > -                             struct task_struct *tsk);
    > +                             struct cgroup *old_cgrp, struct task_struct *tsk);

    I'm not shure, that we need old_cgrp, because when cancel_attach is
    executed, a task is in old cgroup and old_cgrp = task_cgroup(tsk);

    ...

    >
    > +static int freezer_task_transition(struct task_struct *task, int state_to)
    > +{
    > +       int retval = 0;
    > +
    > +       switch (state_to) {
    > +       case CGROUP_THAWED:
    > +               thaw_process(task);
    > +               break;
    > +       case CGROUP_FROZEN:
    > +       case CGROUP_FREEZING:
    > +               if (freeze_task(task, true)) {
    > +                       if (!frozen(task)) {
    > +                               if (!freezing(task) &&
    > +                                   !freezer_should_skip(task))
    > +                                       retval = -EBUSY;
    > +                       }
    > +               }
    > +               break;
    > +       }
    > +
    > +       return retval;
    > +}
    > +
    > +static int freezer_can_attach_task(struct cgroup *cgroup, struct task_struct *task)
    > +{
    > +       struct freezer *old_freezer;
    > +       struct freezer *freezer;
    > +
    > +       int goal_state, orig_state;
    > +       int retval = 0;
    > +
    > +       old_freezer = task_freezer(task);
    > +       freezer = cgroup_freezer(cgroup);
    > +
    > +       spin_lock_irq(&freezer->lock);
    > +
    > +       if (!spin_trylock_irq(&old_freezer->lock)) {
    > +               retval = -EBUSY;

    I think EBUSY is not a good idea in this place. We can do something
    like double_rq_lock.

    .....

    > +
    > +static void freezer_cancel_attach(struct cgroup_subsys *ss,
    > +                                 struct cgroup *cgroup,
    > +                                 struct cgroup *old_cgroup,
    > +                                 struct task_struct *task)
    > +{
    > +       struct freezer *freezer = cgroup_freezer(old_cgroup);
    > +       int retval = 0;
    > +
    > +       spin_lock_irq(&freezer->lock);
    > +       retval = freezer_task_transition(task, freezer->state);
    > +       if (retval)
    > +               pr_warning("freezer: Can't move task (pid %d) to %s state\n",
    > +                          task_pid_nr(task),
    > +                          freezer_state_strs[freezer->state]);

    It's strange. A rollback can't fail. We have three situations:

    frozen -> frozen
    thawed -> frozen
    frozen -> thawed

    In first and second cases cancel_request can't fail.
    In the third we have a problem, which may be solved if we will call
    thaw_process(task) from attach_task(), we can do that, because
    thaw_process() can't fail. It solves a problem, because
    freezer_cancel_attach will be executed for the first and second cases
    only.

    If my suggestion is correct, we can replace pr_warning on BUG_ON

    > +       spin_unlock_irq(&freezer->lock);
    > +}
    > +
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-11-28 14:13    [W:0.024 / U:0.384 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site