Linux Power Management development
 help / color / mirror / Atom feed
* [PATCH 6/6] PM / AVS / OMAP: move Smartreflex-class3 driver to power/avs
From: Nishanth Menon @ 2012-11-05 15:02 UTC (permalink / raw)
  To: linux-omap
  Cc: Rafael, Kevin Hilman, Anton Vorontsov, linux-pm, Nishanth Menon
In-Reply-To: <1352127734-30103-1-git-send-email-nm@ti.com>

Move the AVS class 3 driver to AVS directory

Acked-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/Makefile                       |    1 -
 arch/arm/plat-omap/Kconfig                         |    9 ---------
 drivers/power/avs/Kconfig                          |    9 +++++++++
 drivers/power/avs/Makefile                         |    1 +
 .../power/avs}/smartreflex-class3.c                |    0
 5 files changed, 10 insertions(+), 10 deletions(-)
 rename {arch/arm/mach-omap2 => drivers/power/avs}/smartreflex-class3.c (100%)

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index fe40d9e..1eae31d 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -71,7 +71,6 @@ obj-$(CONFIG_SOC_OMAP5)			+= omap-mpuss-lowpower.o sleep44xx.o
 obj-$(CONFIG_PM_DEBUG)			+= pm-debug.o
 
 obj-$(CONFIG_POWER_AVS_OMAP)		+= sr_device.o
-obj-$(CONFIG_POWER_AVS_OMAP_CLASS3)	+= smartreflex-class3.o
 
 AFLAGS_sleep24xx.o			:=-Wa,-march=armv6
 AFLAGS_sleep34xx.o			:=-Wa,-march=armv7-a$(plus_sec)
diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index b49cff3..cd2c60d 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -47,15 +47,6 @@ config OMAP_DEBUG_LEDS
 	depends on OMAP_DEBUG_DEVICES
 	select LEDS_CLASS
 
-config POWER_AVS_OMAP_CLASS3
-	bool "Class 3 mode of Smartreflex Implementation"
-	depends on POWER_AVS_OMAP && TWL4030_CORE
-	help
-	  Say Y to enable Class 3 implementation of Smartreflex
-
-	  Class 3 implementation of Smartreflex employs continuous hardware
-	  voltage calibration.
-
 config OMAP_RESET_CLOCKS
 	bool "Reset unused clocks during boot"
 	depends on ARCH_OMAP
diff --git a/drivers/power/avs/Kconfig b/drivers/power/avs/Kconfig
index 466a391..4f1c1b4 100644
--- a/drivers/power/avs/Kconfig
+++ b/drivers/power/avs/Kconfig
@@ -32,3 +32,12 @@ config POWER_AVS_OMAP
 	  Optionally autocompensation can be enabled in the kernel
 	  by default during system init via the enable_on_init flag
 	  which an be passed as platform data to the smartreflex driver.
+
+config POWER_AVS_OMAP_CLASS3
+	bool "Class 3 mode of Smartreflex Implementation"
+	depends on POWER_AVS_OMAP && TWL4030_CORE
+	help
+	  Say Y to enable Class 3 implementation of Smartreflex
+
+	  Class 3 implementation of Smartreflex employs continuous hardware
+	  voltage calibration.
diff --git a/drivers/power/avs/Makefile b/drivers/power/avs/Makefile
index 0843386..ac72ec5 100644
--- a/drivers/power/avs/Makefile
+++ b/drivers/power/avs/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_POWER_AVS_OMAP)		+= smartreflex.o
+obj-$(CONFIG_POWER_AVS_OMAP_CLASS3)	+= smartreflex-class3.o
diff --git a/arch/arm/mach-omap2/smartreflex-class3.c b/drivers/power/avs/smartreflex-class3.c
similarity index 100%
rename from arch/arm/mach-omap2/smartreflex-class3.c
rename to drivers/power/avs/smartreflex-class3.c
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 2/6] ARM: OMAP: voltage: remove duplicate header definitions
From: Nishanth Menon @ 2012-11-05 15:02 UTC (permalink / raw)
  To: linux-omap
  Cc: Rafael, Kevin Hilman, Anton Vorontsov, linux-pm, Nishanth Menon
In-Reply-To: <1352127734-30103-1-git-send-email-nm@ti.com>

remove duplicate definitions which are already present
in linux/platform_data/voltage-omap.h

Acked-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/voltage.h |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm/mach-omap2/voltage.h b/arch/arm/mach-omap2/voltage.h
index 7283b7e..af9d469 100644
--- a/arch/arm/mach-omap2/voltage.h
+++ b/arch/arm/mach-omap2/voltage.h
@@ -142,7 +142,6 @@ extern void omap3xxx_voltagedomains_init(void);
 extern void am33xx_voltagedomains_init(void);
 extern void omap44xx_voltagedomains_init(void);
 
-struct voltagedomain *voltdm_lookup(const char *name);
 void voltdm_init(struct voltagedomain **voltdm_list);
 int voltdm_add_pwrdm(struct voltagedomain *voltdm, struct powerdomain *pwrdm);
 int voltdm_for_each(int (*fn)(struct voltagedomain *voltdm, void *user),
@@ -150,7 +149,5 @@ int voltdm_for_each(int (*fn)(struct voltagedomain *voltdm, void *user),
 int voltdm_for_each_pwrdm(struct voltagedomain *voltdm,
 			  int (*fn)(struct voltagedomain *voltdm,
 				    struct powerdomain *pwrdm));
-int voltdm_scale(struct voltagedomain *voltdm, unsigned long target_volt);
 void voltdm_reset(struct voltagedomain *voltdm);
-unsigned long voltdm_get_voltage(struct voltagedomain *voltdm);
 #endif
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 4/6] ARM: OMAP: SmartReflex: provide SoC integration API for VP
From: Nishanth Menon @ 2012-11-05 15:02 UTC (permalink / raw)
  To: linux-omap
  Cc: Rafael, Kevin Hilman, Anton Vorontsov, linux-pm, Nishanth Menon
In-Reply-To: <1352127734-30103-1-git-send-email-nm@ti.com>

SoC integration of SmartReflex AVS block is varied. Some use
Voltage Processor for a hardware loop in certain OMAP SoC (called
hardware loop), while others have just the AVS block without
hardware loop automatic calibration mechanism for AVS block
to talk through. So provide the Voltage Processor API
to allow for SmartReflex class drivers to use the same.

NOTE: SmartReflex class 3 mode of operation mandates VP APIs
so, refuse to enable AVS driver if corresponding APIs are
not available.

As part of this change, remove the inclusion of voltage.h
which is no longer needed as smartreflex.h includes
linux/platform_data/voltage-omap.h which contain relevant
definitions used here.

Acked-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/smartreflex-class3.c |   16 +++++++++++++---
 arch/arm/mach-omap2/sr_device.c          |    5 +++++
 drivers/power/avs/smartreflex.c          |    2 ++
 include/linux/power/smartreflex.h        |   18 ++++++++++++++++++
 4 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex-class3.c b/arch/arm/mach-omap2/smartreflex-class3.c
index 1da8f03..7ccf57f 100644
--- a/arch/arm/mach-omap2/smartreflex-class3.c
+++ b/arch/arm/mach-omap2/smartreflex-class3.c
@@ -12,7 +12,6 @@
  */
 
 #include <linux/power/smartreflex.h>
-#include "voltage.h"
 
 static int sr_class3_enable(struct omap_sr *sr)
 {
@@ -23,15 +22,26 @@ static int sr_class3_enable(struct omap_sr *sr)
 				__func__, sr->name);
 		return -ENODATA;
 	}
+	if (!sr->soc_ops.vp_enable) {
+		pr_warn("%s: no VP enable available. Cannot enable %s!!\n",
+			__func__, sr->name);
+		return -EINVAL;
+	}
 
