lkml.org 
[lkml]   [2005]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 2/5] try2: x86_64: CPU hotplug support.
    On Mon, Jun 06, 2005 at 03:11:56PM -0700, Andrew Morton wrote:
    > Ashok Raj <ashok.raj@intel.com> wrote:
    > >
    > > Experimental CPU hotplug patch for x86_64
    >
    > What does "experimental" mean?

    Well, stictly since these sections are still under CONFIG_EXPERIMENTAL
    in arch/x86_64/Kconfig. Evolving... ?

    >
    > > +
    > > + if (!keventd_up() || current_is_keventd())
    > > + work.func(work.data);
    > > + else {
    > > + schedule_work(&work);
    > > + wait_for_completion(&c_idle.done);
    > > + }
    >
    > This shouldn't be diddling with workqueue internals. Why is this code
    > here? If the workqueue API is inadequate then we should prefer to extend
    > it rather than working around any shortcoming.

    This has been around for ages.. even in ia64 code. For forking idle threads
    we want to do them in clean state so we dont acquire state from threads
    from where the cpu_up is being invoked. Hence we want them to start from
    keventd() threads. But when system boot is happening, there is no keventd()
    yet, hence we need to create them right away.

    the other problem we ran into was ACPI code that handles physical cpu hotplug
    also queues to keventd(), this becomes permanently blocking when called from
    code already running in kevend().

    > > + Dprintk ("do_boot_cpu %d Already started\n", cpu);
    >
    > Please try to adopt a consistent coding style.

    I was actually trying to be consistent :-), rest of the debug code
    was under Dprintk() hence didnt want to use a new style. Not sure
    what you need here exactly. Do you want to convert the rest of the code to
    not use Dprintk()? or just leave this with a printk? I dont have a
    particular preference here... i would rather leave it with Dprintk() as the
    rest of the debug code.
    >
    > Using printk("%s", __FUNCTION__); is preferred, as it will still work if
    > someone later refactors this code into a new function. (It can increase
    > code size. Or decrease it if the string gets shared. But that's moot if
    > the code is inside a normally-disabled macro like Dprintk. Whatever that
    > is.)
    >
    > > +static void
    > > +remove_siblinginfo(int cpu)
    >
    > Unneeded newline here.

    I can remove.. when iam inside the file, iam used to search for fn;s from
    start of line.. no biggie.. can revert.

    Cheers,
    ashok
    -
    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: 2005-06-07 01:53    [W:0.023 / U:32.780 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site