lkml.org 
[lkml]   [2012]   [Jul]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/9] workqueue: perform cpu down operations from low priority cpu_notifier()
    On Tue, Jul 17, 2012 at 10:12:21AM -0700, Tejun Heo wrote:
    > Currently, all workqueue cpu hotplug operations run off
    > CPU_PRI_WORKQUEUE which is higher than normal notifiers. This is to
    > ensure that workqueue is up and running while bringing up a CPU before
    > other notifiers try to use workqueue on the CPU.
    >
    > Per-cpu workqueues are supposed to remain working and bound to the CPU
    > for normal CPU_DOWN_PREPARE notifiers. This holds mostly true even
    > with workqueue offlining running with higher priority because
    > workqueue CPU_DOWN_PREPARE only creates a bound trustee thread which
    > runs the per-cpu workqueue without concurrency management without
    > explicitly detaching the existing workers.
    >
    > However, if the trustee needs to create new workers, it creates
    > unbound workers which may wander off to other CPUs while
    > CPU_DOWN_PREPARE notifiers are in progress. Furthermore, if the CPU
    > down is cancelled, the per-CPU workqueue may end up with workers which
    > aren't bound to the CPU.
    >
    > While reliably reproducible with a convoluted artificial test-case
    > involving scheduling and flushing CPU burning work items from CPU down
    > notifiers, this isn't very likely to happen in the wild, and, even
    > when it happens, the effects are likely to be hidden by the following
    > successful CPU down.
    >
    > Fix it by using different priorities for up and down notifiers - high
    > priority for up operations and low priority for down operations.

    Cool!!!

    This certainly provides another data point in favor of running down
    notifiers in the opposite order from up notifiers. ;-)

    This series passes light rcutorture/hotplug testing, will be testing
    it more.

    Thanx, Paul

    > Workqueue cpu hotplug operations will soon go through further cleanup.
    >
    > Signed-off-by: Tejun Heo <tj@kernel.org>
    > Cc: stable@vger.kernel.org
    > ---
    > include/linux/cpu.h | 5 +++--
    > kernel/workqueue.c | 38 +++++++++++++++++++++++++++++++++++++-
    > 2 files changed, 40 insertions(+), 3 deletions(-)
    >
    > diff --git a/include/linux/cpu.h b/include/linux/cpu.h
    > index 2e9b9eb..ce7a074 100644
    > --- a/include/linux/cpu.h
    > +++ b/include/linux/cpu.h
    > @@ -73,8 +73,9 @@ enum {
    > /* migration should happen before other stuff but after perf */
    > CPU_PRI_PERF = 20,
    > CPU_PRI_MIGRATION = 10,
    > - /* prepare workqueues for other notifiers */
    > - CPU_PRI_WORKQUEUE = 5,
    > + /* bring up workqueues before normal notifiers and down after */
    > + CPU_PRI_WORKQUEUE_UP = 5,
    > + CPU_PRI_WORKQUEUE_DOWN = -5,
    > };
    >
    > #define CPU_ONLINE 0x0002 /* CPU (unsigned)v is up */
    > diff --git a/kernel/workqueue.c b/kernel/workqueue.c
    > index 4fa9e35..f59b7fd 100644
    > --- a/kernel/workqueue.c
    > +++ b/kernel/workqueue.c
    > @@ -3644,6 +3644,41 @@ err_destroy:
    > return NOTIFY_BAD;
    > }
    >
    > +/*
    > + * Workqueues should be brought up before normal priority CPU notifiers.
    > + * This will be registered high priority CPU notifier.
    > + */
    > +static int __devinit workqueue_cpu_up_callback(struct notifier_block *nfb,
    > + unsigned long action,
    > + void *hcpu)
    > +{
    > + switch (action & ~CPU_TASKS_FROZEN) {
    > + case CPU_UP_PREPARE:
    > + case CPU_UP_CANCELED:
    > + case CPU_DOWN_FAILED:
    > + case CPU_ONLINE:
    > + return workqueue_cpu_callback(nfb, action, hcpu);
    > + }
    > + return NOTIFY_OK;
    > +}
    > +
    > +/*
    > + * Workqueues should be brought down after normal priority CPU notifiers.
    > + * This will be registered as low priority CPU notifier.
    > + */
    > +static int __devinit workqueue_cpu_down_callback(struct notifier_block *nfb,
    > + unsigned long action,
    > + void *hcpu)
    > +{
    > + switch (action & ~CPU_TASKS_FROZEN) {
    > + case CPU_DOWN_PREPARE:
    > + case CPU_DYING:
    > + case CPU_POST_DEAD:
    > + return workqueue_cpu_callback(nfb, action, hcpu);
    > + }
    > + return NOTIFY_OK;
    > +}
    > +
    > #ifdef CONFIG_SMP
    >
    > struct work_for_cpu {
    > @@ -3839,7 +3874,8 @@ static int __init init_workqueues(void)
    > unsigned int cpu;
    > int i;
    >
    > - cpu_notifier(workqueue_cpu_callback, CPU_PRI_WORKQUEUE);
    > + cpu_notifier(workqueue_cpu_up_callback, CPU_PRI_WORKQUEUE_UP);
    > + cpu_notifier(workqueue_cpu_down_callback, CPU_PRI_WORKQUEUE_DOWN);
    >
    > /* initialize gcwqs */
    > for_each_gcwq_cpu(cpu) {
    > --
    > 1.7.7.3
    >
    > --
    > 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: 2012-07-21 00:41    [W:0.034 / U:0.016 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site