Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1218980 > unrolled thread

[RFC v0 0/9] Remove CPU_*_FROZEN

Back to article view | Back to linux.kernel

Contents

  [RFC v0 0/9] Remove CPU_*_FROZEN Daniel Wagner <daniel.wagner@bmw-carit.de> - 2015-09-04 15:40 +0200
    [RFC v0 8/9] cpu: Do not set CPU_TASKS_FROZEN anymore Daniel Wagner <daniel.wagner@bmw-carit.de> - 2015-09-04 15:40 +0200
    [RFC v0 1/9] smpboot: Add a separate CPU state when a surviving CPU times out Daniel Wagner <daniel.wagner@bmw-carit.de> - 2015-09-04 15:40 +0200
    [RFC v0 2/9] suspend: Add getter function to report if freezing is active Daniel Wagner <daniel.wagner@bmw-carit.de> - 2015-09-04 15:40 +0200
      Re: [RFC v0 2/9] suspend: Add getter function to report if freezing is active "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-09-05 03:50 +0200
        Re: [RFC v0 2/9] suspend: Add getter function to report if freezing  is active Daniel Wagner <daniel.wagner@bmw-carit.de> - 2015-09-07 11:00 +0200
          Re: [RFC v0 2/9] suspend: Add getter function to report if freezing is active "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-09-07 15:20 +0200
          Re: [RFC v0 2/9] suspend: Add getter function to report if freezing is active "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-09-07 23:20 +0200
            Re: [RFC v0 2/9] suspend: Add getter function to report if freezing  is active Daniel Wagner <daniel.wagner@bmw-carit.de> - 2015-09-08 10:20 +0200
    [RFC v0 5/9] sched: Use freeze_active() instead CPU_*_FROZEN state information Daniel Wagner <daniel.wagner@bmw-carit.de> - 2015-09-04 15:40 +0200
    [RFC v0 4/9] smpboot: Use freeze_active() instead CPU_DEAD_FROZEN state information Daniel Wagner <daniel.wagner@bmw-carit.de> - 2015-09-04 15:40 +0200
      Re: [RFC v0 4/9] smpboot: Use freeze_active() instead CPU_DEAD_FROZEN  state information Daniel Wagner <wagi@monom.org> - 2015-09-08 10:50 +0200
    [RFC v0 6/9] cpu: Restructure FROZEN state handling Daniel Wagner <daniel.wagner@bmw-carit.de> - 2015-09-04 15:40 +0200
    [RFC v0 3/9] x86: Use freeze_active() instead of CPU_*_FROZEN Daniel Wagner <daniel.wagner@bmw-carit.de> - 2015-09-04 15:40 +0200
    [RFC v0 9/9] doc: Update cpu-hotplug documents on removal of CPU_TASKS_FROZEN Daniel Wagner <daniel.wagner@bmw-carit.de> - 2015-09-04 15:40 +0200

#1218980 — [RFC v0 0/9] Remove CPU_*_FROZEN

Hi

I was looking at Thomas' "CPU hotplug rework - episode I" series [1]
and noted the CPU_*_FROZEN bits in there.

In 2007 CPU_TASK_FROZEN was introduced to allow subsystem to
distinguish between normal CPU hotplug events and CPU hotplug events
under system-wide suspend or resume operations [2]. As it turns out
almost no subsystem is interested in this information. 

So this begs the question why having the additional complexity in the
CPU state handling instead of having an explicit function for
retrieving this information. Here and attempt to rip out
CPU_TASKS_FROZEN bits.

Overall I think it is worth doing so but you might see it differently.
FWIW, the image size is slightly smaller too in my sample
configuration.

   text    data     bss     dec     hex filename
16794542        4462208 14954496        36211246        2288a2e vmlinux
16794267        4462208 14954496        36210971        228891b vmlinux-wo-frozen

Patch 1: I think this patch fixes a real bug. Even Paul agreed during
a chat at LinuxCon. He needed an addition state to and grabbed just
one of the FROZEN ones.

Patch 2: Adds a new freeze_active() call which tells if PM is active
or not.

Patch 3, 4 and 5: Update the only users of FROZEN.

Patch 6: Is the refactoring patch from Thomas hotplug rework [1].

Patch 7: Remove all FROZEN references. It should contain only simple
changes. I did that manually. Probably some scripting could be done to
ensure the changes are more correct. This patch could be spitted and
the pieces could be applied one after the other.

Patch 8: Get rid of the definitions of FROZEN.

Patch 9: And finally update the documentation.

I starred at this code for while and compiled it for different
architectures (x86, ARM, S390, powerpc). I also tested by executing
Steven's stress-cpu-hotplug script and then do suspend-resume cycles.
Nothing exploded but that is not a real proof all is okay. So please
have a close look at the changes on the FROZEN users.

Thanks,
Daniel

