lkml.org 
[lkml]   [2007]   [Feb]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
    On 02/17, Rafael J. Wysocki wrote:
    >
    > On Saturday, 17 February 2007 22:34, Oleg Nesterov wrote:
    > >
    > > static inline int is_user_space(struct task_struct *p)
    > > {
    > > return p->mm && !(p->flags & PF_BORROWED_MM);
    > > }
    > >
    > > This doesn't look right. First, an exiting task has ->mm == NULL after
    > > do_exit()->exit_mm(). Probably not a problem. However, PF_BORROWED_MM
    > > check is racy without task_lock(), so we can have a false positive as
    > > well. Is it ok? We can freeze aio_wq prematurely.
    >
    > Right now aio_wq is not freezeable (PF_NOFREEZE).

    Right now yes, but we are going to change this?

    > > cancel_freezing(p);
    > > continue;
    > >
    > > Is it right? Shouldn't we increment "todo" counter?
    >
    > No. It would be wrong to do that, because TASK_TRACED tasks with frozen
    > parents cannot be frozen any further.

    TASK_TRACED task could be woken by SIGKILL. cancel_freezing() clears TIF_FREEZE.
    The task may start do_exit() when try_to_freeze_tasks() returns "success".
    Probably not a problem.

    > > if (!p->vfork_done)
    > > freeze_process(p);
    > >
    > >
    > > Racy. do_fork(CLONE_VFORK) first does copy_process() which puts 'p' on
    > > the task list and unlocks tasklist_lock. This means that 'p' is visible
    > > to try_to_freeze_tasks(), and p->vfork_done == NULL. try_to_freeze_tasks()
    > > sets TIF_FREEZE.
    > >
    > > Now, do_fork() continues, sets ->vfork_done, p goes to user space, notices
    > > the fake signal and goes to refrigerator while its parent is blocked on
    > > "struct completion vfork". Freezing failed.
    >
    > You are right, but this has never happened, AFAICS.
    >
    > > So, shouldn't we do
    > >
    > > if (p->vfork_done)
    > > cancel_freezing(p);
    > >
    > > instead?
    >
    > I don't think so. If p hasn't got TIF_FREEZE set yet or it has already been
    > frozen, cancel_freezing(p) is a noop.

    Yes, I misread cancel_freezing(), it doesn't wake up the task if it is frozen.

    > Alternatively, we can move the check into refrigerator(), like this:
    >
    > --- linux-2.6.20-git13.orig/kernel/power/process.c
    > +++ linux-2.6.20-git13/kernel/power/process.c
    > @@ -39,6 +39,11 @@ void refrigerator(void)
    > /* Hmm, should we be allowed to suspend when there are realtime
    > processes around? */
    > long save;
    > +
    > + /* Freeze the task unless there is a vfork completion pending */
    > + if (current->vfork_done)
    > + return;
    > +

    This means that "current" returns to user space (get_signal_to_deliver
    will clear TIF_SIGPENDING) and runs. While try_to_freeze_tasks() thinks
    it is frozen.

    Oleg.

    -
    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: 2007-02-18 00:45    [from the cache]
    ©2003-2014 Jasper Spaans. hosted at Digital Ocean