-	omap_vp_enable(sr->voltdm);
+	sr->soc_ops.vp_enable(sr->voltdm);
 	return sr_enable(sr->voltdm, volt);
 }
 
 static int sr_class3_disable(struct omap_sr *sr, int is_volt_reset)
 {
+	if (!sr->soc_ops.vp_disable) {
+		pr_warn("%s: no VP disable available. Cannot disable %s!!\n",
+			__func__, sr->name);
+		return -EINVAL;
+	}
 	sr_disable_errgen(sr->voltdm);
-	omap_vp_disable(sr->voltdm);
+
+	sr->soc_ops.vp_disable(sr->voltdm);
 	sr_disable(sr->voltdm);
 	if (is_volt_reset)
 		voltdm_reset(sr->voltdm);
diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index f8217a5..6aac2c7 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -139,6 +139,11 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
 
 	sr_data->enable_on_init = sr_enable_on_init;
 
+	if (sr_data->voltdm->vp) {
+		sr_data->soc_ops.vp_enable = omap_vp_enable;
+		sr_data->soc_ops.vp_disable = omap_vp_disable;
+	}
+
 	pdev = omap_device_build(name, i, oh, sr_data, sizeof(*sr_data),
 				 NULL, 0, 0);
 	if (IS_ERR(pdev))
diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index 24768a2..32a9e3e 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -920,6 +920,8 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	sr_info->pdev = pdev;
 	sr_info->srid = pdev->id;
 	sr_info->voltdm = pdata->voltdm;
+	sr_info->soc_ops.vp_enable =  pdata->soc_ops.vp_enable;
+	sr_info->soc_ops.vp_disable =  pdata->soc_ops.vp_disable;
 	sr_info->nvalue_table = pdata->nvalue_table;
 	sr_info->nvalue_count = pdata->nvalue_count;
 	sr_info->senn_mod = pdata->senn_mod;
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 4a496eb..203fc64 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -143,6 +143,21 @@
 #define OMAP3430_SR_ERRWEIGHT		0x04
 #define OMAP3430_SR_ERRMAXLIMIT		0x02
 
+/**
+ * struct omap_sr_soc_ops - SoC specific APIs
+ * @vp_enable:	Voltage Processor enable
+ * @vp_disable:	Voltage Processor disable
+ *
+ * SmartReflex AVS module integration tends to be SoC
+ * variant. some are integrated with modules like
+ * Voltage Processor (VP), while, some SoC integration
+ * donot use VP. Provide that variance here.
+ */
+struct omap_sr_soc_ops {
+	void (*vp_enable)(struct voltagedomain *voltdm);
+	void (*vp_disable)(struct voltagedomain *voltdm);
+};
+
 struct omap_sr {
 	char				*name;
 	struct list_head		node;
@@ -165,6 +180,7 @@ struct omap_sr {
 	u32				senp_mod;
 	u32				senn_mod;
 	void __iomem			*base;
+	struct omap_sr_soc_ops		soc_ops;
 };
 
 /**
@@ -268,6 +284,7 @@ struct omap_sr_nvalue_table {
  * @nvalue_table:	table containing the  efuse offsets and nvalues
  *			corresponding to them.
  * @voltdm:		Pointer to the voltage domain associated with the SR
+ * @soc_ops:		SoC specific ops to deal with integration variance
  */
 struct omap_sr_data {
 	const char			*name;
@@ -278,6 +295,7 @@ struct omap_sr_data {
 	bool				enable_on_init;
 	struct omap_sr_nvalue_table	*nvalue_table;
 	struct voltagedomain		*voltdm;
+	struct omap_sr_soc_ops		soc_ops;
 };
 
 /* Smartreflex module enable/disable interface */
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 0/6] ARM: OMAP3+: move smartreflex-class3.c to drivers/power/avs
From: Nishanth Menon @ 2012-11-05 15:02 UTC (permalink / raw)
  To: linux-omap
  Cc: Rafael, Kevin Hilman, Anton Vorontsov, linux-pm, Nishanth Menon
In-Reply-To: <1352127734-30103-1-git-send-email-nm@ti.com>

smartreflex.c now resides in drivers/power/avs directory,
but class driver is in mach-omap2. High time we move it
to drivers/power/avs.

This series *does not* try to fix VP/VC to be voltage
regulator OR introduce a new OMAP voltage regulator series.
Instead, it purely tries to do the minimal changes needed
to move code to drivers/power/avs as a start.

Baseline: k.org v3.7-rc4
Testing: Platform: beagle XM C1 (3730)
while [ 1 ]
do
echo -n "0" >/sys/kernel/debug/smartreflex/smartreflex_core/autocomp
date
echo -n "1" >/sys/kernel/debug/smartreflex/smartreflex_core/autocomp
done
Screen capture after the series(RFC): on inductor L5 (VDD2 - core):
https://plus.google.com/photos/112464029509057661457/albums/5715034179943520193/5802619719180530114

RFC: http://marc.info/?t=135102876700003&r=1&w=2

Nishanth Menon (6):
  PM / AVS / OMAP: move Kconfig definition of smartreflex to avs
    directory
  ARM: OMAP: voltage: remove duplicate header definitions
  ARM: OMAP: voltage: move voltdm_reset to platform_data header
  ARM: OMAP: SmartReflex: provide SoC integration API for VP
  ARM: OMAP: SmartReflex: use pr_warn instead of pr_warning
  PM / AVS / OMAP: move Smartreflex-class3 driver to power/avs

 arch/arm/mach-omap2/Makefile                       |    1 -
 arch/arm/mach-omap2/sr_device.c                    |    5 ++++
 arch/arm/mach-omap2/voltage.h                      |    4 ---
 arch/arm/plat-omap/Kconfig                         |   31 --------------------
 drivers/power/avs/Kconfig                          |   31 ++++++++++++++++++++
 drivers/power/avs/Makefile                         |    1 +
 .../power/avs}/smartreflex-class3.c                |   20 +++++++++----
 drivers/power/avs/smartreflex.c                    |    2 ++
 include/linux/platform_data/voltage-omap.h         |    1 +
 include/linux/power/smartreflex.h                  |   18 ++++++++++++
 10 files changed, 73 insertions(+), 41 deletions(-)
 rename {arch/arm/mach-omap2 => drivers/power/avs}/smartreflex-class3.c (75%)
Regards,
Nishanth Menon
-- 
1.7.9.5


^ permalink raw reply

* [PATCH 5/6] ARM: OMAP: SmartReflex: use pr_warn instead of pr_warning
From: Nishanth Menon @ 2012-11-05 15:02 UTC (permalink / raw)
  To: linux-omap
  Cc: Rafael, Kevin Hilman, Anton Vorontsov, linux-pm, Nishanth Menon
In-Reply-To: <1352127734-30103-1-git-send-email-nm@ti.com>

Minor cleanup to use the preferred pr_warn

Acked-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/smartreflex-class3.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex-class3.c b/arch/arm/mach-omap2/smartreflex-class3.c
index 7ccf57f..23d45f9 100644
--- a/arch/arm/mach-omap2/smartreflex-class3.c
+++ b/arch/arm/mach-omap2/smartreflex-class3.c
@@ -18,8 +18,8 @@ static int sr_class3_enable(struct omap_sr *sr)
 	unsigned long volt = voltdm_get_voltage(sr->voltdm);
 
 	if (!volt) {
-		pr_warning("%s: Curr voltage unknown. Cannot enable %s\n",
-				__func__, sr->name);
+		pr_warn("%s: Curr voltage unknown. Cannot enable %s\n",
+			__func__, sr->name);
 		return -ENODATA;
 	}
 	if (!sr->soc_ops.vp_enable) {
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 3/6] ARM: OMAP: voltage: move voltdm_reset to platform_data header
From: Nishanth Menon @ 2012-11-05 15:02 UTC (permalink / raw)
  To: linux-omap
  Cc: Rafael, Kevin Hilman, Anton Vorontsov, linux-pm, Nishanth Menon
In-Reply-To: <1352127734-30103-1-git-send-email-nm@ti.com>

Move voltdm_reset to include/linux/platform_data/voltage-omap.h

Acked-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/voltage.h              |    1 -
 include/linux/platform_data/voltage-omap.h |    1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/voltage.h b/arch/arm/mach-omap2/voltage.h
index af9d469..0665f21 100644
--- a/arch/arm/mach-omap2/voltage.h
+++ b/arch/arm/mach-omap2/voltage.h
@@ -149,5 +149,4 @@ int voltdm_for_each(int (*fn)(struct voltagedomain *voltdm, void *user),
 int voltdm_for_each_pwrdm(struct voltagedomain *voltdm,
 			  int (*fn)(struct voltagedomain *voltdm,
 				    struct powerdomain *pwrdm));
-void voltdm_reset(struct voltagedomain *voltdm);
 #endif
diff --git a/include/linux/platform_data/voltage-omap.h b/include/linux/platform_data/voltage-omap.h
index 5be4d5d..4eb3d43 100644
--- a/include/linux/platform_data/voltage-omap.h
+++ b/include/linux/platform_data/voltage-omap.h
@@ -36,4 +36,5 @@ int voltdm_scale(struct voltagedomain *voltdm, unsigned long target_volt);
 unsigned long voltdm_get_voltage(struct voltagedomain *voltdm);
 struct omap_volt_data *omap_voltage_get_voltdata(struct voltagedomain *voltdm,
 		unsigned long volt);
+void voltdm_reset(struct voltagedomain *voltdm);
 #endif
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 1/6] PM / AVS / OMAP: move Kconfig definition of smartreflex to avs directory
From: Nishanth Menon @ 2012-11-05 15:02 UTC (permalink / raw)
  To: linux-omap
  Cc: Rafael, Kevin Hilman, Anton Vorontsov, linux-pm, Nishanth Menon
In-Reply-To: <1352127734-30103-1-git-send-email-nm@ti.com>

Don't see why the source should be in drivers/power/avs, but not config option

Acked-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/plat-omap/Kconfig |   22 ----------------------
 drivers/power/avs/Kconfig  |   22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 82fcb20..b49cff3 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -47,28 +47,6 @@ config OMAP_DEBUG_LEDS
 	depends on OMAP_DEBUG_DEVICES
 	select LEDS_CLASS
 
-config POWER_AVS_OMAP
-	bool "AVS(Adaptive Voltage Scaling) support for OMAP IP versions 1&2"
-	depends on POWER_AVS && (ARCH_OMAP3 || ARCH_OMAP4) && PM
-	select POWER_SUPPLY
-	help
-	  Say Y to enable AVS(Adaptive Voltage Scaling)
-	  support on OMAP containing the version 1 or
-	  version 2 of the SmartReflex IP.
-	  V1 is the 65nm version used in OMAP3430.
-	  V2 is the update for the 45nm version of the IP used in OMAP3630
-	  and OMAP4430
-
-	  Please note, that by default SmartReflex is only
-	  initialized and not enabled. To enable the automatic voltage
-	  compensation for vdd mpu and vdd core from user space,
-	  user must write 1 to
-		/debug/smartreflex/sr_<X>/autocomp,
-	  where X is mpu_iva or core for OMAP3.
-	  Optionally autocompensation can be enabled in the kernel
-	  by default during system init via the enable_on_init flag
-	  which an be passed as platform data to the smartreflex driver.
-
 config POWER_AVS_OMAP_CLASS3
 	bool "Class 3 mode of Smartreflex Implementation"
 	depends on POWER_AVS_OMAP && TWL4030_CORE
diff --git a/drivers/power/avs/Kconfig b/drivers/power/avs/Kconfig
index 2a1008b..466a391 100644
--- a/drivers/power/avs/Kconfig
+++ b/drivers/power/avs/Kconfig
@@ -10,3 +10,25 @@ menuconfig POWER_AVS
 	  AVS is also called SmartReflex on OMAP devices.
 
 	  Say Y here to enable Adaptive Voltage Scaling class support.
+
+config POWER_AVS_OMAP
+	bool "AVS(Adaptive Voltage Scaling) support for OMAP IP versions 1&2"
+	depends on POWER_AVS && (ARCH_OMAP3 || ARCH_OMAP4) && PM
+	select POWER_SUPPLY
+	help
+	  Say Y to enable AVS(Adaptive Voltage Scaling)
+	  support on OMAP containing the version 1 or
+	  version 2 of the SmartReflex IP.
+	  V1 is the 65nm version used in OMAP3430.
+	  V2 is the update for the 45nm version of the IP used in OMAP3630
+	  and OMAP4430
+
+	  Please note, that by default SmartReflex is only
+	  initialized and not enabled. To enable the automatic voltage
+	  compensation for vdd mpu and vdd core from user space,
+	  user must write 1 to
+		/debug/smartreflex/sr_<X>/autocomp,
+	  where X is mpu_iva or core for OMAP3.
+	  Optionally autocompensation can be enabled in the kernel
+	  by default during system init via the enable_on_init flag
+	  which an be passed as platform data to the smartreflex driver.
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 0/6] ARM: OMAP3+: move smartreflex-class3.c to drivers/power/avs
From: Nishanth Menon @ 2012-11-05 15:02 UTC (permalink / raw)
  To: linux-omap
  Cc: Rafael, Kevin Hilman, Anton Vorontsov, linux-pm, Nishanth Menon

smartreflex.c now resides in drivers/power/avs directory,
but class driver is in mach-omap2. High time we move it
to drivers/power/avs.

This series *does not* try to fix VP/VC to be voltage
regulator OR introduce a new OMAP voltage regulator series.
Instead, it purely tries to do the minimal changes needed
to move code to drivers/power/avs as a start.

Baseline: k.org v3.7-rc4
Testing: Platform: beagle XM C1 (3730)
while [ 1 ]
do
echo -n "0" >/sys/kernel/debug/smartreflex/smartreflex_core/autocomp
date
echo -n "1" >/sys/kernel/debug/smartreflex/smartreflex_core/autocomp
done
Screen capture after the series(RFC): on inductor L5 (VDD2 - core):
https://plus.google.com/photos/112464029509057661457/albums/5715034179943520193/5802619719180530114

RFC: http://marc.info/?t=135102876700003&r=1&w=2
Changes since RFC:
 - added Jean's Acked-by
 - review comments addressed
Nishanth Menon (6):
  PM / AVS / OMAP: move Kconfig definition of smartreflex to avs
    directory
  ARM: OMAP: voltage: remove duplicate header definitions
  ARM: OMAP: voltage: move voltdm_reset to platform_data header
  ARM: OMAP: SmartReflex: provide SoC integration API for VP
  ARM: OMAP: SmartReflex: use pr_warn instead of pr_warning
  PM / AVS / OMAP: move Smartreflex-class3 driver to power/avs

 arch/arm/mach-omap2/Makefile                       |    1 -
 arch/arm/mach-omap2/sr_device.c                    |    5 ++++
 arch/arm/mach-omap2/voltage.h                      |    4 ---
 arch/arm/plat-omap/Kconfig                         |   31 --------------------
 drivers/power/avs/Kconfig                          |   31 ++++++++++++++++++++
 drivers/power/avs/Makefile                         |    1 +
 .../power/avs}/smartreflex-class3.c                |   20 +++++++++----
 drivers/power/avs/smartreflex.c                    |    2 ++
 include/linux/platform_data/voltage-omap.h         |    1 +
 include/linux/power/smartreflex.h                  |   18 ++++++++++++
 10 files changed, 73 insertions(+), 41 deletions(-)
 rename {arch/arm/mach-omap2 => drivers/power/avs}/smartreflex-class3.c (75%)

