lkml.org 
[lkml]   [2012]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 00/18] SMP: Boot and CPU hotplug refactoring - Part 1
    On Fri, 20 Apr 2012, Suresh Siddha wrote:
    > On Fri, 2012-04-20 at 15:47 +0200, Thomas Gleixner wrote:
    > > On Fri, 20 Apr 2012, Peter Zijlstra wrote:
    > >
    > > > On Fri, 2012-04-20 at 13:05 +0000, Thomas Gleixner wrote:
    > > > > This first part moves the idle thread management for non-boot cpus
    > > > > into the core. fork_idle() is called in a workqueue as it is
    > > > > implemented in a few architectures already. This is necessary when not
    > > > > all cpus are brought up by the early boot code as otherwise we would
    > > > > take a ref on the user task VM of the thread which brings the cpu up
    > > > > via the sysfs interface.
    > > >
    > > > So I was thinking about this and I think we should make that kthreadd
    > > > instead of a random workqueue thread due to all that cgroup crap. People
    > > > are wanting to place all sorts of kernel threads in cgroups and I'm
    > > > still arguing that kthreadd should not be allowed in cgroups.
    > >
    > > So your fear is that the idle_thread will end up in some random cgroup
    > > because some illdesigned user space code decided to stick kernel
    > > threads into cgroups.
    > >
    > > Can we please have some sanity restrictions on this cgroup muck? I
    > > don't care when user space creates cgroups in circles, but holding the
    > > whole kernel hostage of this madness is going too far.
    > >
    >
    > Also, do we really need the workqueue/kthreadd based allocation? Just
    > like the percpu areas getting allocated for each possible cpu during
    > boot, shouldn't we extend this to the per-cpu idle threads too? So
    > something like the appended should be ok to?

    The idea is correct, there are just a few problems :)

    > Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
    > ---
    > diff --git a/kernel/cpu.c b/kernel/cpu.c
    > index 05c46ba..a5144ab 100644
    > --- a/kernel/cpu.c
    > +++ b/kernel/cpu.c
    > @@ -303,10 +303,6 @@ static int __cpuinit _cpu_up(unsigned int cpu, int tasks_frozen)
    >
    > cpu_hotplug_begin();
    >
    > - ret = smpboot_prepare(cpu);
    > - if (ret)
    > - goto out;
    > -

    If we failed to allocate an idle_thread for this cpu in smp_init()
    then we unconditionally call __cpu_up() with a NULL pointer. That
    might surprise the arch code :)

    Aside of that, we now miss to reinitialize the idle thread. We call
    init_idle() once when we allocate the thread, but not after a cpu
    offline operation. That might leave stuff in weird state.

    Thanks,

    tglx


    \
     
     \ /
      Last update: 2012-05-03 12:21    [W:0.043 / U:90.548 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site