cpu/hotplug: Rework callback invocation logic

This is preparation for the following patch.
This rework here changes the arguments of cpuhp_invoke_callback(). It
passes now `state' and whether `startup' or `teardown' callback should
be invoked. The callback then is looked up by the function.

The following is a clanup of callers:
- cpuhp_issue_call() has one argument less
- struct cpuhp_cpu_state (which is used by the hotplug thread) gets also
  its callback removed. The decision if it is a single callback
  invocation moved to the `single' variable. Also a `bringup' variable
  has been added to distinguish between startup and teardown callback.
- take_cpu_down() needs to start one step earlier. We always get here
  via CPUHP_TEARDOWN_CPU callback. Before that change cpuhp_ap_states +
  CPUHP_TEARDOWN_CPU pointed to an empty entry because TEARDOWN is saved
  in bp_states for this reason. Now that we use cpuhp_get_step() to
  lookup the state we must explicitly skip it in order not to invoke it
  twice.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: rt@linutronix.de
Link: http://lkml.kernel.org/r/1471024183-12666-2-git-send-email-bigeasy@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

diff --git a/kernel/cpu.c b/kernel/cpu.c
index ec12b726fa6f056dda3176055619fdf5ce06982a..d36d8e0abfb8ddbb4c65a4762d10edfaabdbe451 100644 (file)
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -37,8 +37,9 @@
  * @thread:    Pointer to the hotplug thread
  * @should_run:        Thread should execute
  * @rollback:  Perform a rollback
- * @cb_stat:   The state for a single callback (install/uninstall)
- * @cb:                Single callback function (install/uninstall)
+ * @single:    Single callback invocation
+ * @bringup:   Single callback bringup or teardown selector
+ * @cb_state:  The state for a single callback (install/uninstall)
  * @result:    Result of the operation
  * @done:      Signal completion to the issuer of the task
  */
@@ -49,8 +50,9 @@ struct cpuhp_cpu_state {
        struct task_struct      *thread;
        bool                    should_run;
        bool                    rollback;
+       bool                    single;
+       bool                    bringup;
        enum cpuhp_state        cb_state;
-       int                     (*cb)(unsigned int cpu);
        int                     result;
        struct completion       done;
 #endif
@@ -79,24 +81,43 @@ static DEFINE_MUTEX(cpuhp_state_mutex);
 static struct cpuhp_step cpuhp_bp_states[];
 static struct cpuhp_step cpuhp_ap_states[];
 
+static bool cpuhp_is_ap_state(enum cpuhp_state state)
+{
+       /*
+        * The extra check for CPUHP_TEARDOWN_CPU is only for documentation
+        * purposes as that state is handled explicitly in cpu_down.
+        */
+       return state > CPUHP_BRINGUP_CPU && state != CPUHP_TEARDOWN_CPU;
+}
+
+static struct cpuhp_step *cpuhp_get_step(enum cpuhp_state state)
+{
+       struct cpuhp_step *sp;
+
+       sp = cpuhp_is_ap_state(state) ? cpuhp_ap_states : cpuhp_bp_states;
+       return sp + state;
+}
+
 /**
  * cpuhp_invoke_callback _ Invoke the callbacks for a given state
  * @cpu:       The cpu for which the callback should be invoked
  * @step:      The step in the state machine
- * @cb:                The callback function to invoke
+ * @bringup:   True if the bringup callback should be invoked
  *
  * Called from cpu hotplug and from the state register machinery
  */
-static int cpuhp_invoke_callback(unsigned int cpu, enum cpuhp_state step,
-                                int (*cb)(unsigned int))
+static int cpuhp_invoke_callback(unsigned int cpu, enum cpuhp_state state,
+                                bool bringup)
 {
        struct cpuhp_cpu_state *st = per_cpu_ptr(&cpuhp_state, cpu);
+       struct cpuhp_step *step = cpuhp_get_step(state);
+       int (*cb)(unsigned int cpu) = bringup ? step->startup : step->teardown;
        int ret = 0;
 
        if (cb) {
-               trace_cpuhp_enter(cpu, st->target, step, cb);
+               trace_cpuhp_enter(cpu, st->target, state, cb);
                ret = cb(cpu);
-               trace_cpuhp_exit(cpu, st->state, step, ret);
+               trace_cpuhp_exit(cpu, st->state, state, ret);
        }
        return ret;
 }
@@ -371,62 +392,55 @@ static int bringup_cpu(unsigned int cpu)
 /*
  * Hotplug state machine related functions
  */
-static void undo_cpu_down(unsigned int cpu, struct cpuhp_cpu_state *st,
-                         struct cpuhp_step *steps)
+static void undo_cpu_down(unsigned int cpu, struct cpuhp_cpu_state *st)
 {
        for (st->state++; st->state < st->target; st->state++) {
-               struct cpuhp_step *step = steps + st->state;
+               struct cpuhp_step *step = cpuhp_get_step(st->state);
 
                if (!step->skip_onerr)
-                       cpuhp_invoke_callback(cpu, st->state, step->startup);
+                       cpuhp_invoke_callback(cpu, st->state, true);
        }
 }
 
 static int cpuhp_down_callbacks(unsigned int cpu, struct cpuhp_cpu_state *st,
-                               struct cpuhp_step *steps, enum cpuhp_state target)
+                               enum cpuhp_state target)
 {
        enum cpuhp_state prev_state = st->state;
        int ret = 0;
 
        for (; st->state > target; st->state--) {
-               struct cpuhp_step *step = steps + st->state;
-
-               ret = cpuhp_invoke_callback(cpu, st->state, step->teardown);
+               ret = cpuhp_invoke_callback(cpu, st->state, false);
                if (ret) {
                        st->target = prev_state;
-                       undo_cpu_down(cpu, st, steps);
+                       undo_cpu_down(cpu, st);
                        break;
                }
        }
        return ret;
 }
 