Regards,
Nishanth Menon
-- 
1.7.9.5


^ permalink raw reply

* Re: [PATCH 1/9] cgroup: add cgroup_subsys->post_create()
From: Glauber Costa @ 2012-11-05 13:42 UTC (permalink / raw)
  To: Tejun Heo
  Cc: rjw-KKrjLPT3xs0, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, mhocko-AlSwsSmVLrQ,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1351931915-1701-2-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On 11/03/2012 09:38 AM, Tejun Heo wrote:
> Currently, there's no way for a controller to find out whether a new
> cgroup finished all ->create() allocatinos successfully and is
> considered "live" by cgroup.
> 
> This becomes a problem later when we add generic descendants walking
> to cgroup which can be used by controllers as controllers don't have a
> synchronization point where it can synchronize against new cgroups
> appearing in such walks.
> 
> This patch adds ->post_create().  It's called after all ->create()
> succeeded and the cgroup is linked into the generic cgroup hierarchy.
> This plays the counterpart of ->pre_destroy().
> 
> Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

Tejun, If we do it this way, we end up with two callbacks that are
called after create: post_clone and post_create. I myself prefer the
approach I took, that convert post_clone into post_create, and would
prefer if you would pick that up.

For me, post_clone is totally a glitch that should not exist. Merging
this with post_create gives the following semantics:

* A while after cgroup creation, you will get a callback. In that
callback, you do whatever initialization you may need that you could not
in create. Why is reacting to a flag being set any different?

^ permalink raw reply

* Re: [PATCH 0/8] clk: ux500: Fixup smp_twd clk for clk notifiers
From: Ulf Hansson @ 2012-11-05 12:33 UTC (permalink / raw)
  To: Mike Turquette, Mike Turquette
  Cc: Philippe Begnic, Vincent Guittot, Samuel Ortiz, Jonas Aberg,
	linux-pm, Linus Walleij, cpufreq, Rickard Andersson,
	Rafael J. Wysocki, Ulf Hansson, linux-arm-kernel
In-Reply-To: <20121028225918.GC5411@sortiz-mobl>

On 28 October 2012 23:59, Samuel Ortiz <sameo@linux.intel.com> wrote:
> Hi Ulf,
>
> On Fri, Oct 26, 2012 at 10:19:24AM +0200, Ulf Hansson wrote:
>> On 11 October 2012 17:19, Lee Jones <lee.jones@linaro.org> wrote:
>> > On Thu, 11 Oct 2012, Linus Walleij wrote:
>> >
>> >> On Thu, Oct 11, 2012 at 3:44 PM, Lee Jones <lee.jones@linaro.org> wrote:
>> >>
>> >> >> Patches are based on Linus Torvalds tree with latest commit as of okt 10.
>> >> >
>> >> > Hmm... I get:
>> >> >
>> >> > Applying: clk: ux500: Support for prcmu_scalable_rate clock
>> >> > error: drivers/clk/ux500/clk-prcmu.c: does not exist in index
>> >> > error: drivers/clk/ux500/clk.h: does not exist in index
>> >> > Patch failed at 0001 clk: ux500: Support for prcmu_scalable_rate clock
>> >> >
>> >> > So when did drivers/clk/ux500/* arrive?
>> >>
>> >> Exactly here, 10 days ago on Torvalds' master branch:
>> >
>> > Ah I see. Basing patches on commits half way through the merge
>> > window. Good move! ;)
>> >
>> > --
>> > Lee Jones
>> > Linaro ST-Ericsson Landing Team Lead
>> > Linaro.org │ Open source software for ARM SoCs
>> > Follow Linaro: Facebook | Twitter | Blog
>>
>> Samuel, a kind reminder on this, trying to collect acks to be able to
>> advise Mike to take this series through his clk tree. There are two
>> mfd patches.
>> [PATCH 2/8] mfd: db8500: Provide cpufreq table as platform data
>> [PATCH 5/8] mfd: db8500: Connect ARMSS clk to ARM OPP
> Sorry for the delay:
>
> Acked-by: Samuel Ortiz <sameo@linux.intel.com>
>
> Cheers,
> Samuel.
>
> --
> Intel Open Source Technology Centre
> http://oss.intel.com/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

A kind ping for this this series for you Mike. I believe all patches
has received acks now as well.

Kind regards
Ulf Hansson

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [BUGFIX] PM: Fix active child counting when disabled and forbidden
From: Alan Stern @ 2012-11-05  1:56 UTC (permalink / raw)
  To: Huang Ying; +Cc: Rafael J. Wysocki, linux-kernel, linux-pm
In-Reply-To: <1352078237-20622-1-git-send-email-ying.huang@intel.com>

On Mon, 5 Nov 2012, Huang Ying wrote:

> In current runtime PM implementation, the active child count of the
> parent device may be decreased if the runtime PM of the child device
> is disabled and forbidden.  For example, to unbind a PCI driver with a
> PCI device, the following code path is possible:
> 
>   pci_device_remove
>     pm_runtime_set_suspended
>       __pm_runtime_set_status
>         atomic_add_unless(&parent->power.child_count, -1, 0)
> 
> That is, the parent device may be suspended, even if the runtime PM of
> child device is forbidden to be suspended.  This violate the rule that
> parent is allowed to be suspended only after all its children are
> suspended, and may cause issue.

This doesn't sound like a correct description of the situation.  The 
rule is not violated.  After pm_runtime_set_suspended runs, the child 
_is_ suspended.  Thus there's no reason not to allow the parent to be 
suspended.

The problem -- if there really is one -- is that a driver can put a 
device into the suspended state by calling pm_runtime_disable followed 
by pm_runtime_set_suspended, even if the usage count is > 0.

I'm not so sure this should count as a problem.  Generally devices 
aren't disabled for runtime PM unless something is wrong.  Under those 
circumstances, the meaning of pm_runtime_forbid isn't very well 
defined.

Alan Stern


^ permalink raw reply

* [PATCH] Fixup compile error of try_to_freeze_nowarn
From: Li Haifeng @ 2012-11-05  1:42 UTC (permalink / raw)
  To: Len Brown, Pavel Machek, Rafael J. Wysocki, linux-pm,
	linux-kernel

Subject: [PATCH] Fixup compile error of try_to_freeze_nowarn

If FREEZER is not defined, the error as following will be throw
when compiled.
arch/arm/kernel/signal.c:645: error: implicit declaration of function
'try_to_freeze_nowarn'

Signed-off-by: Haifeng Li <omycle@gmail.com>
---
 include/linux/freezer.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index a628084..3c0c1fb 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -188,6 +188,7 @@ static inline int freeze_kernel_threads(void) {
return -ENOSYS; }
 static inline void thaw_processes(void) {}
 static inline void thaw_kernel_threads(void) {}

+static inline bool try_to_freeze_nowarn(void) { return false; }
 static inline bool try_to_freeze(void) { return false; }

 static inline void freezer_do_not_count(void) {}
--
1.7.9.5

^ permalink raw reply related

* [BUGFIX] PM: Fix active child counting when disabled and forbidden
From: Huang Ying @ 2012-11-05  1:17 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: linux-kernel, linux-pm, Huang Ying

In current runtime PM implementation, the active child count of the
parent device may be decreased if the runtime PM of the child device
is disabled and forbidden.  For example, to unbind a PCI driver with a
PCI device, the following code path is possible:

  pci_device_remove
    pm_runtime_set_suspended
      __pm_runtime_set_status
        atomic_add_unless(&parent->power.child_count, -1, 0)

That is, the parent device may be suspended, even if the runtime PM of
child device is forbidden to be suspended.  This violate the rule that
parent is allowed to be suspended only after all its children are
suspended, and may cause issue.

This issue is fixed via checking the usage count of the child device
because if the runtime PM of the child device is forbidden, the
usage_count of the child device will be non-zero.

Signed-off-by: Huang Ying <ying.huang@intel.com>
---
 drivers/base/power/runtime.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -916,7 +916,7 @@ int __pm_runtime_set_status(struct devic
 
 	if (status == RPM_SUSPENDED) {
 		/* It always is possible to set the status to 'suspended'. */
-		if (parent) {
+		if (parent && atomic_read(&dev->power.usage_count) == 0) {
 			atomic_add_unless(&parent->power.child_count, -1, 0);
 			notify_parent = !parent->power.ignore_children;
 		}

^ permalink raw reply

* Re: [PATCH] PM / Qos: Ensure device not in PRM_SUSPENDED when pm qos flags request functions are invoked in the pm core.
From: Lan Tianyu @ 2012-11-04 15:09 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: stern, linux-pm, linux-acpi
In-Reply-To: <3462113.4oKq44aJDs@vostro.rjw.lan>

On 2012/11/3 4:11, Rafael J. Wysocki wrote:
>>>>    }
>>>> > >>   EXPORT_SYMBOL_GPL(dev_pm_qos_expose_flags);
>>>> > >>@@ -645,7 +649,9 @@ void dev_pm_qos_hide_flags(struct device *dev)
>>>> > >>   {
>>>> > >>   	if (dev->power.qos && dev->power.qos->flags_req) {
>>>> > >>   		pm_qos_sysfs_remove_flags(dev);
>>>> > >>+		pm_runtime_get_sync(dev);
>>>> > >>   		__dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_FLAGS);
>>>> > >>+		pm_runtime_put(dev);
>>> > >
>>> > >I'm not sure if these two are necessary.  If we remove a request,
>>> > >then what happens worst case is that some flags will be cleared
>>> > >effectively which means fewer restrictions on the next sleep state.
>>> > >Then, it shouldn't hurt that the current sleep state is more
>>> > >restricted.
>> >
>> >But this mean the device can be put into lower power state(power off).
>> >So why not do that? that can save more power, right?
> Correct.  On the other hand, though, if the device already is in the
> deepest low-power state available, we will resume it unnecessarily this
> way.  Which may not be a big deal, however, and since we do that in other
> cases, we may as well do it here.
Yeah. This seems not very reasonable. But we can optimize this 
later.From my previous opinion, add notifier for flags and let device 
driver or bus driver to determine when the device should be resumed. 
Since you said at another email you would remove all notifiers in the pm 
qos to make some functions able to be invoked in interrupt context. I 
have a thought that check the context before call notifiers chain. If it 
was in interrupt, not call notifier chain and trigger a work queue or 
other choices to do that. If not, call the chain. Does this make sense? :)

>
> Thanks,
> Rafael


-- 
Best Regards
Tianyu Lan
linux kernel enabling team

^ permalink raw reply

* Re: [BUGFIX 2/2] PCI/PM: Resume device before shutdown
From: Huang Ying @ 2012-11-04  3:44 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-kernel, linux-pci, linux-pm, Rafael J. Wysocki
In-Reply-To: <CAErSpo7=6kCG=P-u_MOACD+HVV8cj1JLO-beZYr6tn9bOzfrBg@mail.gmail.com>

On Sat, 2012-11-03 at 11:21 -0600, Bjorn Helgaas wrote:
> On Fri, Nov 2, 2012 at 11:05 PM, Huang Ying <ying.huang@intel.com> wrote:
> > On Fri, 2012-11-02 at 10:52 -0600, Bjorn Helgaas wrote:
> >> On Wed, Oct 24, 2012 at 12:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> >> > Some actions during shutdown need device to be in D0 state, such as
> >> > MSI shutdown etc, so resume device before shutdown.
> >>
> >> Is there a problem report or bugzilla for this issue?  What are the
> >> symptoms by which a user could figure out that he needs this fix?
> >
> > No bugzilla for this issue.  This issue will cause the corresponding
> > device lost in kexeced kernel.
> 
> So would the following be accurate changelog text?
> 
>     Without this patch, a device may not work correctly after a kexec
>     because the new kernel expects devices to be in D0.