[1] https://lwn.net/Articles/535764/
[2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8bb7844286fb8c9fce6f65d8288aeb09d03a5e0d


"H. Peter Anvin" <hpa@zytor.com>
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
"Rafael J. Wysocki" <rjw@rjwysocki.net>
Akinobu Mita <akinobu.mita@gmail.com>
Andrew Morton <akpm@linux-foundation.org>
Boris Ostrovsky <boris.ostrovsky@oracle.com>
Borislav Petkov <bp@alien8.de>
Chris Metcalf <cmetcalf@ezchip.com>
Daniel Wagner <daniel.wagner@bmw-carit.de>
David Hildenbrand <dahi@linux.vnet.ibm.com>
David Vrabel <david.vrabel@citrix.com>
Don Zickus <dzickus@redhat.com>
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Ingo Molnar <mingo@redhat.com>
John Hubbard <jhubbard@nvidia.com>
Jonathan Corbet <corbet@lwn.net>
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Lai Jiangshan <laijs@cn.fujitsu.com>
Len Brown <len.brown@intel.com>
Luis R. Rodriguez <mcgrof@do-not-panic.com>
Mathias Krause <minipli@googlemail.com>
Nicolas Iooss <nicolas.iooss_linux@m4x.org>
Paul Gortmaker <paul.gortmaker@windriver.com>
Pavel Machek <pavel@ucw.cz>
Peter Zijlstra <peterz@infradead.org>
Sudeep Holla <sudeep.holla@arm.com>
Thomas Gleixner <tglx@linutronix.de>
Tony Luck <tony.luck@intel.com>
Vitaly Kuznetsov <vkuznets@redhat.com>

Daniel Wagner (8):
  smpboot: Add a separate CPU state when a surviving CPU times out
  suspend: Add getter function to report if freezing is active
  x86: Use freeze_active() instead of CPU_*_FROZEN
  smpboot: Use freeze_active() instead CPU_DEAD_FROZEN state information
  sched: Use freeze_active() instead CPU_*_FROZEN state information
  cpu: Remove unused CPU_*_FROZEN states
  cpu: Do not set CPU_TASKS_FROZEN anymore
  doc: Update cpu-hotplug documents on removal of CPU_TASKS_FROZEN

Thomas Gleixner (1):
  cpu: Restructure FROZEN state handling

 Documentation/cpu-hotplug.txt                      | 12 ++--
 .../fault-injection/notifier-error-inject.txt      |  2 -
 Documentation/power/suspend-and-cpuhotplug.txt     | 13 +---
 arch/arm/kernel/hw_breakpoint.c                    |  2 +-
 arch/arm/kernel/perf_event.c                       |  2 +-
 arch/arm/kernel/smp_twd.c                          |  2 +-
 arch/arm/kvm/arm.c                                 |  1 -
 arch/arm/mm/cache-l2x0.c                           |  2 +-
 arch/arm/vfp/vfpmodule.c                           |  4 +-
 arch/arm64/kernel/armv8_deprecated.c               |  2 +-
 arch/arm64/kernel/fpsimd.c                         |  1 -
 arch/blackfin/kernel/perf_event.c                  |  2 +-
 arch/ia64/kernel/err_inject.c                      |  2 -
 arch/ia64/kernel/mca.c                             |  1 -
 arch/ia64/kernel/palinfo.c                         |  2 -
 arch/ia64/kernel/salinfo.c                         |  2 -
 arch/ia64/kernel/topology.c                        |  2 -
 arch/metag/kernel/perf/perf_event.c                |  2 +-
 arch/mips/loongson64/loongson-3/smp.c              |  3 -
 arch/mips/oprofile/op_model_loongson3.c            |  2 -
 arch/powerpc/kernel/sysfs.c                        |  2 -
 arch/powerpc/mm/mmu_context_nohash.c               |  3 -
 arch/powerpc/mm/numa.c                             |  3 -
 arch/powerpc/perf/core-book3s.c                    |  2 +-
 arch/powerpc/platforms/powermac/smp.c              |  2 -
 arch/s390/kernel/perf_cpum_cf.c                    |  2 +-
 arch/s390/kernel/perf_cpum_sf.c                    |  3 +-
 arch/s390/kernel/smp.c                             |  2 +-
 arch/s390/mm/fault.c                               |  2 +-
 arch/sh/kernel/perf_event.c                        |  2 +-
 arch/sparc/kernel/sysfs.c                          |  2 -
 arch/x86/entry/vdso/vma.c                          |  2 +-
 arch/x86/kernel/apic/x2apic_cluster.c              |  1 -
 arch/x86/kernel/cpu/mcheck/mce.c                   | 15 ++---
 arch/x86/kernel/cpu/mcheck/mce_amd.c               |  2 -
 arch/x86/kernel/cpu/mcheck/therm_throt.c           |  3 -
 arch/x86/kernel/cpu/microcode/core.c               | 12 ++--
 arch/x86/kernel/cpu/perf_event.c                   |  2 +-
 arch/x86/kernel/cpu/perf_event_amd_ibs.c           |  2 +-
 arch/x86/kernel/cpu/perf_event_amd_uncore.c        |  2 +-
 arch/x86/kernel/cpu/perf_event_intel_cqm.c         |  2 +-
 arch/x86/kernel/cpu/perf_event_intel_rapl.c        |  2 +-
 arch/x86/kernel/cpu/perf_event_intel_uncore.c      |  4 +-
 arch/x86/kernel/cpuid.c                            |  1 -
 arch/x86/kernel/kvm.c                              |  2 -
 arch/x86/kernel/msr.c                              |  1 -
 arch/x86/pci/amd_bus.c                             |  1 -
 arch/x86/xen/smp.c                                 |  2 +-
 arch/xtensa/kernel/perf_event.c                    |  2 +-
 block/blk-iopoll.c                                 |  2 +-
 block/blk-mq.c                                     |  5 +-
 block/blk-softirq.c                                |  2 +-
 drivers/acpi/processor_driver.c                    |  1 -
 drivers/base/cacheinfo.c                           |  2 +-
 drivers/base/topology.c                            |  3 -
 drivers/bus/arm-cci.c                              |  2 +-
 drivers/bus/arm-ccn.c                              |  2 +-
 drivers/bus/mips_cdmm.c                            |  2 +-
 drivers/clocksource/arm_arch_timer.c               |  2 +-
 drivers/clocksource/arm_global_timer.c             |  2 +-
 drivers/clocksource/dummy_timer.c                  |  2 +-
 drivers/clocksource/exynos_mct.c                   |  2 +-
 drivers/clocksource/metag_generic.c                |  1 -
 drivers/clocksource/mips-gic-timer.c               |  2 +-
 drivers/clocksource/qcom-timer.c                   |  2 +-
 drivers/clocksource/time-armada-370-xp.c           |  2 +-
 drivers/clocksource/timer-atlas7.c                 |  2 +-
 drivers/cpufreq/acpi-cpufreq.c                     |  2 -
 drivers/cpufreq/cpufreq.c                          |  2 +-
 drivers/cpuidle/coupled.c                          |  4 +-
 drivers/cpuidle/cpuidle-powernv.c                  |  2 -
 drivers/cpuidle/cpuidle-pseries.c                  |  2 -
 drivers/hwtracing/coresight/coresight-etm3x.c      |  2 +-
 drivers/hwtracing/coresight/coresight-etm4x.c      |  2 +-
 drivers/idle/intel_idle.c                          |  2 +-
 drivers/irqchip/irq-armada-370-xp.c                |  4 +-
 drivers/irqchip/irq-gic-v3.c                       |  2 +-
 drivers/irqchip/irq-gic.c                          |  2 +-
 drivers/irqchip/irq-hip04.c                        |  2 +-
 drivers/leds/trigger/ledtrig-cpu.c                 |  2 +-
 drivers/md/raid5.c                                 |  2 -
 drivers/net/virtio_net.c                           |  2 +-
 drivers/oprofile/timer_int.c                       |  2 -
 drivers/pci/host/pci-xgene-msi.c                   |  2 -
 drivers/powercap/intel_rapl.c                      |  3 -
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c                  |  2 -
 drivers/scsi/bnx2i/bnx2i_init.c                    |  2 -
 drivers/scsi/fcoe/fcoe.c                           |  2 -
 drivers/scsi/virtio_scsi.c                         |  2 -
 .../staging/lustre/lustre/libcfs/linux/linux-cpu.c |  4 +-
 fs/buffer.c                                        |  2 +-
 include/linux/cpu.h                                | 17 +----
 include/linux/suspend.h                            |  6 ++
 kernel/cpu.c                                       | 74 ++++++++--------------
 kernel/events/core.c                               |  2 +-
 kernel/padata.c                                    |  4 --
 kernel/profile.c                                   |  4 --
 kernel/rcu/tree.c                                  |  5 --
 kernel/relay.c                                     |  2 -
 kernel/sched/core.c                                | 59 +++++++++--------
 kernel/sched/fair.c                                |  2 +-
 kernel/smp.c                                       |  6 +-
 kernel/smpboot.c                                   | 34 +++++-----
 kernel/softirq.c                                   |  1 -
 kernel/time/hrtimer.c                              |  2 -
 kernel/time/tick-sched.c                           |  2 +-
 kernel/time/timer.c                                |  1 -
 kernel/trace/ring_buffer.c                         |  2 -
 kernel/workqueue.c                                 |  4 +-
 lib/cpu-notifier-error-inject.c                    |  2 -
 lib/percpu_counter.c                               |  2 +-
 lib/radix-tree.c                                   |  2 +-
 mm/memcontrol.c                                    |  2 +-
 mm/page-writeback.c                                |  2 +-
 mm/page_alloc.c                                    |  2 +-
 mm/slab.c                                          |  6 --
 mm/slub.c                                          |  2 -
 mm/vmscan.c                                        |  2 +-
 mm/vmstat.c                                        |  4 --
 net/core/dev.c                                     |  2 +-
 net/core/flow.c                                    |  2 -
 net/iucv/iucv.c                                    |  6 --
 virt/kvm/arm/arch_timer.c                          |  2 -
 virt/kvm/arm/vgic.c                                |  2 -
 virt/kvm/kvm_main.c                                |  1 -
 125 files changed, 172 insertions(+), 333 deletions(-)

-- 
2.4.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/

[toc] | [next] | [standalone]

#1218982 — [RFC v0 8/9] cpu: Do not set CPU_TASKS_FROZEN anymore

There is no user left of CPU_TASKS_FROZEN, so we can stop propagating
this information.

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Mathias Krause <minipli@googlemail.com>
Cc: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: linux-kernel@vger.kernel.org
---
 include/linux/cpu.h | 15 ---------------
 kernel/cpu.c        | 22 ++++++++--------------
 2 files changed, 8 insertions(+), 29 deletions(-)

diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 381ea8a..ebd07e7 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -104,21 +104,6 @@ enum {
 #define CPU_DEAD_TIMEOUT	0x000D /* CPU (unsigned)v surviving CPU timed
 					  out */
 
-/* Used for CPU hotplug events occurring while tasks are frozen due to a suspend
- * operation in progress
- */
-#define CPU_TASKS_FROZEN	0x0010
-
-#define CPU_ONLINE_FROZEN	(CPU_ONLINE | CPU_TASKS_FROZEN)
-#define CPU_UP_PREPARE_FROZEN	(CPU_UP_PREPARE | CPU_TASKS_FROZEN)
-#define CPU_UP_CANCELED_FROZEN	(CPU_UP_CANCELED | CPU_TASKS_FROZEN)
-#define CPU_DOWN_PREPARE_FROZEN	(CPU_DOWN_PREPARE | CPU_TASKS_FROZEN)
-#define CPU_DOWN_FAILED_FROZEN	(CPU_DOWN_FAILED | CPU_TASKS_FROZEN)
-#define CPU_DEAD_FROZEN		(CPU_DEAD | CPU_TASKS_FROZEN)
-#define CPU_DYING_FROZEN	(CPU_DYING | CPU_TASKS_FROZEN)
-#define CPU_STARTING_FROZEN	(CPU_STARTING | CPU_TASKS_FROZEN)
-
-
 #ifdef CONFIG_SMP
 /* Need to know about CPUs going up/down? */
 #if defined(CONFIG_HOTPLUG_CPU) || !defined(MODULE)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index e37442d..1f0408c 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -29,7 +29,6 @@
 #ifdef CONFIG_SMP
 /* Serializes the updates to cpu_online_mask, cpu_present_mask */
 static DEFINE_MUTEX(cpu_add_remove_lock);
-static bool cpuhp_tasks_frozen;
 
 /*
  * The following two APIs (cpu_maps_update_begin/done) must be used when
@@ -224,12 +223,11 @@ int __register_cpu_notifier(struct notifier_block *nb)
 static int __cpu_notify(unsigned long val, unsigned int cpu, int nr_to_call,
 			int *nr_calls)
 {
-	unsigned long mod = cpuhp_tasks_frozen ? CPU_TASKS_FROZEN : 0;
 	void *hcpu = (void *)(long)cpu;
 
 	int ret;
 
-	ret = __raw_notifier_call_chain(&cpu_chain, val | mod, hcpu, nr_to_call,
+	ret = __raw_notifier_call_chain(&cpu_chain, val, hcpu, nr_to_call,
 					nr_calls);
 
 	return notifier_to_errno(ret);
@@ -347,7 +345,7 @@ static int take_cpu_down(void *_param)
 }
 
 /* Requires cpu_add_remove_lock to be held */
-static int _cpu_down(unsigned int cpu, int tasks_frozen)
+static int _cpu_down(unsigned int cpu)
 {
 	int err, nr_calls = 0;
 
@@ -359,8 +357,6 @@ static int _cpu_down(unsigned int cpu, int tasks_frozen)
 
 	cpu_hotplug_begin();
 
-	cpuhp_tasks_frozen = tasks_frozen;
-
 	err = __cpu_notify(CPU_DOWN_PREPARE, cpu, -1, &nr_calls);
 	if (err) {
 		nr_calls--;
@@ -448,7 +444,7 @@ int cpu_down(unsigned int cpu)
 		goto out;
 	}
 
-	err = _cpu_down(cpu, 0);
+	err = _cpu_down(cpu);
 
 out:
 	cpu_maps_update_done();
@@ -465,7 +461,7 @@ static int smpboot_thread_call(struct notifier_block *nfb,
 {
 	int cpu = (long)hcpu;
 
-	switch (action & ~CPU_TASKS_FROZEN) {
+	switch (action) {
 
 	case CPU_DOWN_FAILED:
 	case CPU_ONLINE:
@@ -490,7 +486,7 @@ void smpboot_thread_init(void)
 }
 
 /* Requires cpu_add_remove_lock to be held */
-static int _cpu_up(unsigned int cpu, int tasks_frozen)
+static int _cpu_up(unsigned int cpu)
 {
 	struct task_struct *idle;
 	int ret, nr_calls = 0;
@@ -512,8 +508,6 @@ static int _cpu_up(unsigned int cpu, int tasks_frozen)
 	if (ret)
 		goto out;
 
-	cpuhp_tasks_frozen = tasks_frozen;
-
 	ret = __cpu_notify(CPU_UP_PREPARE, cpu, -1, &nr_calls);
 	if (ret) {
 		nr_calls--;
@@ -565,7 +559,7 @@ int cpu_up(unsigned int cpu)
 		goto out;
 	}
 
-	err = _cpu_up(cpu, 0);
+	err = _cpu_up(cpu);
 
 out:
 	cpu_maps_update_done();
@@ -593,7 +587,7 @@ int disable_nonboot_cpus(void)
 		if (cpu == first_cpu)
 			continue;
 		trace_suspend_resume(TPS("CPU_OFF"), cpu, true);
-		error = _cpu_down(cpu, 1);
+		error = _cpu_down(cpu);
 		trace_suspend_resume(TPS("CPU_OFF"), cpu, false);
 		if (!error)
 			cpumask_set_cpu(cpu, frozen_cpus);
@@ -643,7 +637,7 @@ void enable_nonboot_cpus(void)
 
 	for_each_cpu(cpu, frozen_cpus) {
 		trace_suspend_resume(TPS("CPU_ON"), cpu, true);
-		error = _cpu_up(cpu, 1);
+		error = _cpu_up(cpu);
 		trace_suspend_resume(TPS("CPU_ON"), cpu, false);
 		if (!error) {
 			pr_info("CPU%d is up\n", cpu);
-- 
2.4.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/

[toc] | [prev] | [next] | [standalone]

#1218983 — [RFC v0 1/9] smpboot: Add a separate CPU state when a surviving CPU times out

The CPU_DEAD_FROZEN state is abused to report to cpu_wait_death() that
the operation timeout. It has nothing to do with the pm freezing
process. Introduce a new state to allow proper distinction between the
states and also prepares the code to get rid of all FROZEN states.

This was intruced in

8038dad7e888581266c76df15d70ca457a3c5910 smpboot: Add common code for notification from dying CPU
2a442c9c6453d3d043dfd89f2e03a1deff8a6f06 x86: Use common outgoing-CPU-notification code

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Chris Metcalf <cmetcalf@ezchip.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: x86@kernel.org
Cc: xen-devel@lists.xenproject.org
Cc: linux-kernel@vger.kernel.org
---
 arch/x86/xen/smp.c  | 2 +-
 include/linux/cpu.h | 2 ++
 kernel/smpboot.c    | 4 ++--
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 8648438..7a8bc03 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -740,7 +740,7 @@ static int xen_hvm_cpu_up(unsigned int cpu, struct task_struct *tidle)
 	 * This can happen if CPU was offlined earlier and
 	 * offlining timed out in common_cpu_die().
 	 */
-	if (cpu_report_state(cpu) == CPU_DEAD_FROZEN) {
+	if (cpu_report_state(cpu) == CPU_DEAD_TIMEOUT) {
 		xen_smp_intr_free(cpu);
 		xen_uninit_lock_cpu(cpu);
 	}
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 23c30bd..381ea8a 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -101,6 +101,8 @@ enum {
 					* idle loop. */
 #define CPU_BROKEN		0x000C /* CPU (unsigned)v did not die properly,
 					* perhaps due to preemption. */
+#define CPU_DEAD_TIMEOUT	0x000D /* CPU (unsigned)v surviving CPU timed
+					  out */
 
 /* Used for CPU hotplug events occurring while tasks are frozen due to a suspend
  * operation in progress
diff --git a/kernel/smpboot.c b/kernel/smpboot.c
index 7c434c3..e37efbf 100644
--- a/kernel/smpboot.c
+++ b/kernel/smpboot.c
@@ -505,7 +505,7 @@ update_state:
  * Called by the outgoing CPU to report its successful death.  Return
  * false if this report follows the surviving CPU's timing out.
  *
- * A separate "CPU_DEAD_FROZEN" is used when the surviving CPU
+ * A separate "CPU_DEAD_TIMEOUT" is used when the surviving CPU
  * timed out.  This approach allows architectures to omit calls to
  * cpu_check_up_prepare() and cpu_set_state_online() without defeating
  * the next cpu_wait_death()'s polling loop.
@@ -521,7 +521,7 @@ bool cpu_report_death(void)
 		if (oldstate != CPU_BROKEN)
 			newstate = CPU_DEAD;
 		else
-			newstate = CPU_DEAD_FROZEN;
+			newstate = CPU_DEAD_TIMEOUT;
 	} while (atomic_cmpxchg(&per_cpu(cpu_hotplug_state, cpu),
 				oldstate, newstate) != oldstate);
 	return newstate == CPU_DEAD;
-- 
2.4.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/

[toc] | [prev] | [next] | [standalone]

#1218985 — [RFC v0 2/9] suspend: Add getter function to report if freezing is active

Instead encode the FREEZE state via the CPU state we allow the
interesting subsystems (MCE, microcode) to query the power
subsystem directly. Most notifiers are not interested at all
in this information so rather have explicit calls to freeze_active()
instead adding complexity to the rest of the users of the CPU
notifiers.

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <len.brown@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 include/linux/suspend.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 5efe743..5e15ade 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -216,6 +216,11 @@ static inline bool idle_should_freeze(void)
 	return unlikely(suspend_freeze_state == FREEZE_STATE_ENTER);
 }
 
+static inline bool freeze_active(void)
+{
+	return unlikely(suspend_freeze_state != FREEZE_STATE_NONE);
+}
+
 extern void freeze_set_ops(const struct platform_freeze_ops *ops);
 extern void freeze_wake(void);
 
@@ -244,6 +249,7 @@ extern int pm_suspend(suspend_state_t state);
 static inline void suspend_set_ops(const struct platform_suspend_ops *ops) {}
 static inline int pm_suspend(suspend_state_t state) { return -ENOSYS; }
 static inline bool idle_should_freeze(void) { return false; }
+static inline bool freeze_active(void) { return false; }
 static inline void freeze_set_ops(const struct platform_freeze_ops *ops) {}
 static inline void freeze_wake(void) {}
 #endif /* !CONFIG_SUSPEND */
-- 
2.4.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/

[toc] | [prev] | [next] | [standalone]

#1219413 — Re: [RFC v0 2/9] suspend: Add getter function to report if freezing is active

On Friday, September 04, 2015 03:34:55 PM Daniel Wagner wrote:
> Instead encode the FREEZE state via the CPU state we allow the
> interesting subsystems (MCE, microcode) to query the power
> subsystem directly.

A use case, please.

> Most notifiers are not interested at all
> in this information so rather have explicit calls to freeze_active()
> instead adding complexity to the rest of the users of the CPU
> notifiers.

Why does it has anything to do with CPU notifiers?  We don't offline
CPUs for suspend-to-idle.


> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  include/linux/suspend.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index 5efe743..5e15ade 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -216,6 +216,11 @@ static inline bool idle_should_freeze(void)
>  	return unlikely(suspend_freeze_state == FREEZE_STATE_ENTER);
>  }
>  
> +static inline bool freeze_active(void)
> +{
> +	return unlikely(suspend_freeze_state != FREEZE_STATE_NONE);
> +}
> +
>  extern void freeze_set_ops(const struct platform_freeze_ops *ops);
>  extern void freeze_wake(void);
>  
> @@ -244,6 +249,7 @@ extern int pm_suspend(suspend_state_t state);
>  static inline void suspend_set_ops(const struct platform_suspend_ops *ops) {}
>  static inline int pm_suspend(suspend_state_t state) { return -ENOSYS; }
>  static inline bool idle_should_freeze(void) { return false; }
> +static inline bool freeze_active(void) { return false; }
>  static inline void freeze_set_ops(const struct platform_freeze_ops *ops) {}
>  static inline void freeze_wake(void) {}
>  #endif /* !CONFIG_SUSPEND */
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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/

[toc] | [prev] | [next] | [standalone]

#1220003 — Re: [RFC v0 2/9] suspend: Add getter function to report if freezing is active

On 09/05/2015 04:11 AM, Rafael J. Wysocki wrote:
> On Friday, September 04, 2015 03:34:55 PM Daniel Wagner wrote:
>> Instead encode the FREEZE state via the CPU state we allow the
>> interesting subsystems (MCE, microcode) to query the power
>> subsystem directly.
> 
> A use case, please.

The motivation for this change is to reduce the complexity in the
hotplug code. As tried to point out in the cover letter, the FROZEN
bits have only a bunch of users after all those years (2007). So it is
worth to have all the notifier users to handle the FROZEN state?

Don't know if that counts as use case.

>> Most notifiers are not interested at all
>> in this information so rather have explicit calls to freeze_active()
>> instead adding complexity to the rest of the users of the CPU
>> notifiers.
> 
> Why does it has anything to do with CPU notifiers?

cpu_{down|up} will call the notifiers with the CPU_TASK_FROZEN bit set
and so most notifiers are doing

	switch (actcion ~CPU_TASK_FROZEN)

to filter it out because they don't need to handle the system wide
ongoing freeze operations.

> We don't offline CPUs for suspend-to-idle.

Sure. As I said the motivation is to reduce the complexity in the
hotplug code.

Thanks,
Daniel
--
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/

[toc] | [prev] | [next] | [standalone]

#1220164 — Re: [RFC v0 2/9] suspend: Add getter function to report if freezing is active

On Monday, September 07, 2015 10:55:43 AM Daniel Wagner wrote:
> On 09/05/2015 04:11 AM, Rafael J. Wysocki wrote:
> > On Friday, September 04, 2015 03:34:55 PM Daniel Wagner wrote:
> >> Instead encode the FREEZE state via the CPU state we allow the
> >> interesting subsystems (MCE, microcode) to query the power
> >> subsystem directly.
> > 
> > A use case, please.
> 
> The motivation for this change is to reduce the complexity in the
> hotplug code. As tried to point out in the cover letter, the FROZEN
> bits have only a bunch of users after all those years (2007). So it is
> worth to have all the notifier users to handle the FROZEN state?
> 
> Don't know if that counts as use case.
> 
> >> Most notifiers are not interested at all
> >> in this information so rather have explicit calls to freeze_active()
> >> instead adding complexity to the rest of the users of the CPU
> >> notifiers.
> > 
> > Why does it has anything to do with CPU notifiers?
> 
> cpu_{down|up} will call the notifiers with the CPU_TASK_FROZEN bit set
> and so most notifiers are doing
> 
> 	switch (actcion ~CPU_TASK_FROZEN)
> 
> to filter it out because they don't need to handle the system wide
> ongoing freeze operations.
> 
> > We don't offline CPUs for suspend-to-idle.
> 
> Sure. As I said the motivation is to reduce the complexity in the
> hotplug code.

Well, it looks like I confused two things.

Let me look at this again.

Thanks,
Rafael

--
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/

[toc] | [prev] | [next] | [standalone]

#1220406 — Re: [RFC v0 2/9] suspend: Add getter function to report if freezing is active

On Monday, September 07, 2015 10:55:43 AM Daniel Wagner wrote:
> On 09/05/2015 04:11 AM, Rafael J. Wysocki wrote:
> > On Friday, September 04, 2015 03:34:55 PM Daniel Wagner wrote:
> >> Instead encode the FREEZE state via the CPU state we allow the
> >> interesting subsystems (MCE, microcode) to query the power
> >> subsystem directly.
> > 
> > A use case, please.
> 
> The motivation for this change is to reduce the complexity in the
> hotplug code. As tried to point out in the cover letter, the FROZEN
> bits have only a bunch of users after all those years (2007). So it is
> worth to have all the notifier users to handle the FROZEN state?
> 
> Don't know if that counts as use case.

Well, the code you're changing has nothing to do with CPU hotplug and
CPU_TASKS_FROZEN.  It is about suspend-to-idle.

Please grep for suspend_freeze_state and see what it is used for.

There is some confusion in the naming, but that is about the freezing of
the whole system, while CPU_TASKS_FROZEN is about the freezing of user space.

> >> Most notifiers are not interested at all
> >> in this information so rather have explicit calls to freeze_active()
> >> instead adding complexity to the rest of the users of the CPU
> >> notifiers.
> > 
> > Why does it has anything to do with CPU notifiers?
> 
> cpu_{down|up} will call the notifiers with the CPU_TASK_FROZEN bit set
> and so most notifiers are doing
> 
> 	switch (actcion ~CPU_TASK_FROZEN)
> 
> to filter it out because they don't need to handle the system wide
> ongoing freeze operations.
> 
> > We don't offline CPUs for suspend-to-idle.
> 
> Sure. As I said the motivation is to reduce the complexity in the
> hotplug code.

You need to fine a different way to do that.

Thanks,
Rafael

--
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/

[toc] | [prev] | [next] | [standalone]

#1220592 — Re: [RFC v0 2/9] suspend: Add getter function to report if freezing is active

On 09/07/2015 11:44 PM, Rafael J. Wysocki wrote:
> On Monday, September 07, 2015 10:55:43 AM Daniel Wagner wrote:
>> On 09/05/2015 04:11 AM, Rafael J. Wysocki wrote:
>>> On Friday, September 04, 2015 03:34:55 PM Daniel Wagner wrote:
>>>> Instead encode the FREEZE state via the CPU state we allow the
>>>> interesting subsystems (MCE, microcode) to query the power
>>>> subsystem directly.
>>>
>>> A use case, please.
>>
>> The motivation for this change is to reduce the complexity in the
>> hotplug code. As tried to point out in the cover letter, the FROZEN
>> bits have only a bunch of users after all those years (2007). So it is
>> worth to have all the notifier users to handle the FROZEN state?
>>
>> Don't know if that counts as use case.
> 
> Well, the code you're changing has nothing to do with CPU hotplug and
> CPU_TASKS_FROZEN.  It is about suspend-to-idle.
> 
> Please grep for suspend_freeze_state and see what it is used for.
> 
> There is some confusion in the naming, but that is about the freezing of
> the whole system, while CPU_TASKS_FROZEN is about the freezing of user space.

You are right. I got confused by all those frozen/freezing naming scheme.

>>>> Most notifiers are not interested at all
>>>> in this information so rather have explicit calls to freeze_active()
>>>> instead adding complexity to the rest of the users of the CPU
>>>> notifiers.
>>>
>>> Why does it has anything to do with CPU notifiers?
>>
>> cpu_{down|up} will call the notifiers with the CPU_TASK_FROZEN bit set
>> and so most notifiers are doing
>>
>> 	switch (actcion ~CPU_TASK_FROZEN)
>>
>> to filter it out because they don't need to handle the system wide
>> ongoing freeze operations.
>>
>>> We don't offline CPUs for suspend-to-idle.
>>
>> Sure. As I said the motivation is to reduce the complexity in the
>> hotplug code.
> 
> You need to fine a different way to do that.

I'll try something else.

Thanks for taking the time explaining!

cheers,
Daniel
--
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/

[toc] | [prev] | [next] | [standalone]

#1218988 — [RFC v0 5/9] sched: Use freeze_active() instead CPU_*_FROZEN state information

In order to get rid of all CPU_*_FROZEN states we need to convert all
users first.

cpuset_cpu_active() tracks via num_cpus_frozen if the current CPU is
the last one. So there is no need to track the exact CPU is in
a CPU_*_FROZEN state. Instead we can probe freeze_active() to tell
if supsens or resume is ongoing.

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org
---
 kernel/sched/core.c | 48 +++++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 5de2c9e..36b00eb 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -74,6 +74,7 @@
 #include <linux/binfmts.h>
 #include <linux/context_tracking.h>
 #include <linux/compiler.h>
+#include <linux/suspend.h>
 
 #include <asm/switch_to.h>
 #include <asm/tlb.h>
@@ -7128,28 +7129,28 @@ static int cpuset_cpu_active(struct notifier_block *nfb, unsigned long action,
 			     void *hcpu)
 {
 	switch (action) {
-	case CPU_ONLINE_FROZEN:
-	case CPU_DOWN_FAILED_FROZEN:
-
-		/*
-		 * num_cpus_frozen tracks how many CPUs are involved in suspend
-		 * resume sequence. As long as this is not the last online
-		 * operation in the resume sequence, just build a single sched
-		 * domain, ignoring cpusets.
-		 */
-		num_cpus_frozen--;
-		if (likely(num_cpus_frozen)) {
-			partition_sched_domains(1, NULL, NULL);
-			break;
+	case CPU_ONLINE:
+	case CPU_DOWN_FAILED:
+		if (freeze_active()) {
+			/*
+			 * num_cpus_frozen tracks how many CPUs are
+			 * involved in suspend resume sequence. As
+			 * long as this is not the last online
+			 * operation in the resume sequence, just
+			 * build a single sched domain, ignoring
+			 * cpusets.
+			 */
+			num_cpus_frozen--;
+			if (likely(num_cpus_frozen)) {
+				partition_sched_domains(1, NULL, NULL);
+				break;
+			}
 		}
-
 		/*
-		 * This is the last CPU online operation. So fall through and
-		 * restore the original sched domains by considering the
-		 * cpuset configurations.
+		 * This is the last CPU online operation. Restore the
+		 * original sched domains by considering the cpuset
+		 * configurations.
 		 */
-
-	case CPU_ONLINE:
 		cpuset_update_active_cpus(true);
 		break;
 	default:
@@ -7169,6 +7170,11 @@ static int cpuset_cpu_inactive(struct notifier_block *nfb, unsigned long action,
 
 	switch (action) {
 	case CPU_DOWN_PREPARE:
+		if (freeze_active()) {
+			num_cpus_frozen++;
+			partition_sched_domains(1, NULL, NULL);
+			break;
+		}
 		rcu_read_lock_sched();
 		dl_b = dl_bw_of(cpu);
 
@@ -7183,10 +7189,6 @@ static int cpuset_cpu_inactive(struct notifier_block *nfb, unsigned long action,
 			return notifier_from_errno(-EBUSY);
 		cpuset_update_active_cpus(false);
 		break;
-	case CPU_DOWN_PREPARE_FROZEN:
-		num_cpus_frozen++;
-		partition_sched_domains(1, NULL, NULL);
-		break;
 	default:
 		return NOTIFY_DONE;
 	}
-- 
2.4.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/

[toc] | [prev] | [next] | [standalone]

#1218990 — [RFC v0 4/9] smpboot: Use freeze_active() instead CPU_DEAD_FROZEN state information

In order to get rid of all CPU_*_FROZEN states we need to convert all
users first.

cpu_check_up_prepare() wants to report different errors depending on
an ongoing suspend or not. freeze_active() reports back if that is the
case so we don't have to rely on the CPU_DEAD_FROZEN anymore.

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Chris Metcalf <cmetcalf@ezchip.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: linux-kernel@vger.kernel.org
---
 kernel/smpboot.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/kernel/smpboot.c b/kernel/smpboot.c
index e37efbf..49ce4e9 100644
--- a/kernel/smpboot.c
+++ b/kernel/smpboot.c
@@ -13,6 +13,7 @@
 #include <linux/percpu.h>
 #include <linux/kthread.h>
 #include <linux/smpboot.h>
+#include <linux/suspend.h>
 
 #include "smpboot.h"
 
@@ -407,26 +408,25 @@ int cpu_check_up_prepare(int cpu)
 	switch (atomic_read(&per_cpu(cpu_hotplug_state, cpu))) {
 
 	case CPU_POST_DEAD:
+		if (freeze_active()) {
+			/*
+			 * Timeout during CPU death, so let caller know.
+			 * The outgoing CPU completed its processing, but after
+			 * cpu_wait_death() timed out and reported the error. The
+			 * caller is free to proceed, in which case the state
+			 * will be reset properly by cpu_set_state_online().
+			 * Proceeding despite this -EBUSY return makes sense
+			 * for systems where the outgoing CPUs take themselves
+			 * offline, with no post-death manipulation required from
+			 * a surviving CPU.
+			 */
+			return -EBUSY;
+		}
 
 		/* The CPU died properly, so just start it up again. */
 		atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_UP_PREPARE);
 		return 0;
 
-	case CPU_DEAD_FROZEN:
-
-		/*
-		 * Timeout during CPU death, so let caller know.
-		 * The outgoing CPU completed its processing, but after
-		 * cpu_wait_death() timed out and reported the error. The
-		 * caller is free to proceed, in which case the state
-		 * will be reset properly by cpu_set_state_online().
-		 * Proceeding despite this -EBUSY return makes sense
-		 * for systems where the outgoing CPUs take themselves
-		 * offline, with no post-death manipulation required from
-		 * a surviving CPU.
-		 */
-		return -EBUSY;
-
 	case CPU_BROKEN:
 
 		/*
-- 
2.4.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/

[toc] | [prev] | [next] | [standalone]

#1220607 — Re: [RFC v0 4/9] smpboot: Use freeze_active() instead CPU_DEAD_FROZEN state information

On 09/04/2015 03:34 PM, Daniel Wagner wrote:
> In order to get rid of all CPU_*_FROZEN states we need to convert all
> users first.
> 
> cpu_check_up_prepare() wants to report different errors depending on
> an ongoing suspend or not. freeze_active() reports back if that is the
> case so we don't have to rely on the CPU_DEAD_FROZEN anymore.

Just realized, this is patch doesn't make sense. The FROZEN bits in the
current code have nothing to do with CPU_TASK_FROZEN. Instead it should
use CPU_DEAD_TIMEOUT as explained in patch 01.
--
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/

[toc] | [prev] | [next] | [standalone]

#1218993 — [RFC v0 6/9] cpu: Restructure FROZEN state handling

From: Thomas Gleixner <tglx@linutronix.de>

There are only a few callbacks which really care about FROZEN
vs. !FROZEN. No need to have extra states for this.

Publish the frozen state in an extra variable which is updated under
the hotplug lock and let the users interested deal with it w/o
imposing that extra state checks on everyone.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Mathias Krause <minipli@googlemail.com>
Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: linux-kernel@vger.kernel.org
---
 kernel/cpu.c | 66 +++++++++++++++++++++++++-----------------------------------
 1 file changed, 27 insertions(+), 39 deletions(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 82cf9df..e37442d 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -29,6 +29,7 @@
 #ifdef CONFIG_SMP
 /* Serializes the updates to cpu_online_mask, cpu_present_mask */
 static DEFINE_MUTEX(cpu_add_remove_lock);
+static bool cpuhp_tasks_frozen;
 
 /*
  * The following two APIs (cpu_maps_update_begin/done) must be used when
@@ -220,27 +221,30 @@ int __register_cpu_notifier(struct notifier_block *nb)
 	return raw_notifier_chain_register(&cpu_chain, nb);
 }
 
-static int __cpu_notify(unsigned long val, void *v, int nr_to_call,
+static int __cpu_notify(unsigned long val, unsigned int cpu, int nr_to_call,
 			int *nr_calls)
 {
+	unsigned long mod = cpuhp_tasks_frozen ? CPU_TASKS_FROZEN : 0;
+	void *hcpu = (void *)(long)cpu;
+
 	int ret;
 
-	ret = __raw_notifier_call_chain(&cpu_chain, val, v, nr_to_call,
+	ret = __raw_notifier_call_chain(&cpu_chain, val | mod, hcpu, nr_to_call,
 					nr_calls);
 
 	return notifier_to_errno(ret);
 }
 
-static int cpu_notify(unsigned long val, void *v)
+static int cpu_notify(unsigned long val, unsigned int cpu)
 {
-	return __cpu_notify(val, v, -1, NULL);
+	return __cpu_notify(val, cpu, -1, NULL);
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
 
-static void cpu_notify_nofail(unsigned long val, void *v)
+static void cpu_notify_nofail(unsigned long val, unsigned int cpu)
 {
-	BUG_ON(cpu_notify(val, v));
+	BUG_ON(cpu_notify(val, cpu));
 }
 EXPORT_SYMBOL(register_cpu_notifier);
 EXPORT_SYMBOL(__register_cpu_notifier);
@@ -324,23 +328,17 @@ static inline void check_for_tasks(int dead_cpu)
 	read_unlock_irq(&tasklist_lock);
 }
 
-struct take_cpu_down_param {
-	unsigned long mod;
-	void *hcpu;
-};
-
 /* Take this CPU down. */
 static int take_cpu_down(void *_param)
 {
-	struct take_cpu_down_param *param = _param;
-	int err;
+	int err, cpu = smp_processor_id();
 
 	/* Ensure this CPU doesn't handle any more interrupts. */
 	err = __cpu_disable();
 	if (err < 0)
 		return err;
 
-	cpu_notify(CPU_DYING | param->mod, param->hcpu);
+	cpu_notify(CPU_DYING, cpu);
 	/* Give up timekeeping duties */
 	tick_handover_do_timer();
 	/* Park the stopper thread */
@@ -352,12 +350,6 @@ static int take_cpu_down(void *_param)
 static int _cpu_down(unsigned int cpu, int tasks_frozen)
 {
 	int err, nr_calls = 0;
-	void *hcpu = (void *)(long)cpu;
-	unsigned long mod = tasks_frozen ? CPU_TASKS_FROZEN : 0;
-	struct take_cpu_down_param tcd_param = {
-		.mod = mod,
-		.hcpu = hcpu,
-	};
 
 	if (num_online_cpus() == 1)
 		return -EBUSY;
@@ -367,10 +359,12 @@ static int _cpu_down(unsigned int cpu, int tasks_frozen)
 
 	cpu_hotplug_begin();
 
-	err = __cpu_notify(CPU_DOWN_PREPARE | mod, hcpu, -1, &nr_calls);
+	cpuhp_tasks_frozen = tasks_frozen;
+
+	err = __cpu_notify(CPU_DOWN_PREPARE, cpu, -1, &nr_calls);
 	if (err) {
 		nr_calls--;
-		__cpu_notify(CPU_DOWN_FAILED | mod, hcpu, nr_calls, NULL);
+		__cpu_notify(CPU_DOWN_FAILED, cpu, nr_calls, NULL);
 		pr_warn("%s: attempt to take down CPU %u failed\n",
 			__func__, cpu);
 		goto out_release;
@@ -402,10 +396,10 @@ static int _cpu_down(unsigned int cpu, int tasks_frozen)
 	/*
 	 * So now all preempt/rcu users must observe !cpu_active().
 	 */
-	err = stop_machine(take_cpu_down, &tcd_param, cpumask_of(cpu));
+	err = stop_machine(take_cpu_down, NULL, cpumask_of(cpu));
 	if (err) {
 		/* CPU didn't die: tell everyone.  Can't complain. */
-		cpu_notify_nofail(CPU_DOWN_FAILED | mod, hcpu);
+		cpu_notify_nofail(CPU_DOWN_FAILED, cpu);
 		irq_unlock_sparse();
 		goto out_release;
 	}
@@ -432,14 +426,14 @@ static int _cpu_down(unsigned int cpu, int tasks_frozen)
 
 	/* CPU is completely dead: tell everyone.  Too late to complain. */
 	tick_cleanup_dead_cpu(cpu);
-	cpu_notify_nofail(CPU_DEAD | mod, hcpu);
+	cpu_notify_nofail(CPU_DEAD, cpu);
 
 	check_for_tasks(cpu);
 
 out_release:
 	cpu_hotplug_done();
 	if (!err)
-		cpu_notify_nofail(CPU_POST_DEAD | mod, hcpu);
+		cpu_notify_nofail(CPU_POST_DEAD, cpu);
 	return err;
 }
 
@@ -498,10 +492,8 @@ void smpboot_thread_init(void)
 /* Requires cpu_add_remove_lock to be held */
 static int _cpu_up(unsigned int cpu, int tasks_frozen)
 {
-	int ret, nr_calls = 0;
-	void *hcpu = (void *)(long)cpu;
-	unsigned long mod = tasks_frozen ? CPU_TASKS_FROZEN : 0;
 	struct task_struct *idle;
+	int ret, nr_calls = 0;
 
 	cpu_hotplug_begin();
 
@@ -520,7 +512,9 @@ static int _cpu_up(unsigned int cpu, int tasks_frozen)
 	if (ret)
 		goto out;
 
-	ret = __cpu_notify(CPU_UP_PREPARE | mod, hcpu, -1, &nr_calls);
+	cpuhp_tasks_frozen = tasks_frozen;
+
+	ret = __cpu_notify(CPU_UP_PREPARE, cpu, -1, &nr_calls);
 	if (ret) {
 		nr_calls--;
 		pr_warn("%s: attempt to bring up CPU %u failed\n",
@@ -536,11 +530,11 @@ static int _cpu_up(unsigned int cpu, int tasks_frozen)
 	BUG_ON(!cpu_online(cpu));
 
 	/* Now call notifier in preparation. */
-	cpu_notify(CPU_ONLINE | mod, hcpu);
+	cpu_notify(CPU_ONLINE, cpu);
 
 out_notify:
 	if (ret != 0)
-		__cpu_notify(CPU_UP_CANCELED | mod, hcpu, nr_calls, NULL);
+		__cpu_notify(CPU_UP_CANCELED, cpu, nr_calls, NULL);
 out:
 	cpu_hotplug_done();
 
@@ -732,13 +726,7 @@ core_initcall(cpu_hotplug_pm_sync_init);
  */
 void notify_cpu_starting(unsigned int cpu)
 {
-	unsigned long val = CPU_STARTING;
-
-#ifdef CONFIG_PM_SLEEP_SMP
-	if (frozen_cpus != NULL && cpumask_test_cpu(cpu, frozen_cpus))
-		val = CPU_STARTING_FROZEN;
-#endif /* CONFIG_PM_SLEEP_SMP */
-	cpu_notify(val, (void *)(long)cpu);
+	cpu_notify(CPU_STARTING, cpu);
 }
 
 #endif /* CONFIG_SMP */
-- 
2.4.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/

[toc] | [prev] | [next] | [standalone]

#1218995 — [RFC v0 3/9] x86: Use freeze_active() instead of CPU_*_FROZEN

The CPU state encodes if the CPU hotplug operation happens during suspend
or hibernate operations. Instead at looking at the encoded fields in the
CPU state variable, ask the PM subsystem directly.

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: linux-edac@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 arch/x86/kernel/cpu/mcheck/mce.c     | 13 ++++++-------
 arch/x86/kernel/cpu/microcode/core.c | 10 ++++++----
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 9d014b82..1bd421b 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -18,6 +18,7 @@
 #include <linux/rcupdate.h>
 #include <linux/kobject.h>
 #include <linux/uaccess.h>
+#include <linux/suspend.h>
 #include <linux/kdebug.h>
 #include <linux/kernel.h>
 #include <linux/percpu.h>
@@ -2341,13 +2342,12 @@ static void mce_device_remove(unsigned int cpu)
 /* Make sure there are no machine checks on offlined CPUs. */
 static void mce_disable_cpu(void *h)
 {
-	unsigned long action = *(unsigned long *)h;
 	int i;
 
 	if (!mce_available(raw_cpu_ptr(&cpu_info)))
 		return;
 
-	if (!(action & CPU_TASKS_FROZEN))
+	if (!freeze_active())
 		cmci_clear();
 	for (i = 0; i < mca_cfg.banks; i++) {
 		struct mce_bank *b = &mce_banks[i];
@@ -2359,13 +2359,12 @@ static void mce_disable_cpu(void *h)
 
 static void mce_reenable_cpu(void *h)
 {
-	unsigned long action = *(unsigned long *)h;
 	int i;
 
 	if (!mce_available(raw_cpu_ptr(&cpu_info)))
 		return;
 
-	if (!(action & CPU_TASKS_FROZEN))
+	if (!freeze_active())
 		cmci_reenable();
 	for (i = 0; i < mca_cfg.banks; i++) {
 		struct mce_bank *b = &mce_banks[i];
@@ -2395,15 +2394,15 @@ mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
 		mce_intel_hcpu_update(cpu);
 
 		/* intentionally ignoring frozen here */
-		if (!(action & CPU_TASKS_FROZEN))
+		if (!freeze_active())
 			cmci_rediscover();
 		break;
 	case CPU_DOWN_PREPARE:
-		smp_call_function_single(cpu, mce_disable_cpu, &action, 1);
+		smp_call_function_single(cpu, mce_disable_cpu, NULL, 1);
 		del_timer_sync(t);
 		break;
 	case CPU_DOWN_FAILED:
-		smp_call_function_single(cpu, mce_reenable_cpu, &action, 1);
+		smp_call_function_single(cpu, mce_reenable_cpu, NULL, 1);
 		mce_start_timer(cpu, t);
 		break;
 	}
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 9e3f3c7..e49ec2c 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -18,6 +18,7 @@
 #include <linux/platform_device.h>
 #include <linux/miscdevice.h>
 #include <linux/capability.h>
+#include <linux/suspend.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -442,6 +443,11 @@ mc_cpu_callback(struct notifier_block *nb, unsigned long action, void *hcpu)
 		pr_debug("CPU%d removed\n", cpu);
 		break;
 
+	case CPU_UP_CANCELED:
+		/* The CPU refused to come up during a system resume */
+		if (freeze_active())
+			microcode_fini_cpu(cpu);
+		break;
 	/*
 	 * case CPU_DEAD:
 	 *
@@ -452,10 +458,6 @@ mc_cpu_callback(struct notifier_block *nb, unsigned long action, void *hcpu)
 	 */
 	}
 
-	/* The CPU refused to come up during a system resume */
-	if (action == CPU_UP_CANCELED_FROZEN)
-		microcode_fini_cpu(cpu);
-
 	return NOTIFY_OK;
 }
 
-- 
2.4.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/

[toc] | [prev] | [next] | [standalone]

#1219003 — [RFC v0 9/9] doc: Update cpu-hotplug documents on removal of CPU_TASKS_FROZEN

CPU_*_FROZEN states are gone update the documentation accordingly.

I am not completely convinced that the listed known race condition
can be removed as it is done in this patch. If a suspend/resume is
ongoing while cpu_{down|up}() is called renders at least the 'always
passing 0 as tasks_frozen in cpu_up()' argument false.

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Len Brown <len.brown@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 Documentation/cpu-hotplug.txt                           | 12 +++++-------
 Documentation/fault-injection/notifier-error-inject.txt |  2 --
 Documentation/power/suspend-and-cpuhotplug.txt          | 13 ++-----------
 3 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/Documentation/cpu-hotplug.txt b/Documentation/cpu-hotplug.txt
index f9ad5e0..e362f42 100644
--- a/Documentation/cpu-hotplug.txt
+++ b/Documentation/cpu-hotplug.txt
@@ -244,9 +244,9 @@ Q: What happens when a CPU is being logically offlined?
 A: The following happen, listed in no particular order :-)
 
 - A notification is sent to in-kernel registered modules by sending an event
-  CPU_DOWN_PREPARE or CPU_DOWN_PREPARE_FROZEN, depending on whether or not the
-  CPU is being offlined while tasks are frozen due to a suspend operation in
-  progress
+  CPU_DOWN_PREPARE. Via freeze_active it is possible to inquire if a suspend
+  operation is ongoing, that is a CPU is beeing offlined while tasks
+  are frozen.
 - All processes are migrated away from this outgoing CPU to new CPUs.
   The new CPU is chosen from each process' current cpuset, which may be
   a subset of all online CPUs.
@@ -255,8 +255,8 @@ A: The following happen, listed in no particular order :-)
 - Once all services are migrated, kernel calls an arch specific routine
   __cpu_disable() to perform arch specific cleanup.
 - Once this is successful, an event for successful cleanup is sent by an event
-  CPU_DEAD (or CPU_DEAD_FROZEN if tasks are frozen due to a suspend while the
-  CPU is being offlined).
+  CPU_DEAD (if tasks are frozen due to a suspend while the
+  CPU is being offlined can be probed via freeze_active()).
 
   "It is expected that each service cleans up when the CPU_DOWN_PREPARE
   notifier is called, when CPU_DEAD is called its expected there is nothing
@@ -274,11 +274,9 @@ A: This is what you would need in your kernel code to receive notifications.
 
 		switch (action) {
 		case CPU_ONLINE:
-		case CPU_ONLINE_FROZEN:
 			foobar_online_action(cpu);
 			break;
 		case CPU_DEAD:
-		case CPU_DEAD_FROZEN:
 			foobar_dead_action(cpu);
 			break;
 		}
diff --git a/Documentation/fault-injection/notifier-error-inject.txt b/Documentation/fault-injection/notifier-error-inject.txt
index 09adabe..1ec4e84 100644
--- a/Documentation/fault-injection/notifier-error-inject.txt
+++ b/Documentation/fault-injection/notifier-error-inject.txt
@@ -23,9 +23,7 @@ the error code to debugfs interface
 Possible CPU notifier events to be failed are:
 
  * CPU_UP_PREPARE
- * CPU_UP_PREPARE_FROZEN
  * CPU_DOWN_PREPARE
- * CPU_DOWN_PREPARE_FROZEN
 
 Example1: Inject CPU offline error (-1 == -EPERM)
 
diff --git a/Documentation/power/suspend-and-cpuhotplug.txt b/Documentation/power/suspend-and-cpuhotplug.txt
index 2fc9095..ab657cd 100644
--- a/Documentation/power/suspend-and-cpuhotplug.txt
+++ b/Documentation/power/suspend-and-cpuhotplug.txt
@@ -232,7 +232,7 @@ d. Handling microcode update during suspend/hibernate:
    hibernate/restore cycle.]
 
    In the current design of the kernel however, during a CPU offline operation
-   as part of the suspend/hibernate cycle (the CPU_DEAD_FROZEN notification),
+   as part of the suspend/hibernate cycle (see freeze_active()),
    the existing copy of microcode image in the kernel is not freed up.
    And during the CPU online operations (during resume/restore), since the
    kernel finds that it already has copies of the microcode images for all the
@@ -248,16 +248,7 @@ III. Are there any known problems when regular CPU hotplug and suspend race
 
 Yes, they are listed below:
 
-1. When invoking regular CPU hotplug, the 'tasks_frozen' argument passed to
-   the _cpu_down() and _cpu_up() functions is *always* 0.
-   This might not reflect the true current state of the system, since the
-   tasks could have been frozen by an out-of-band event such as a suspend
-   operation in progress. Hence, it will lead to wrong notifications being
-   sent during the cpu online/offline events (eg, CPU_ONLINE notification
-   instead of CPU_ONLINE_FROZEN) which in turn will lead to execution of
-   inappropriate code by the callbacks registered for such CPU hotplug events.
-
-2. If a regular CPU hotplug stress test happens to race with the freezer due
+1. If a regular CPU hotplug stress test happens to race with the freezer due
    to a suspend operation in progress at the same time, then we could hit the
    situation described below:
 
-- 
2.4.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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web