-static void undo_cpu_up(unsigned int cpu, struct cpuhp_cpu_state *st,
-                       struct cpuhp_step *steps)
+static void undo_cpu_up(unsigned int cpu, struct cpuhp_cpu_state *st)
 {
        for (st->state--; st->state > st->target; st->state--) {
-               struct cpuhp_step *step = steps + st->state;
+               struct cpuhp_step *step = cpuhp_get_step(st->state);
 
                if (!step->skip_onerr)
-                       cpuhp_invoke_callback(cpu, st->state, step->teardown);
+                       cpuhp_invoke_callback(cpu, st->state, false);
        }
 }
 
 static int cpuhp_up_callbacks(unsigned int cpu, struct cpuhp_cpu_state *st,
-                             struct cpuhp_step *steps, enum cpuhp_state target)
+                             enum cpuhp_state target)
 {
        enum cpuhp_state prev_state = st->state;
        int ret = 0;
 
        while (st->state < target) {
-               struct cpuhp_step *step;
-
                st->state++;
-               step = steps + st->state;
-               ret = cpuhp_invoke_callback(cpu, st->state, step->startup);
+               ret = cpuhp_invoke_callback(cpu, st->state, true);
                if (ret) {
                        st->target = prev_state;
-                       undo_cpu_up(cpu, st, steps);
+                       undo_cpu_up(cpu, st);
                        break;
                }
        }
@@ -455,13 +469,13 @@ static int cpuhp_ap_offline(unsigned int cpu, struct cpuhp_cpu_state *st)
 {
        enum cpuhp_state target = max((int)st->target, CPUHP_TEARDOWN_CPU);
 
-       return cpuhp_down_callbacks(cpu, st, cpuhp_ap_states, target);
+       return cpuhp_down_callbacks(cpu, st, target);
 }
 
 /* Execute the online startup callbacks. Used to be CPU_ONLINE */
 static int cpuhp_ap_online(unsigned int cpu, struct cpuhp_cpu_state *st)
 {
-       return cpuhp_up_callbacks(cpu, st, cpuhp_ap_states, st->target);
+       return cpuhp_up_callbacks(cpu, st, st->target);
 }
 
 /*
@@ -484,18 +498,20 @@ static void cpuhp_thread_fun(unsigned int cpu)
        st->should_run = false;
 
        /* Single callback invocation for [un]install ? */
-       if (st->cb) {
+       if (st->single) {
                if (st->cb_state < CPUHP_AP_ONLINE) {
                        local_irq_disable();
-                       ret = cpuhp_invoke_callback(cpu, st->cb_state, st->cb);
+                       ret = cpuhp_invoke_callback(cpu, st->cb_state,
+                                                   st->bringup);
                        local_irq_enable();
                } else {
-                       ret = cpuhp_invoke_callback(cpu, st->cb_state, st->cb);
+                       ret = cpuhp_invoke_callback(cpu, st->cb_state,
+                                                   st->bringup);
                }
        } else if (st->rollback) {
                BUG_ON(st->state < CPUHP_AP_ONLINE_IDLE);
 
-               undo_cpu_down(cpu, st, cpuhp_ap_states);
+               undo_cpu_down(cpu, st);
                /*
                 * This is a momentary workaround to keep the notifier users
                 * happy. Will go away once we got rid of the notifiers.
@@ -517,8 +533,8 @@ static void cpuhp_thread_fun(unsigned int cpu)
 }
 
 /* Invoke a single callback on a remote cpu */
-static int cpuhp_invoke_ap_callback(int cpu, enum cpuhp_state state,
-                                   int (*cb)(unsigned int))
+static int
+cpuhp_invoke_ap_callback(int cpu, enum cpuhp_state state, bool bringup)
 {
        struct cpuhp_cpu_state *st = per_cpu_ptr(&cpuhp_state, cpu);
 
@@ -530,10 +546,12 @@ static int cpuhp_invoke_ap_callback(int cpu, enum cpuhp_state state,
         * we invoke the thread function directly.
         */
        if (!st->thread)
-               return cpuhp_invoke_callback(cpu, state, cb);
+               return cpuhp_invoke_callback(cpu, state, bringup);
 
        st->cb_state = state;
-       st->cb = cb;
+       st->single = true;
+       st->bringup = bringup;
+
        /*
         * Make sure the above stores are visible before should_run becomes
         * true. Paired with the mb() above in cpuhp_thread_fun()
@@ -549,7 +567,7 @@ static int cpuhp_invoke_ap_callback(int cpu, enum cpuhp_state state,
 static void __cpuhp_kick_ap_work(struct cpuhp_cpu_state *st)
 {
        st->result = 0;
-       st->cb = NULL;
+       st->single = false;
        /*
         * Make sure the above stores are visible before should_run becomes
         * true. Paired with the mb() above in cpuhp_thread_fun()
@@ -700,12 +718,16 @@ static int take_cpu_down(void *_param)
        if (err < 0)
                return err;
 
+       /*
+        * We get here while we are in CPUHP_TEARDOWN_CPU state and we must not
+        * do this step again.
+        */
+       WARN_ON(st->state != CPUHP_TEARDOWN_CPU);
+       st->state--;
        /* Invoke the former CPU_DYING callbacks */
-       for (; st->state > target; st->state--) {
-               struct cpuhp_step *step = cpuhp_ap_states + st->state;
+       for (; st->state > target; st->state--)
+               cpuhp_invoke_callback(cpu, st->state, false);
 
-               cpuhp_invoke_callback(cpu, st->state, step->teardown);
-       }
        /* Give up timekeeping duties */
        tick_handover_do_timer();
        /* Park the stopper thread */
@@ -844,7 +866,7 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen,
         * The AP brought itself down to CPUHP_TEARDOWN_CPU. So we need
         * to do the further cleanups.
         */
-       ret = cpuhp_down_callbacks(cpu, st, cpuhp_bp_states, target);
+       ret = cpuhp_down_callbacks(cpu, st, target);
        if (ret && st->state > CPUHP_TEARDOWN_CPU && st->state < prev_state) {
                st->target = prev_state;
                st->rollback = true;
@@ -898,11 +920,8 @@ void notify_cpu_starting(unsigned int cpu)
        enum cpuhp_state target = min((int)st->target, CPUHP_AP_ONLINE);
 
        while (st->state < target) {
-               struct cpuhp_step *step;
-
                st->state++;
-               step = cpuhp_ap_states + st->state;
-               cpuhp_invoke_callback(cpu, st->state, step->startup);
+               cpuhp_invoke_callback(cpu, st->state, true);
        }
 }
 
@@ -987,7 +1006,7 @@ static int _cpu_up(unsigned int cpu, int tasks_frozen, enum cpuhp_state target)
         * responsible for bringing it up to the target state.
         */
        target = min((int)target, CPUHP_BRINGUP_CPU);
-       ret = cpuhp_up_callbacks(cpu, st, cpuhp_bp_states, target);
+       ret = cpuhp_up_callbacks(cpu, st, target);
 out:
        cpu_hotplug_done();
        return ret;
@@ -1364,23 +1383,6 @@ static int cpuhp_cb_check(enum cpuhp_state state)
        return 0;
 }
 
-static bool cpuhp_is_ap_state(enum cpuhp_state state)
-{
-       /*
-        * The extra check for CPUHP_TEARDOWN_CPU is only for documentation
-        * purposes as that state is handled explicitely in cpu_down.
-        */
-       return state > CPUHP_BRINGUP_CPU && state != CPUHP_TEARDOWN_CPU;
-}
-
-static struct cpuhp_step *cpuhp_get_step(enum cpuhp_state state)
-{
-       struct cpuhp_step *sp;
-
-       sp = cpuhp_is_ap_state(state) ? cpuhp_ap_states : cpuhp_bp_states;
-       return sp + state;
-}
-
 static void cpuhp_store_callbacks(enum cpuhp_state state,
                                  const char *name,
                                  int (*startup)(unsigned int cpu),
@@ -1406,12 +1408,12 @@ static void *cpuhp_get_teardown_cb(enum cpuhp_state state)
  * Call the startup/teardown function for a step either on the AP or
  * on the current CPU.
  */
-static int cpuhp_issue_call(int cpu, enum cpuhp_state state,
-                           int (*cb)(unsigned int), bool bringup)
+static int cpuhp_issue_call(int cpu, enum cpuhp_state state, bool bringup)
 {
+       struct cpuhp_step *sp = cpuhp_get_step(state);
        int ret;
 
-       if (!cb)
+       if ((bringup && !sp->startup) || (!bringup && !sp->teardown))
                return 0;
        /*
         * The non AP bound callbacks can fail on bringup. On teardown
@@ -1419,11 +1421,11 @@ static int cpuhp_issue_call(int cpu, enum cpuhp_state state,
         */
 #ifdef CONFIG_SMP
        if (cpuhp_is_ap_state(state))
-               ret = cpuhp_invoke_ap_callback(cpu, state, cb);
+               ret = cpuhp_invoke_ap_callback(cpu, state, bringup);
        else
-               ret = cpuhp_invoke_callback(cpu, state, cb);
+               ret = cpuhp_invoke_callback(cpu, state, bringup);
 #else
-       ret = cpuhp_invoke_callback(cpu, state, cb);
+       ret = cpuhp_invoke_callback(cpu, state, bringup);
 #endif
        BUG_ON(ret && !bringup);
        return ret;
@@ -1434,14 +1436,10 @@ static int cpuhp_issue_call(int cpu, enum cpuhp_state state,
  *
  * Note: The teardown callbacks for rollback are not allowed to fail!
  */
-static void cpuhp_rollback_install(int failedcpu, enum cpuhp_state state,
-                                  int (*teardown)(unsigned int cpu))
+static void cpuhp_rollback_install(int failedcpu, enum cpuhp_state state)
 {
        int cpu;
 
-       if (!teardown)
-               return;
-
        /* Roll back the already executed steps on the other cpus */
        for_each_present_cpu(cpu) {
                struct cpuhp_cpu_state *st = per_cpu_ptr(&cpuhp_state, cpu);
@@ -1452,7 +1450,7 @@ static void cpuhp_rollback_install(int failedcpu, enum cpuhp_state state,
 
                /* Did we invoke the startup call on that cpu ? */
                if (cpustate >= state)
-                       cpuhp_issue_call(cpu, state, teardown, false);
+                       cpuhp_issue_call(cpu, state, false);
        }
 }
 
@@ -1527,9 +1525,10 @@ int __cpuhp_setup_state(enum cpuhp_state state,
                if (cpustate < state)
                        continue;
 
-               ret = cpuhp_issue_call(cpu, state, startup, true);
+               ret = cpuhp_issue_call(cpu, state, true);
                if (ret) {
-                       cpuhp_rollback_install(cpu, state, teardown);
+                       if (teardown)
+                               cpuhp_rollback_install(cpu, state);
                        cpuhp_store_callbacks(state, NULL, NULL, NULL);
                        goto out;
                }
@@ -1553,14 +1552,13 @@ EXPORT_SYMBOL(__cpuhp_setup_state);
  */
 void __cpuhp_remove_state(enum cpuhp_state state, bool invoke)
 {
-       int (*teardown)(unsigned int cpu) = cpuhp_get_teardown_cb(state);
        int cpu;
 
        BUG_ON(cpuhp_cb_check(state));
 
        get_online_cpus();
 
-       if (!invoke || !teardown)
+       if (!invoke || !cpuhp_get_teardown_cb(state))
                goto remove;
 
        /*
@@ -1573,7 +1571,7 @@ void __cpuhp_remove_state(enum cpuhp_state state, bool invoke)
                int cpustate = st->state;
 
                if (cpustate >= state)
-                       cpuhp_issue_call(cpu, state, teardown, false);
+                       cpuhp_issue_call(cpu, state, false);
        }
 remove:
        cpuhp_store_callbacks(state, NULL, NULL, NULL);