The issue I encountered is

	Without this patch, a device may not be enumerated after a kexec
	because the corresponding bridge is not in D0, so that
	configuration space of the device is not accessible.

> >> Should this be put in the stable tree as well?  If so, for v3.6 only?
> 
> What about the stable tree?

Yes.  This patch should be for v3.6 stable tree only.

Best Regards,
Huang Ying

> >> > Signed-off-by: Huang Ying <ying.huang@intel.com>
> >> > ---
> >> >  drivers/pci/pci-driver.c |   12 ++----------
> >> >  1 file changed, 2 insertions(+), 10 deletions(-)
> >> >
> >> > --- a/drivers/pci/pci-driver.c
> >> > +++ b/drivers/pci/pci-driver.c
> >> > @@ -398,6 +398,8 @@ static void pci_device_shutdown(struct d
> >> >         struct pci_dev *pci_dev = to_pci_dev(dev);
> >> >         struct pci_driver *drv = pci_dev->driver;
> >> >
> >> > +       pm_runtime_resume(dev);
> >> > +
> >> >         if (drv && drv->shutdown)
> >> >                 drv->shutdown(pci_dev);
> >> >         pci_msi_shutdown(pci_dev);
> >> > @@ -408,16 +410,6 @@ static void pci_device_shutdown(struct d
> >> >          * continue to do DMA
> >> >          */
> >> >         pci_disable_device(pci_dev);
> >> > -
> >> > -       /*
> >> > -        * Devices may be enabled to wake up by runtime PM, but they need not
> >> > -        * be supposed to wake up the system from its "power off" state (e.g.
> >> > -        * ACPI S5).  Therefore disable wakeup for all devices that aren't
> >> > -        * supposed to wake up the system at this point.  The state argument
> >> > -        * will be ignored by pci_enable_wake().
> >> > -        */
> >> > -       if (!device_may_wakeup(dev))
> >> > -               pci_enable_wake(pci_dev, PCI_UNKNOWN, false);
> >> >  }
> >> >
> >> >  #ifdef CONFIG_PM
> >
> >



^ permalink raw reply

* Re: [BUGFIX 1/2] PCI/PM: Fix deadlock when unbind device if its parent in D3cold
From: Huang Ying @ 2012-11-04  3:38 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-kernel, linux-pci, linux-pm, Rafael J. Wysocki,
	Zhang Yanmin
In-Reply-To: <CAErSpo4k+SjqdBt3w3POkg=toCnZufH4gnwJpV8BW8DH5abkqw@mail.gmail.com>

On Sat, 2012-11-03 at 11:22 -0600, Bjorn Helgaas wrote:
> On Fri, Nov 2, 2012 at 11:06 PM, Huang Ying <ying.huang@intel.com> wrote:
> > On Fri, 2012-11-02 at 10:50 -0600, Bjorn Helgaas wrote:
> >> On Wed, Oct 24, 2012 at 12:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> >> > If a PCI device and its parents are put into D3cold, unbinding the
> >> > device will trigger deadlock as follow:
> >> >
> >> > - driver_unbind
> >> >   - device_release_driver
> >> >     - device_lock(dev)                          <--- previous lock here
> >> >     - __device_release_driver
> >> >       - pm_runtime_get_sync
> >> >         ...
> >> >           - rpm_resume(dev)
> >> >             - rpm_resume(dev->parent)
> >> >               ...
> >> >                 - pci_pm_runtime_resume
> >> >                   ...
> >> >                   - pci_set_power_state
> >> >                     - __pci_start_power_transition
> >> >                       - pci_wakeup_bus(dev->parent->subordinate)
> >> >                         - pci_walk_bus
> >> >                           - device_lock(dev)    <--- dead lock here
> >> >
> >> >
> >> > If we do not do device_lock in pci_walk_bus, we can avoid dead lock.
> >> > Device_lock in pci_walk_bus is introduced in commit:
> >> > d71374dafbba7ec3f67371d3b7e9f6310a588808, corresponding email thread
> >> > is: https://lkml.org/lkml/2006/5/26/38.  The patch author Zhang Yanmin
> >> > said device_lock is added to pci_walk_bus because:
> >> >
> >> >   Some error handling functions call pci_walk_bus. For example, PCIe
> >> >   aer. Here we lock the device, so the driver wouldn't detach from the
> >> >   device, as the cb might call driver's callback function.
> >> >
> >> > So I fixed the dead lock as follow:
> >> >
> >> > - remove device_lock from pci_walk_bus
> >> > - add device_lock into callback if callback will call driver's callback
> >> >
> >> > I checked pci_walk_bus users one by one, and found only PCIe aer needs
> >> > device lock.
> >>
> >> Is there a problem report or bugzilla for this issue?
> >
> > No.  I found this issue when I developed kexec fixing.
> >
> > Best Regards,
> > Huang Ying
> >
> >> > Signed-off-by: Huang Ying <ying.huang@intel.com>
> >> > Cc: Zhang Yanmin <yanmin.zhang@intel.com>
> >>
> >> Should this go to stable as well?  The D3cold support appeared in
> >> v3.6, so my guess is that this fix could go to v3.6.x.
> 
> What about the stable tree?

You are right.  This fix should go to v3.6.x stable tree.

Best Regards,
Huang Ying

> >> > ---
> >> >  drivers/pci/bus.c                  |    3 ---
> >> >  drivers/pci/pcie/aer/aerdrv_core.c |   20 ++++++++++++++++----
> >> >  2 files changed, 16 insertions(+), 7 deletions(-)
> >> >
> >> > --- a/drivers/pci/bus.c
> >> > +++ b/drivers/pci/bus.c
> >> > @@ -320,10 +320,7 @@ void pci_walk_bus(struct pci_bus *top, i
> >> >                 } else
> >> >                         next = dev->bus_list.next;
> >> >
> >> > -               /* Run device routines with the device locked */
> >> > -               device_lock(&dev->dev);
> >> >                 retval = cb(dev, userdata);
> >> > -               device_unlock(&dev->dev);
> >> >                 if (retval)
> >> >                         break;
> >> >         }
> >> > --- a/drivers/pci/pcie/aer/aerdrv_core.c
> >> > +++ b/drivers/pci/pcie/aer/aerdrv_core.c
> >> > @@ -213,6 +213,7 @@ static int report_error_detected(struct
> >> >         struct aer_broadcast_data *result_data;
> >> >         result_data = (struct aer_broadcast_data *) data;
> >> >
> >> > +       device_lock(&dev->dev);
> >> >         dev->error_state = result_data->state;
> >> >
> >> >         if (!dev->driver ||
> >> > @@ -231,12 +232,14 @@ static int report_error_detected(struct
> >> >                                    dev->driver ?
> >> >                                    "no AER-aware driver" : "no driver");
> >> >                 }
> >> > -               return 0;
> >> > +               goto out;
> >> >         }
> >> >
> >> >         err_handler = dev->driver->err_handler;
> >> >         vote = err_handler->error_detected(dev, result_data->state);
> >> >         result_data->result = merge_result(result_data->result, vote);
> >> > +out:
> >> > +       device_unlock(&dev->dev);
> >> >         return 0;
> >> >  }
> >> >
> >> > @@ -247,14 +250,17 @@ static int report_mmio_enabled(struct pc
> >> >         struct aer_broadcast_data *result_data;
> >> >         result_data = (struct aer_broadcast_data *) data;
> >> >
> >> > +       device_lock(&dev->dev);
> >> >         if (!dev->driver ||
> >> >                 !dev->driver->err_handler ||
> >> >                 !dev->driver->err_handler->mmio_enabled)
> >> > -               return 0;
> >> > +               goto out;
> >> >
> >> >         err_handler = dev->driver->err_handler;
> >> >         vote = err_handler->mmio_enabled(dev);
> >> >         result_data->result = merge_result(result_data->result, vote);
> >> > +out:
> >> > +       device_unlock(&dev->dev);
> >> >         return 0;
> >> >  }
> >> >
> >> > @@ -265,14 +271,17 @@ static int report_slot_reset(struct pci_
> >> >         struct aer_broadcast_data *result_data;
> >> >         result_data = (struct aer_broadcast_data *) data;
> >> >
> >> > +       device_lock(&dev->dev);
> >> >         if (!dev->driver ||
> >> >                 !dev->driver->err_handler ||
> >> >                 !dev->driver->err_handler->slot_reset)
> >> > -               return 0;
> >> > +               goto out;
> >> >
> >> >         err_handler = dev->driver->err_handler;
> >> >         vote = err_handler->slot_reset(dev);
> >> >         result_data->result = merge_result(result_data->result, vote);
> >> > +out:
> >> > +       device_unlock(&dev->dev);
> >> >         return 0;
> >> >  }
> >> >
> >> > @@ -280,15 +289,18 @@ static int report_resume(struct pci_dev
> >> >  {
> >> >         const struct pci_error_handlers *err_handler;
> >> >
> >> > +       device_lock(&dev->dev);
> >> >         dev->error_state = pci_channel_io_normal;
> >> >
> >> >         if (!dev->driver ||
> >> >                 !dev->driver->err_handler ||
> >> >                 !dev->driver->err_handler->resume)
> >> > -               return 0;
> >> > +               goto out;
> >> >
> >> >         err_handler = dev->driver->err_handler;
> >> >         err_handler->resume(dev);
> >> > +out:
> >> > +       device_unlock(&dev->dev);
> >> >         return 0;
> >> >  }
> >> >
> >
> >



^ permalink raw reply

* Re: [BUGFIX 1/2] PCI/PM: Fix deadlock when unbind device if its parent in D3cold
From: Bjorn Helgaas @ 2012-11-03 17:22 UTC (permalink / raw)
  To: Huang Ying
  Cc: linux-kernel, linux-pci, linux-pm, Rafael J. Wysocki,
	Zhang Yanmin
In-Reply-To: <1351919165.13106.2.camel@yhuang-mobile.sh.intel.com>

On Fri, Nov 2, 2012 at 11:06 PM, Huang Ying <ying.huang@intel.com> wrote:
> On Fri, 2012-11-02 at 10:50 -0600, Bjorn Helgaas wrote:
>> On Wed, Oct 24, 2012 at 12:54 AM, Huang Ying <ying.huang@intel.com> wrote:
>> > If a PCI device and its parents are put into D3cold, unbinding the
>> > device will trigger deadlock as follow:
>> >
>> > - driver_unbind
>> >   - device_release_driver
>> >     - device_lock(dev)                          <--- previous lock here
>> >     - __device_release_driver
>> >       - pm_runtime_get_sync
>> >         ...
>> >           - rpm_resume(dev)
>> >             - rpm_resume(dev->parent)
>> >               ...
>> >                 - pci_pm_runtime_resume
>> >                   ...
>> >                   - pci_set_power_state
>> >                     - __pci_start_power_transition
>> >                       - pci_wakeup_bus(dev->parent->subordinate)
>> >                         - pci_walk_bus
>> >                           - device_lock(dev)    <--- dead lock here
>> >
>> >
>> > If we do not do device_lock in pci_walk_bus, we can avoid dead lock.
>> > Device_lock in pci_walk_bus is introduced in commit:
>> > d71374dafbba7ec3f67371d3b7e9f6310a588808, corresponding email thread
>> > is: https://lkml.org/lkml/2006/5/26/38.  The patch author Zhang Yanmin
>> > said device_lock is added to pci_walk_bus because:
>> >
>> >   Some error handling functions call pci_walk_bus. For example, PCIe
>> >   aer. Here we lock the device, so the driver wouldn't detach from the
>> >   device, as the cb might call driver's callback function.
>> >
>> > So I fixed the dead lock as follow:
>> >
>> > - remove device_lock from pci_walk_bus
>> > - add device_lock into callback if callback will call driver's callback
>> >
>> > I checked pci_walk_bus users one by one, and found only PCIe aer needs
>> > device lock.
>>
>> Is there a problem report or bugzilla for this issue?
>
> No.  I found this issue when I developed kexec fixing.
>
> Best Regards,
> Huang Ying
>
>> > Signed-off-by: Huang Ying <ying.huang@intel.com>
>> > Cc: Zhang Yanmin <yanmin.zhang@intel.com>
>>
>> Should this go to stable as well?  The D3cold support appeared in
>> v3.6, so my guess is that this fix could go to v3.6.x.

What about the stable tree?

>> > ---
>> >  drivers/pci/bus.c                  |    3 ---
>> >  drivers/pci/pcie/aer/aerdrv_core.c |   20 ++++++++++++++++----
>> >  2 files changed, 16 insertions(+), 7 deletions(-)
>> >
>> > --- a/drivers/pci/bus.c
>> > +++ b/drivers/pci/bus.c
>> > @@ -320,10 +320,7 @@ void pci_walk_bus(struct pci_bus *top, i
>> >                 } else
>> >                         next = dev->bus_list.next;
>> >
>> > -               /* Run device routines with the device locked */
>> > -               device_lock(&dev->dev);
>> >                 retval = cb(dev, userdata);
>> > -               device_unlock(&dev->dev);
>> >                 if (retval)
>> >                         break;
>> >         }
>> > --- a/drivers/pci/pcie/aer/aerdrv_core.c
>> > +++ b/drivers/pci/pcie/aer/aerdrv_core.c
>> > @@ -213,6 +213,7 @@ static int report_error_detected(struct
>> >         struct aer_broadcast_data *result_data;
>> >         result_data = (struct aer_broadcast_data *) data;
>> >
>> > +       device_lock(&dev->dev);
>> >         dev->error_state = result_data->state;
>> >
>> >         if (!dev->driver ||
>> > @@ -231,12 +232,14 @@ static int report_error_detected(struct
>> >                                    dev->driver ?
>> >                                    "no AER-aware driver" : "no driver");
>> >                 }
>> > -               return 0;
>> > +               goto out;
>> >         }
>> >
>> >         err_handler = dev->driver->err_handler;
>> >         vote = err_handler->error_detected(dev, result_data->state);
>> >         result_data->result = merge_result(result_data->result, vote);
>> > +out:
>> > +       device_unlock(&dev->dev);
>> >         return 0;
>> >  }
>> >
>> > @@ -247,14 +250,17 @@ static int report_mmio_enabled(struct pc
>> >         struct aer_broadcast_data *result_data;
>> >         result_data = (struct aer_broadcast_data *) data;
>> >
>> > +       device_lock(&dev->dev);
>> >         if (!dev->driver ||
>> >                 !dev->driver->err_handler ||
>> >                 !dev->driver->err_handler->mmio_enabled)
>> > -               return 0;
>> > +               goto out;
>> >
>> >         err_handler = dev->driver->err_handler;
>> >         vote = err_handler->mmio_enabled(dev);
>> >         result_data->result = merge_result(result_data->result, vote);
>> > +out:
>> > +       device_unlock(&dev->dev);
>> >         return 0;
>> >  }
>> >
>> > @@ -265,14 +271,17 @@ static int report_slot_reset(struct pci_
>> >         struct aer_broadcast_data *result_data;
>> >         result_data = (struct aer_broadcast_data *) data;
>> >
>> > +       device_lock(&dev->dev);
>> >         if (!dev->driver ||
>> >                 !dev->driver->err_handler ||
>> >                 !dev->driver->err_handler->slot_reset)
>> > -               return 0;
>> > +               goto out;
>> >
>> >         err_handler = dev->driver->err_handler;
>> >         vote = err_handler->slot_reset(dev);
>> >         result_data->result = merge_result(result_data->result, vote);
>> > +out:
>> > +       device_unlock(&dev->dev);
>> >         return 0;
>> >  }
>> >
>> > @@ -280,15 +289,18 @@ static int report_resume(struct pci_dev
>> >  {
>> >         const struct pci_error_handlers *err_handler;
>> >
>> > +       device_lock(&dev->dev);
>> >         dev->error_state = pci_channel_io_normal;
>> >
>> >         if (!dev->driver ||
>> >                 !dev->driver->err_handler ||
>> >                 !dev->driver->err_handler->resume)
>> > -               return 0;
>> > +               goto out;
>> >
>> >         err_handler = dev->driver->err_handler;
>> >         err_handler->resume(dev);
>> > +out:
>> > +       device_unlock(&dev->dev);
>> >         return 0;
>> >  }
>> >
>
>

^ permalink raw reply

* Re: [BUGFIX 2/2] PCI/PM: Resume device before shutdown
From: Bjorn Helgaas @ 2012-11-03 17:21 UTC (permalink / raw)
  To: Huang Ying; +Cc: linux-kernel, linux-pci, linux-pm, Rafael J. Wysocki
In-Reply-To: <1351919112.13106.1.camel@yhuang-mobile.sh.intel.com>

On Fri, Nov 2, 2012 at 11:05 PM, Huang Ying <ying.huang@intel.com> wrote:
> On Fri, 2012-11-02 at 10:52 -0600, Bjorn Helgaas wrote:
>> On Wed, Oct 24, 2012 at 12:54 AM, Huang Ying <ying.huang@intel.com> wrote:
>> > Some actions during shutdown need device to be in D0 state, such as
>> > MSI shutdown etc, so resume device before shutdown.
>>
>> Is there a problem report or bugzilla for this issue?  What are the
>> symptoms by which a user could figure out that he needs this fix?
>
> No bugzilla for this issue.  This issue will cause the corresponding
> device lost in kexeced kernel.

So would the following be accurate changelog text?

    Without this patch, a device may not work correctly after a kexec
    because the new kernel expects devices to be in D0.

>> Should this be put in the stable tree as well?  If so, for v3.6 only?

What about the stable tree?

>> > Signed-off-by: Huang Ying <ying.huang@intel.com>
>> > ---
>> >  drivers/pci/pci-driver.c |   12 ++----------
>> >  1 file changed, 2 insertions(+), 10 deletions(-)
>> >
>> > --- a/drivers/pci/pci-driver.c
>> > +++ b/drivers/pci/pci-driver.c
>> > @@ -398,6 +398,8 @@ static void pci_device_shutdown(struct d
>> >         struct pci_dev *pci_dev = to_pci_dev(dev);
>> >         struct pci_driver *drv = pci_dev->driver;
>> >
>> > +       pm_runtime_resume(dev);
>> > +
>> >         if (drv && drv->shutdown)
>> >                 drv->shutdown(pci_dev);
>> >         pci_msi_shutdown(pci_dev);
>> > @@ -408,16 +410,6 @@ static void pci_device_shutdown(struct d
>> >          * continue to do DMA
>> >          */
>> >         pci_disable_device(pci_dev);
>> > -
>> > -       /*
>> > -        * Devices may be enabled to wake up by runtime PM, but they need not
>> > -        * be supposed to wake up the system from its "power off" state (e.g.
>> > -        * ACPI S5).  Therefore disable wakeup for all devices that aren't
>> > -        * supposed to wake up the system at this point.  The state argument
>> > -        * will be ignored by pci_enable_wake().
>> > -        */
>> > -       if (!device_may_wakeup(dev))
>> > -               pci_enable_wake(pci_dev, PCI_UNKNOWN, false);
>> >  }
>> >
>> >  #ifdef CONFIG_PM
>
>

^ permalink raw reply

* Re: Linux 3.7-rc3
From: Alan Stern @ 2012-11-03 16:35 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Rafael J. Wysocki, Linux Kernel Mailing List, Linux PM list,
	Greg Kroah-Hartman
In-Reply-To: <CA+55aFx+w2hFifGTr_u76D9d_sDKxKJBDd2L4p0GVk-RXCczMA@mail.gmail.com>

On Fri, 2 Nov 2012, Linus Torvalds wrote:

> On Fri, Nov 2, 2012 at 3:43 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >
> > Well, not everything is rosy in the suspend land, though.  This is a
> > failure to freeze khubd during the second in a row attempt to suspend to
> > RAM (your current tree):
> 
> Ugh. So khubd is blocked in usb_start_wait_urb(), and apparently the
> timeout for that block is longer than the freezing timeout.
> 
> There's a comment about why khubd needs to be freezable, but I wonder
> if that whole thing isn't doing something wrong. Causing the suspend
> to fail is definitely always the wrong thing.

khubs has been a potential problem for suspend since the beginning.

The USB spec mandates timeouts of 5 seconds.  In addition, khubd does
lots of retries when errors occur (probably way too many) and checks
for freezability only when its to-do list is empty.

Under normal circumstances this isn't a problem.  Issues arise when a 
non-cooperative device is plugged in shortly before a suspend starts.

I suppose we could scatter a whole bunch of checks at spots throughout 
the device-initialization code.  This seems awkward but I can't think 
of anything better.  Does anyone have other suggestions?

Alan Stern

^ permalink raw reply

* [PATCH 9/9] cgroup_freezer: implement proper hierarchy support
From: Tejun Heo @ 2012-11-03  8:38 UTC (permalink / raw)
  To: lizefan-hv44wF8Li93QT0dZR+AlfA, mhocko-AlSwsSmVLrQ,
	rjw-KKrjLPT3xs0
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1351931915-1701-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Up until now, cgroup_freezer didn't implement hierarchy properly.
cgroups could be arranged in hierarchy but it didn't make any
difference in how each cgroup_freezer behaved.  They all operated
separately.

This patch implements proper hierarchy support.  If a cgroup is
frozen, all its descendants are frozen.  A cgroup is thawed iff it and
all its ancestors are THAWED.  freezer.self_freezing shows the current
freezing state for the cgroup itself.  freezer.parent_freezing shows
whether the cgroup is freezing because any of its ancestors is
freezing.

freezer_post_create() locks the parent and new cgroup and inherits the
parent's state and freezer_change_state() applies new state top-down
using cgroup_for_each_descendant_pre() which guarantees that no child
can escape its parent's state.  update_if_frozen() uses
cgroup_for_each_descendant_post() to propagate frozen states
bottom-up.

Synchronization could be coarser and easier by using a single mutex to
protect all hierarchy operations.  Finer grained approach was used
because it wasn't too difficult for cgroup_freezer and I think it's
beneficial to have an example implementation and cgroup_freezer is
rather simple and can serve a good one.

As this makes cgroup_freezer properly hierarchical,
freezer_subsys.broken_hierarchy marking is removed.

Note that this patch changes userland visible behavior - freezing a
cgroup now freezes all its descendants too.  This behavior change is
intended and has been warned via .broken_hierarchy.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 kernel/cgroup_freezer.c | 161 ++++++++++++++++++++++++++++++++++++------------
 1 file changed, 123 insertions(+), 38 deletions(-)

diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index 4f12d31..3262537 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -22,6 +22,13 @@
 #include <linux/freezer.h>
 #include <linux/seq_file.h>
 
+/*
+ * A cgroup is freezing if any FREEZING flags are set.  FREEZING_SELF is
+ * set if "FROZEN" is written to freezer.state cgroupfs file, and cleared
+ * for "THAWED".  FREEZING_PARENT is set if the parent freezer is FREEZING
+ * for whatever reason.  IOW, a cgroup has FREEZING_PARENT set if one of
+ * its ancestors has FREEZING_SELF set.
+ */
 enum freezer_state_flags {
 	CGROUP_FREEZER_ONLINE	= (1 << 0), /* freezer is fully online */
 	CGROUP_FREEZING_SELF	= (1 << 1), /* this freezer is freezing */
@@ -50,6 +57,15 @@ static inline struct freezer *task_freezer(struct task_struct *task)
 			    struct freezer, css);
 }
 
+static struct freezer *parent_freezer(struct freezer *freezer)
+{
+	struct cgroup *pcg = freezer->css.cgroup->parent;
+
+	if (pcg)
+		return cgroup_freezer(pcg);
+	return NULL;
+}
+
 bool cgroup_freezing(struct task_struct *task)
 {
 	bool ret;
@@ -74,17 +90,6 @@ static const char *freezer_state_strs(unsigned int state)
 	return "THAWED";
 };
 
-/*
- * State diagram
- * Transitions are caused by userspace writes to the freezer.state file.
- * The values in parenthesis are state labels. The rest are edge labels.
- *
- * (THAWED) --FROZEN--> (FREEZING) --FROZEN--> (FROZEN)
- *    ^ ^                    |                     |
- *    | \_______THAWED_______/                     |
- *    \__________________________THAWED____________/
- */
-
 struct cgroup_subsys freezer_subsys;
 
 static struct cgroup_subsys_state *freezer_create(struct cgroup *cgroup)
@@ -103,15 +108,34 @@ static struct cgroup_subsys_state *freezer_create(struct cgroup *cgroup)
  * freezer_post_create - commit creation of a freezer cgroup
  * @cgroup: cgroup being created
  *
- * We're committing to creation of @cgroup.  Mark it online.
+ * We're committing to creation of @cgroup.  Mark it online and inherit
+ * parent's freezing state while holding both parent's and our
+ * freezer->lock.
  */
 static void freezer_post_create(struct cgroup *cgroup)
 {
 	struct freezer *freezer = cgroup_freezer(cgroup);
+	struct freezer *parent = parent_freezer(freezer);
+
+	/*
+	 * The following double locking and freezing state inheritance
+	 * guarantee that @cgroup can never escape ancestors' freezing
+	 * states.  See cgroup_for_each_descendant_pre() for details.
+	 */
+	if (parent)
+		spin_lock_irq(&parent->lock);
+	spin_lock_nested(&freezer->lock, SINGLE_DEPTH_NESTING);
 
-	spin_lock_irq(&freezer->lock);
 	freezer->state |= CGROUP_FREEZER_ONLINE;
-	spin_unlock_irq(&freezer->lock);
+
+	if (parent && (parent->state & CGROUP_FREEZING)) {
+		freezer->state |= CGROUP_FREEZING_PARENT | CGROUP_FROZEN;
+		atomic_inc(&system_freezing_cnt);
+	}
+
+	spin_unlock(&freezer->lock);
+	if (parent)
+		spin_unlock_irq(&parent->lock);
 }
 
 /**
@@ -153,6 +177,7 @@ static void freezer_attach(struct cgroup *new_cgrp, struct cgroup_taskset *tset)
 {
 	struct freezer *freezer = cgroup_freezer(new_cgrp);
 	struct task_struct *task;
+	bool clear_frozen = false;
 
 	spin_lock_irq(&freezer->lock);
 
@@ -172,10 +197,25 @@ static void freezer_attach(struct cgroup *new_cgrp, struct cgroup_taskset *tset)
 		} else {
 			freeze_task(task);
 			freezer->state &= ~CGROUP_FROZEN;
+			clear_frozen = true;
 		}
 	}
 
 	spin_unlock_irq(&freezer->lock);
+
+	/*
+	 * Propagate FROZEN clearing upwards.  We may race with
+	 * update_if_frozen(), but as long as both work bottom-up, either
+	 * update_if_frozen() sees child's FROZEN cleared or we clear the
+	 * parent's FROZEN later.  No parent w/ !FROZEN children can be
+	 * left FROZEN.
+	 */
+	while (clear_frozen && (freezer = parent_freezer(freezer))) {
+		spin_lock_irq(&freezer->lock);
+		freezer->state &= ~CGROUP_FROZEN;
+		clear_frozen = freezer->state & CGROUP_FREEZING;
+		spin_unlock_irq(&freezer->lock);
+	}
 }
 
 static void freezer_fork(struct task_struct *task)
@@ -200,24 +240,47 @@ out:
 	rcu_read_unlock();
 }
 
-/*
- * We change from FREEZING to FROZEN lazily if the cgroup was only
- * partially frozen when we exitted write.  Caller must hold freezer->lock.
+/**
+ * update_if_frozen - update whether a cgroup finished freezing
+ * @cgroup: cgroup of interest
+ *
+ * Once FREEZING is initiated, transition to FROZEN is lazily updated by
+ * calling this function.  If the current state is FREEZING but not FROZEN,
+ * this function checks whether all tasks of this cgroup and the descendant
+ * cgroups finished freezing and, if so, sets FROZEN.
+ *
+ * The caller is responsible for grabbing RCU read lock and calling
+ * update_if_frozen() on all descendants prior to invoking this function.
  *
  * Task states and freezer state might disagree while tasks are being
  * migrated into or out of @cgroup, so we can't verify task states against
  * @freezer state here.  See freezer_attach() for details.
  */
-static void update_if_frozen(struct freezer *freezer)
+static void update_if_frozen(struct cgroup *cgroup)
 {
-	struct cgroup *cgroup = freezer->css.cgroup;
+	struct freezer *freezer = cgroup_freezer(cgroup);
+	struct cgroup *pos;
 	struct cgroup_iter it;
 	struct task_struct *task;
 
+	WARN_ON_ONCE(!rcu_read_lock_held());
+
+	spin_lock_irq(&freezer->lock);
+
 	if (!(freezer->state & CGROUP_FREEZING) ||
 	    (freezer->state & CGROUP_FROZEN))
-		return;
+		goto out_unlock;
+
+	/* are all (live) children frozen? */
+	cgroup_for_each_child(pos, cgroup) {
+		struct freezer *child = cgroup_freezer(pos);
 
+		if ((child->state & CGROUP_FREEZER_ONLINE) &&
+		    !(child->state & CGROUP_FROZEN))
+			goto out_unlock;
+	}
+
+	/* are all tasks frozen? */
 	cgroup_iter_start(cgroup, &it);
 
 	while ((task = cgroup_iter_next(cgroup, &it))) {
@@ -229,27 +292,32 @@ static void update_if_frozen(struct freezer *freezer)
 			 * the usual frozen condition.
 			 */
 			if (!frozen(task) && !freezer_should_skip(task))
-				goto notyet;
+				goto out_iter_end;
 		}
 	}
 
 	freezer->state |= CGROUP_FROZEN;
-notyet:
+out_iter_end:
 	cgroup_iter_end(cgroup, &it);
+out_unlock:
+	spin_unlock_irq(&freezer->lock);
 }
 
 static int freezer_read(struct cgroup *cgroup, struct cftype *cft,
 			struct seq_file *m)
 {
-	struct freezer *freezer = cgroup_freezer(cgroup);
-	unsigned int state;
+	struct cgroup *pos;
 
-	spin_lock_irq(&freezer->lock);
-	update_if_frozen(freezer);
-	state = freezer->state;
-	spin_unlock_irq(&freezer->lock);
+	rcu_read_lock();
 
-	seq_puts(m, freezer_state_strs(state));
+	/* update states bottom-up */
+	cgroup_for_each_descendant_post(pos, cgroup)
+		update_if_frozen(pos);
+	update_if_frozen(cgroup);
+
+	rcu_read_unlock();
+
+	seq_puts(m, freezer_state_strs(cgroup_freezer(cgroup)->state));
 	seq_putc(m, '\n');
 	return 0;
 }
@@ -320,14 +388,39 @@ static void freezer_apply_state(struct freezer *freezer, bool freeze,
  * @freezer: freezer of interest
  * @freeze: whether to freeze or thaw
  *
- * Freeze or thaw @cgroup according to @freeze.
+ * Freeze or thaw @freezer according to @freeze.  The operations are
+ * recursive - all descendants of @freezer will be affected.
  */
 static void freezer_change_state(struct freezer *freezer, bool freeze)
 {
+	struct cgroup *pos;
+
 	/* update @freezer */
 	spin_lock_irq(&freezer->lock);
 	freezer_apply_state(freezer, freeze, CGROUP_FREEZING_SELF);
 	spin_unlock_irq(&freezer->lock);
+
+	/*
+	 * Update all its descendants in pre-order traversal.  Each
+	 * descendant will try to inherit its parent's FREEZING state as
+	 * CGROUP_FREEZING_PARENT.
+	 */
+	rcu_read_lock();
+	cgroup_for_each_descendant_pre(pos, freezer->css.cgroup) {
+		struct freezer *pos_f = cgroup_freezer(pos);
+		struct freezer *parent = parent_freezer(freezer);
+
+		/*
+		 * Our update to @parent->state is already visible which is
+		 * all we need.  No need to lock @parent.  For more info on
+		 * synchronization, see freezer_post_create().
+		 */
+		spin_lock_irq(&pos_f->lock);
+		freezer_apply_state(pos_f, parent->state & CGROUP_FREEZING,
+				    CGROUP_FREEZING_PARENT);
+		spin_unlock_irq(&pos_f->lock);
+	}
+	rcu_read_unlock();
 }
 
 static int freezer_write(struct cgroup *cgroup, struct cftype *cft,
@@ -390,12 +483,4 @@ struct cgroup_subsys freezer_subsys = {
 	.attach		= freezer_attach,
 	.fork		= freezer_fork,
 	.base_cftypes	= files,
-
-	/*
-	 * freezer subsys doesn't handle hierarchy at all.  Frozen state
-	 * should be inherited through the hierarchy - if a parent is
-	 * frozen, all its children should be frozen.  Fix it and remove
-	 * the following.
-	 */
-	.broken_hierarchy = true,
 };
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH 8/9] cgroup_freezer: add ->post_create() and ->pre_destroy() and track online state
From: Tejun Heo @ 2012-11-03  8:38 UTC (permalink / raw)
  To: lizefan-hv44wF8Li93QT0dZR+AlfA, mhocko-AlSwsSmVLrQ,
	rjw-KKrjLPT3xs0
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1351931915-1701-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

A cgroup is online and visible to iteration between ->post_create()
and ->pre_destroy().  This patch introduces CGROUP_FREEZER_ONLINE and
toggles it from the newly added freezer_post_create() and
freezer_pre_destroy() while holding freezer->lock such that a
cgroup_freezer can be reilably distinguished to be online.  This will
be used by full hierarchy support.

ONLINE test is added to freezer_apply_state() but it currently doesn't
make any difference as freezer_write() can only be called for an
online cgroup.

Adjusting system_freezing_cnt on destruction is moved from
freezer_destroy() to the new freezer_pre_destroy() for consistency.

This patch doesn't introduce any noticeable behavior change.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 kernel/cgroup_freezer.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index b8ad93c..4f12d31 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -23,6 +23,7 @@
 #include <linux/seq_file.h>
 
 enum freezer_state_flags {
+	CGROUP_FREEZER_ONLINE	= (1 << 0), /* freezer is fully online */
 	CGROUP_FREEZING_SELF	= (1 << 1), /* this freezer is freezing */
 	CGROUP_FREEZING_PARENT	= (1 << 2), /* the parent freezer is freezing */
 	CGROUP_FROZEN		= (1 << 3), /* this and its descendants frozen */
@@ -98,13 +99,45 @@ static struct cgroup_subsys_state *freezer_create(struct cgroup *cgroup)
 	return &freezer->css;
 }
 
-static void freezer_destroy(struct cgroup *cgroup)
+/**
+ * freezer_post_create - commit creation of a freezer cgroup
+ * @cgroup: cgroup being created
+ *
+ * We're committing to creation of @cgroup.  Mark it online.
+ */
+static void freezer_post_create(struct cgroup *cgroup)
 {
 	struct freezer *freezer = cgroup_freezer(cgroup);
 
+	spin_lock_irq(&freezer->lock);
+	freezer->state |= CGROUP_FREEZER_ONLINE;
+	spin_unlock_irq(&freezer->lock);
+}
+
+/**
+ * freezer_pre_destroy - initiate destruction of @cgroup
+ * @cgroup: cgroup being destroyed
+ *
+ * @cgroup is going away.  Mark it dead and decrement system_freezing_count
+ * if it was holding one.
+ */
+static void freezer_pre_destroy(struct cgroup *cgroup)
+{
+	struct freezer *freezer = cgroup_freezer(cgroup);
+
+	spin_lock_irq(&freezer->lock);
+
 	if (freezer->state & CGROUP_FREEZING)
 		atomic_dec(&system_freezing_cnt);
-	kfree(freezer);
+
+	freezer->state = 0;
+
+	spin_unlock_irq(&freezer->lock);
+}
+
+static void freezer_destroy(struct cgroup *cgroup)
+{
+	kfree(cgroup_freezer(cgroup));
 }
 
 /*
@@ -260,6 +293,9 @@ static void freezer_apply_state(struct freezer *freezer, bool freeze,
 	/* also synchronizes against task migration, see freezer_attach() */
 	lockdep_assert_held(&freezer->lock);
 
+	if (!(freezer->state & CGROUP_FREEZER_ONLINE))
+		return;
+
 	if (freeze) {
 		if (!(freezer->state & CGROUP_FREEZING))
 			atomic_inc(&system_freezing_cnt);
@@ -347,6 +383,8 @@ static struct cftype files[] = {
 struct cgroup_subsys freezer_subsys = {
 	.name		= "freezer",
 	.create		= freezer_create,
+	.post_create	= freezer_post_create,
+	.pre_destroy	= freezer_pre_destroy,
 	.destroy	= freezer_destroy,
 	.subsys_id	= freezer_subsys_id,
 	.attach		= freezer_attach,
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH 7/9] cgroup_freezer: introduce CGROUP_FREEZING_[SELF|PARENT]
From: Tejun Heo @ 2012-11-03  8:38 UTC (permalink / raw)
  To: lizefan-hv44wF8Li93QT0dZR+AlfA, mhocko-AlSwsSmVLrQ,
	rjw-KKrjLPT3xs0
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1351931915-1701-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Introduce FREEZING_SELF and FREEZING_PARENT and make FREEZING OR of
the two flags.  This is to prepare for full hierarchy support.

freezer_apply_date() is updated such that it can handle setting and
clearing of both flags.  The two flags are also exposed to userland
via read-only files self_freezing and parent_freezing.

Other than the added cgroupfs files, this patch doesn't introduce any
behavior change.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 kernel/cgroup_freezer.c | 55 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 47 insertions(+), 8 deletions(-)

diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index e76aa9f..b8ad93c 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -23,8 +23,12 @@
 #include <linux/seq_file.h>
 
 enum freezer_state_flags {
-	CGROUP_FREEZING		= (1 << 1), /* this freezer is freezing */
+	CGROUP_FREEZING_SELF	= (1 << 1), /* this freezer is freezing */
+	CGROUP_FREEZING_PARENT	= (1 << 2), /* the parent freezer is freezing */
 	CGROUP_FROZEN		= (1 << 3), /* this and its descendants frozen */
+
+	/* mask for all FREEZING flags */
+	CGROUP_FREEZING		= CGROUP_FREEZING_SELF | CGROUP_FREEZING_PARENT,
 };
 
 struct freezer {
@@ -245,8 +249,13 @@ static void unfreeze_cgroup(struct freezer *freezer)
  * freezer_apply_state - apply state change to a single cgroup_freezer
  * @freezer: freezer to apply state change to
  * @freeze: whether to freeze or unfreeze
+ * @state: CGROUP_FREEZING_* flag to set or clear
+ *
+ * Set or clear @state on @cgroup according to @freeze, and perform
+ * freezing or thawing as necessary.
  */
-static void freezer_apply_state(struct freezer *freezer, bool freeze)
+static void freezer_apply_state(struct freezer *freezer, bool freeze,
+				unsigned int state)
 {
 	/* also synchronizes against task migration, see freezer_attach() */
 	lockdep_assert_held(&freezer->lock);
@@ -254,13 +263,19 @@ static void freezer_apply_state(struct freezer *freezer, bool freeze)
 	if (freeze) {
 		if (!(freezer->state & CGROUP_FREEZING))
 			atomic_inc(&system_freezing_cnt);
-		freezer->state |= CGROUP_FREEZING;
+		freezer->state |= state;
 		freeze_cgroup(freezer);
 	} else {
-		if (freezer->state & CGROUP_FREEZING)
-			atomic_dec(&system_freezing_cnt);
-		freezer->state &= ~(CGROUP_FREEZING | CGROUP_FROZEN);
-		unfreeze_cgroup(freezer);
+		bool was_freezing = freezer->state & CGROUP_FREEZING;
+
+		freezer->state &= ~state;
+
+		if (!(freezer->state & CGROUP_FREEZING)) {
+			if (was_freezing)
+				atomic_dec(&system_freezing_cnt);
+			freezer->state &= ~CGROUP_FROZEN;
+			unfreeze_cgroup(freezer);
+		}
 	}
 }
 
@@ -275,7 +290,7 @@ static void freezer_change_state(struct freezer *freezer, bool freeze)
 {
 	/* update @freezer */
 	spin_lock_irq(&freezer->lock);
-	freezer_apply_state(freezer, freeze);
+	freezer_apply_state(freezer, freeze, CGROUP_FREEZING_SELF);
 	spin_unlock_irq(&freezer->lock);
 }
 
@@ -295,6 +310,20 @@ static int freezer_write(struct cgroup *cgroup, struct cftype *cft,
 	return 0;
 }
 
+static u64 freezer_self_freezing_read(struct cgroup *cgroup, struct cftype *cft)
+{
+	struct freezer *freezer = cgroup_freezer(cgroup);
+
+	return (bool)(freezer->state & CGROUP_FREEZING_SELF);
+}
+
+static u64 freezer_parent_freezing_read(struct cgroup *cgroup, struct cftype *cft)
+{
+	struct freezer *freezer = cgroup_freezer(cgroup);
+
+	return (bool)(freezer->state & CGROUP_FREEZING_PARENT);
+}
+
 static struct cftype files[] = {
 	{
 		.name = "state",
@@ -302,6 +331,16 @@ static struct cftype files[] = {
 		.read_seq_string = freezer_read,
 		.write_string = freezer_write,
 	},
+	{
+		.name = "self_freezing",
+		.flags = CFTYPE_NOT_ON_ROOT,
+		.read_u64 = freezer_self_freezing_read,
+	},
+	{
+		.name = "parent_freezing",
+		.flags = CFTYPE_NOT_ON_ROOT,
+		.read_u64 = freezer_parent_freezing_read,
+	},
 	{ }	/* terminate */
 };
 
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH 6/9] cgroup_freezer: make freezer->state mask of flags
From: Tejun Heo @ 2012-11-03  8:38 UTC (permalink / raw)
  To: lizefan-hv44wF8Li93QT0dZR+AlfA, mhocko-AlSwsSmVLrQ,
	rjw-KKrjLPT3xs0
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1351931915-1701-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

freezer->state was an enum value - one of THAWED, FREEZING and FROZEN.
As the scheduled full hierarchy support requires more than one
freezing condition, switch it to mask of flags.  If FREEZING is not
set, it's thawed.  FREEZING is set if freezing or frozen.  If frozen,
both FREEZING and FROZEN are set.  Now that tasks can be attached to
an already frozen cgroup, this also makes freezing condition checks
more natural.

This patch doesn't introduce any behavior change.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 kernel/cgroup_freezer.c | 60 ++++++++++++++++++++++---------------------------
 1 file changed, 27 insertions(+), 33 deletions(-)

diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index 2690830..e76aa9f 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -22,15 +22,14 @@
 #include <linux/freezer.h>
 #include <linux/seq_file.h>
 
-enum freezer_state {
-	CGROUP_THAWED = 0,
-	CGROUP_FREEZING,
-	CGROUP_FROZEN,
+enum freezer_state_flags {
+	CGROUP_FREEZING		= (1 << 1), /* this freezer is freezing */
+	CGROUP_FROZEN		= (1 << 3), /* this and its descendants frozen */
 };
 
 struct freezer {
 	struct cgroup_subsys_state	css;
-	enum freezer_state		state;
+	unsigned int			state;
 	spinlock_t			lock;
 };
 
@@ -48,12 +47,10 @@ static inline struct freezer *task_freezer(struct task_struct *task)
 
 bool cgroup_freezing(struct task_struct *task)
 {
-	enum freezer_state state;
 	bool ret;
 
 	rcu_read_lock();
-	state = task_freezer(task)->state;
-	ret = state == CGROUP_FREEZING || state == CGROUP_FROZEN;
+	ret = task_freezer(task)->state & CGROUP_FREEZING;
 	rcu_read_unlock();
 
 	return ret;
@@ -63,10 +60,13 @@ bool cgroup_freezing(struct task_struct *task)
  * cgroups_write_string() limits the size of freezer state strings to
  * CGROUP_LOCAL_BUFFER_SIZE
  */
-static const char *freezer_state_strs[] = {
-	"THAWED",
-	"FREEZING",
-	"FROZEN",
+static const char *freezer_state_strs(unsigned int state)
+{
+	if (state & CGROUP_FROZEN)
+		return "FROZEN";
+	if (state & CGROUP_FREEZING)
+		return "FREEZING";
+	return "THAWED";
 };
 
 /*
@@ -91,7 +91,6 @@ static struct cgroup_subsys_state *freezer_create(struct cgroup *cgroup)
 		return ERR_PTR(-ENOMEM);
 
 	spin_lock_init(&freezer->lock);
-	freezer->state = CGROUP_THAWED;
 	return &freezer->css;
 }
 
@@ -99,7 +98,7 @@ static void freezer_destroy(struct cgroup *cgroup)
 {
 	struct freezer *freezer = cgroup_freezer(cgroup);
 
-	if (freezer->state != CGROUP_THAWED)
+	if (freezer->state & CGROUP_FREEZING)
 		atomic_dec(&system_freezing_cnt);
 	kfree(freezer);
 }
@@ -129,15 +128,13 @@ static void freezer_attach(struct cgroup *new_cgrp, struct cgroup_taskset *tset)
 	 * Tasks in @tset are on @new_cgrp but may not conform to its
 	 * current state before executing the following - !frozen tasks may
 	 * be visible in a FROZEN cgroup and frozen tasks in a THAWED one.
-	 * This means that, to determine whether to freeze, one should test
-	 * whether the state equals THAWED.
 	 */
 	cgroup_taskset_for_each(task, new_cgrp, tset) {
-		if (freezer->state == CGROUP_THAWED) {
+		if (!(freezer->state & CGROUP_FREEZING)) {
 			__thaw_task(task);
 		} else {
 			freeze_task(task);
-			freezer->state = CGROUP_FREEZING;
+			freezer->state &= ~CGROUP_FROZEN;
 		}
 	}
 
@@ -159,11 +156,7 @@ static void freezer_fork(struct task_struct *task)
 		goto out;
 
 	spin_lock_irq(&freezer->lock);
-	/*
-	 * @task might have been just migrated into a FROZEN cgroup.  Test
-	 * equality with THAWED.  Read the comment in freezer_attach().
-	 */
-	if (freezer->state != CGROUP_THAWED)
+	if (freezer->state & CGROUP_FREEZING)
 		freeze_task(task);
 	spin_unlock_irq(&freezer->lock);
 out:
@@ -184,7 +177,8 @@ static void update_if_frozen(struct freezer *freezer)
 	struct cgroup_iter it;
 	struct task_struct *task;
 
-	if (freezer->state != CGROUP_FREEZING)
+	if (!(freezer->state & CGROUP_FREEZING) ||
+	    (freezer->state & CGROUP_FROZEN))
 		return;
 
 	cgroup_iter_start(cgroup, &it);
@@ -202,7 +196,7 @@ static void update_if_frozen(struct freezer *freezer)
 		}
 	}
 
-	freezer->state = CGROUP_FROZEN;
+	freezer->state |= CGROUP_FROZEN;
 notyet:
 	cgroup_iter_end(cgroup, &it);
 }
@@ -211,14 +205,14 @@ static int freezer_read(struct cgroup *cgroup, struct cftype *cft,
 			struct seq_file *m)
 {
 	struct freezer *freezer = cgroup_freezer(cgroup);
-	enum freezer_state state;
+	unsigned int state;
 
 	spin_lock_irq(&freezer->lock);
 	update_if_frozen(freezer);
 	state = freezer->state;
 	spin_unlock_irq(&freezer->lock);
 
-	seq_puts(m, freezer_state_strs[state]);
+	seq_puts(m, freezer_state_strs(state));
 	seq_putc(m, '\n');
 	return 0;
 }
@@ -258,14 +252,14 @@ static void freezer_apply_state(struct freezer *freezer, bool freeze)
 	lockdep_assert_held(&freezer->lock);
 
 	if (freeze) {
-		if (freezer->state == CGROUP_THAWED)
+		if (!(freezer->state & CGROUP_FREEZING))
 			atomic_inc(&system_freezing_cnt);
-		freezer->state = CGROUP_FREEZING;
+		freezer->state |= CGROUP_FREEZING;
 		freeze_cgroup(freezer);
 	} else {
-		if (freezer->state != CGROUP_THAWED)
+		if (freezer->state & CGROUP_FREEZING)
 			atomic_dec(&system_freezing_cnt);
-		freezer->state = CGROUP_THAWED;
+		freezer->state &= ~(CGROUP_FREEZING | CGROUP_FROZEN);
 		unfreeze_cgroup(freezer);
 	}
 }
@@ -290,9 +284,9 @@ static int freezer_write(struct cgroup *cgroup, struct cftype *cft,
 {
 	bool freeze;
 
-	if (strcmp(buffer, freezer_state_strs[CGROUP_THAWED]) == 0)
+	if (strcmp(buffer, freezer_state_strs(0)) == 0)
 		freeze = false;
-	else if (strcmp(buffer, freezer_state_strs[CGROUP_FROZEN]) == 0)
+	else if (strcmp(buffer, freezer_state_strs(CGROUP_FROZEN)) == 0)
 		freeze = true;
 	else
 		return -EINVAL;
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH 5/9] cgroup_freezer: prepare freezer_change_state() for full hierarchy support
From: Tejun Heo @ 2012-11-03  8:38 UTC (permalink / raw)
  To: lizefan-hv44wF8Li93QT0dZR+AlfA, mhocko-AlSwsSmVLrQ,
	rjw-KKrjLPT3xs0
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1351931915-1701-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

* Make freezer_change_state() take bool @freeze instead of enum
  freezer_state.

* Separate out freezer_apply_state() out of freezer_change_state().
  This makes freezer_change_state() a rather silly thin wrapper.  It
  will be filled with hierarchy handling later on.

This patch doesn't introduce any behavior change.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 kernel/cgroup_freezer.c | 48 ++++++++++++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index 975b3d8..2690830 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -247,45 +247,57 @@ static void unfreeze_cgroup(struct freezer *freezer)
 	cgroup_iter_end(cgroup, &it);
 }
 
-static void freezer_change_state(struct freezer *freezer,
-				 enum freezer_state goal_state)
+/**
+ * freezer_apply_state - apply state change to a single cgroup_freezer
+ * @freezer: freezer to apply state change to
+ * @freeze: whether to freeze or unfreeze
+ */
+static void freezer_apply_state(struct freezer *freezer, bool freeze)
 {
 	/* also synchronizes against task migration, see freezer_attach() */
-	spin_lock_irq(&freezer->lock);
+	lockdep_assert_held(&freezer->lock);
 
-	switch (goal_state) {
-	case CGROUP_THAWED:
-		if (freezer->state != CGROUP_THAWED)
-			atomic_dec(&system_freezing_cnt);
-		freezer->state = CGROUP_THAWED;
-		unfreeze_cgroup(freezer);
-		break;
-	case CGROUP_FROZEN:
+	if (freeze) {
 		if (freezer->state == CGROUP_THAWED)
 			atomic_inc(&system_freezing_cnt);
 		freezer->state = CGROUP_FREEZING;
 		freeze_cgroup(freezer);
-		break;
-	default:
-		BUG();
+	} else {
+		if (freezer->state != CGROUP_THAWED)
+			atomic_dec(&system_freezing_cnt);
+		freezer->state = CGROUP_THAWED;
+		unfreeze_cgroup(freezer);
 	}
+}
 
+/**
+ * freezer_change_state - change the freezing state of a cgroup_freezer
+ * @freezer: freezer of interest
+ * @freeze: whether to freeze or thaw
+ *
+ * Freeze or thaw @cgroup according to @freeze.
+ */
+static void freezer_change_state(struct freezer *freezer, bool freeze)
+{
+	/* update @freezer */
+	spin_lock_irq(&freezer->lock);
+	freezer_apply_state(freezer, freeze);
 	spin_unlock_irq(&freezer->lock);
 }
 
 static int freezer_write(struct cgroup *cgroup, struct cftype *cft,
 			 const char *buffer)
 {
-	enum freezer_state goal_state;
+	bool freeze;
 
 	if (strcmp(buffer, freezer_state_strs[CGROUP_THAWED]) == 0)
-		goal_state = CGROUP_THAWED;
+		freeze = false;
 	else if (strcmp(buffer, freezer_state_strs[CGROUP_FROZEN]) == 0)
-		goal_state = CGROUP_FROZEN;
+		freeze = true;
 	else
 		return -EINVAL;
 
-	freezer_change_state(cgroup_freezer(cgroup), goal_state);
+	freezer_change_state(cgroup_freezer(cgroup), freeze);
 	return 0;
 }
 
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH 4/9] cgroup_freezer: trivial cleanups
From: Tejun Heo @ 2012-11-03  8:38 UTC (permalink / raw)
  To: lizefan-hv44wF8Li93QT0dZR+AlfA, mhocko-AlSwsSmVLrQ,
	rjw-KKrjLPT3xs0
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1351931915-1701-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

* Clean-up indentation and line-breaks.  Drop the invalid comment
  about freezer->lock.

* Make all internal functions take @freezer instead of both @cgroup
  and @freezer.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 kernel/cgroup_freezer.c | 41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index bedefd9..975b3d8 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -29,17 +29,15 @@ enum freezer_state {
 };
 
 struct freezer {
-	struct cgroup_subsys_state css;
-	enum freezer_state state;
-	spinlock_t lock; /* protects _writes_ to state */
+	struct cgroup_subsys_state	css;
+	enum freezer_state		state;
+	spinlock_t			lock;
 };
 
-static inline struct freezer *cgroup_freezer(
-		struct cgroup *cgroup)
+static inline struct freezer *cgroup_freezer(struct cgroup *cgroup)
 {
-	return container_of(
-		cgroup_subsys_state(cgroup, freezer_subsys_id),
-		struct freezer, css);
+	return container_of(cgroup_subsys_state(cgroup, freezer_subsys_id),
+			    struct freezer, css);
 }
 
 static inline struct freezer *task_freezer(struct task_struct *task)
@@ -180,8 +178,9 @@ out:
  * migrated into or out of @cgroup, so we can't verify task states against
  * @freezer state here.  See freezer_attach() for details.
  */
-static void update_if_frozen(struct cgroup *cgroup, struct freezer *freezer)
+static void update_if_frozen(struct freezer *freezer)
 {
+	struct cgroup *cgroup = freezer->css.cgroup;
 	struct cgroup_iter it;
 	struct task_struct *task;
 
@@ -211,12 +210,11 @@ notyet:
 static int freezer_read(struct cgroup *cgroup, struct cftype *cft,
 			struct seq_file *m)
 {
-	struct freezer *freezer;
+	struct freezer *freezer = cgroup_freezer(cgroup);
 	enum freezer_state state;
 
-	freezer = cgroup_freezer(cgroup);
 	spin_lock_irq(&freezer->lock);
-	update_if_frozen(cgroup, freezer);
+	update_if_frozen(freezer);
 	state = freezer->state;
 	spin_unlock_irq(&freezer->lock);
 
@@ -225,8 +223,9 @@ static int freezer_read(struct cgroup *cgroup, struct cftype *cft,
 	return 0;
 }
 
-static void freeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
+static void freeze_cgroup(struct freezer *freezer)
 {
+	struct cgroup *cgroup = freezer->css.cgroup;
 	struct cgroup_iter it;
 	struct task_struct *task;
 
@@ -236,8 +235,9 @@ static void freeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
 	cgroup_iter_end(cgroup, &it);
 }
 
-static void unfreeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
+static void unfreeze_cgroup(struct freezer *freezer)
 {
+	struct cgroup *cgroup = freezer->css.cgroup;
 	struct cgroup_iter it;
 	struct task_struct *task;
 
@@ -247,11 +247,9 @@ static void unfreeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
 	cgroup_iter_end(cgroup, &it);
 }
 
-static void freezer_change_state(struct cgroup *cgroup,
+static void freezer_change_state(struct freezer *freezer,
 				 enum freezer_state goal_state)
 {
-	struct freezer *freezer = cgroup_freezer(cgroup);
-
 	/* also synchronizes against task migration, see freezer_attach() */
 	spin_lock_irq(&freezer->lock);
 
@@ -260,13 +258,13 @@ static void freezer_change_state(struct cgroup *cgroup,
 		if (freezer->state != CGROUP_THAWED)
 			atomic_dec(&system_freezing_cnt);
 		freezer->state = CGROUP_THAWED;
-		unfreeze_cgroup(cgroup, freezer);
+		unfreeze_cgroup(freezer);
 		break;
 	case CGROUP_FROZEN:
 		if (freezer->state == CGROUP_THAWED)
 			atomic_inc(&system_freezing_cnt);
 		freezer->state = CGROUP_FREEZING;
-		freeze_cgroup(cgroup, freezer);
+		freeze_cgroup(freezer);
 		break;
 	default:
 		BUG();
@@ -275,8 +273,7 @@ static void freezer_change_state(struct cgroup *cgroup,
 	spin_unlock_irq(&freezer->lock);
 }
 
-static int freezer_write(struct cgroup *cgroup,
-			 struct cftype *cft,
+static int freezer_write(struct cgroup *cgroup, struct cftype *cft,
 			 const char *buffer)
 {
 	enum freezer_state goal_state;
@@ -288,7 +285,7 @@ static int freezer_write(struct cgroup *cgroup,
 	else
 		return -EINVAL;
 
-	freezer_change_state(cgroup, goal_state);
+	freezer_change_state(cgroup_freezer(cgroup), goal_state);
 	return 0;
 }
 
-- 
1.7.11.7

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox