Exit ACPI drivers immediately if acpi is disabled

PATCH Thomas Renninger 3 participants 8 messages Apr 21, 2006 – Apr 24, 2006

Subsystems (2)

ACPI ACPI thermal driver

lore

Share & tools

lore

Thomas Renninger

author

2006-04-21 10:43 UTC #0 lore raw

I could not resist to also remove the ACPI function trace from init/exit functions... This has been done already recently IIRC, please tell me if it's in your test tree or whether I should just do the changes on top of the function trace removal patch.

Subject: Exit ACPI modules if acpi is disabled (via acpi=off)

If booting with acpi=off and loading the processor module, it may happen that the kernel oopses when trying to unload the module again. Better exit all ACPI modules immediately if booted with acpi=off.

 drivers/acpi/ac.c              |   13 +++++--------
 drivers/acpi/acpi_memhotplug.c |   13 +++++--------
 drivers/acpi/battery.c         |   13 +++++--------
 drivers/acpi/button.c          |   12 +++++-------
 drivers/acpi/container.c       |    3 +++
 drivers/acpi/fan.c             |   11 +++++------
 drivers/acpi/processor_core.c  |   13 +++++--------
 drivers/acpi/sony_acpi.c       |    3 +++
 drivers/acpi/thermal.c         |   13 ++++++-------
 drivers/acpi/video.c           |   15 ++++++---------
 10 files changed, 48 insertions(+), 61 deletions(-)

Signed-off-by Thomas Renninger <[email protected]>

drivers/acpi/ac.c +5 -8

Index: linux-2.6.16/drivers/acpi/ac.c
===================================================================
@@ -289,31 +289,28 @@ static int __init acpi_ac_init(void)
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_ac_init");
+	if (acpi_disabled)
+		return -ENODEV;
 
 	acpi_ac_dir = proc_mkdir(ACPI_AC_CLASS, acpi_root_dir);
 	if (!acpi_ac_dir)
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	acpi_ac_dir->owner = THIS_MODULE;
 
 	result = acpi_bus_register_driver(&acpi_ac_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_AC_CLASS, acpi_root_dir);
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	}
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_ac_exit(void)
 {
-	ACPI_FUNCTION_TRACE("acpi_ac_exit");
-
 	acpi_bus_unregister_driver(&acpi_ac_driver);
 
 	remove_proc_entry(ACPI_AC_CLASS, acpi_root_dir);
-
-	return_VOID;
 }
 
 module_init(acpi_ac_init);

drivers/acpi/acpi_memhotplug.c +5 -8

Index: linux-2.6.16/drivers/acpi/acpi_memhotplug.c
===================================================================
@@ -489,12 +489,13 @@ static int __init acpi_memory_device_ini
 	int result;
 	acpi_status status;
 
-	ACPI_FUNCTION_TRACE("acpi_memory_device_init");
+	if (acpi_disabled)
+		return -ENODEV;
 
 	result = acpi_bus_register_driver(&acpi_memory_device_driver);
 
 	if (result < 0)
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
 				     ACPI_UINT32_MAX,

@@ -504,18 +505,16 @@ static int __init acpi_memory_device_ini
 	if (ACPI_FAILURE(status)) {
 		ACPI_EXCEPTION((AE_INFO, status, "walk_namespace failed"));
 		acpi_bus_unregister_driver(&acpi_memory_device_driver);
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	}
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_memory_device_exit(void)
 {
 	acpi_status status;
 
-	ACPI_FUNCTION_TRACE("acpi_memory_device_exit");
-
 	/*
 	 * Adding this to un-install notification handlers for all the device
 	 * handles.

@@ -529,8 +528,6 @@ static void __exit acpi_memory_device_ex
 		ACPI_EXCEPTION((AE_INFO, status, "walk_namespace failed"));
 
 	acpi_bus_unregister_driver(&acpi_memory_device_driver);
-
-	return_VOID;
 }
 
 module_init(acpi_memory_device_init);

drivers/acpi/battery.c +5 -8

Index: linux-2.6.16/drivers/acpi/battery.c
===================================================================
@@ -768,31 +768,28 @@ static int __init acpi_battery_init(void
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_battery_init");
+	if (acpi_disabled)
+		return -ENODEV;
 
 	acpi_battery_dir = proc_mkdir(ACPI_BATTERY_CLASS, acpi_root_dir);
 	if (!acpi_battery_dir)
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	acpi_battery_dir->owner = THIS_MODULE;
 
 	result = acpi_bus_register_driver(&acpi_battery_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_BATTERY_CLASS, acpi_root_dir);
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	}
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_battery_exit(void)
 {
-	ACPI_FUNCTION_TRACE("acpi_battery_exit");
-
 	acpi_bus_unregister_driver(&acpi_battery_driver);
 
 	remove_proc_entry(ACPI_BATTERY_CLASS, acpi_root_dir);
-
-	return_VOID;
 }
 
 module_init(acpi_battery_init);

drivers/acpi/button.c +5 -7

Index: linux-2.6.16/drivers/acpi/button.c
===================================================================
@@ -445,25 +445,24 @@ static int __init acpi_button_init(void)
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_button_init");
+	if (acpi_disabled)
+		return -ENODEV;
 
 	acpi_button_dir = proc_mkdir(ACPI_BUTTON_CLASS, acpi_root_dir);
 	if (!acpi_button_dir)
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	acpi_button_dir->owner = THIS_MODULE;
 	result = acpi_bus_register_driver(&acpi_button_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_BUTTON_CLASS, acpi_root_dir);
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	}
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_button_exit(void)
 {
-	ACPI_FUNCTION_TRACE("acpi_button_exit");
-
 	acpi_bus_unregister_driver(&acpi_button_driver);
 
 	if (acpi_power_dir)

@@ -474,7 +473,6 @@ static void __exit acpi_button_exit(void
 		remove_proc_entry(ACPI_BUTTON_SUBCLASS_LID, acpi_button_dir);
 	remove_proc_entry(ACPI_BUTTON_CLASS, acpi_root_dir);
 
-	return_VOID;
 }
 
 module_init(acpi_button_init);

drivers/acpi/container.c +3 -0

Index: linux-2.6.16/drivers/acpi/container.c
===================================================================
@@ -251,6 +251,9 @@ static int __init acpi_container_init(vo
 	int result = 0;
 	int action = INSTALL_NOTIFY_HANDLER;
 
+	if (acpi_disabled)
+		return -ENODEV;
+
 	result = acpi_bus_register_driver(&acpi_container_driver);
 	if (result < 0) {
 		return (result);

drivers/acpi/fan.c +5 -6

Index: linux-2.6.16/drivers/acpi/fan.c
===================================================================
@@ -240,26 +240,25 @@ static int __init acpi_fan_init(void)
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_fan_init");
+	if (acpi_disabled)
+		return -ENODEV;
 
 	acpi_fan_dir = proc_mkdir(ACPI_FAN_CLASS, acpi_root_dir);
 	if (!acpi_fan_dir)
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	acpi_fan_dir->owner = THIS_MODULE;
 
 	result = acpi_bus_register_driver(&acpi_fan_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_FAN_CLASS, acpi_root_dir);
-		return_VALUE(-ENODEV);
+		return ENODEV;
 	}
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_fan_exit(void)
 {
-	ACPI_FUNCTION_TRACE("acpi_fan_exit");
-
 	acpi_bus_unregister_driver(&acpi_fan_driver);
 
 	remove_proc_entry(ACPI_FAN_CLASS, acpi_root_dir);

drivers/acpi/processor_core.c +5 -8

Index: linux-2.6.16/drivers/acpi/processor_core.c
===================================================================
@@ -910,20 +910,21 @@ static int __init acpi_processor_init(vo
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_processor_init");
+	if (acpi_disabled)
+		return -ENODEV;
 
 	memset(&processors, 0, sizeof(processors));
 	memset(&errata, 0, sizeof(errata));
 
 	acpi_processor_dir = proc_mkdir(ACPI_PROCESSOR_CLASS, acpi_root_dir);
 	if (!acpi_processor_dir)
-		return_VALUE(0);
+		return 0;
 	acpi_processor_dir->owner = THIS_MODULE;
 
 	result = acpi_bus_register_driver(&acpi_processor_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_PROCESSOR_CLASS, acpi_root_dir);
-		return_VALUE(0);
+		return 0;
 	}
 
 	acpi_processor_install_hotplug_notify();

@@ -932,13 +933,11 @@ static int __init acpi_processor_init(vo
 
 	acpi_processor_ppc_init();
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_processor_exit(void)
 {
-	ACPI_FUNCTION_TRACE("acpi_processor_exit");
-
 	acpi_processor_ppc_exit();
 
 	acpi_thermal_cpufreq_exit();

@@ -948,8 +947,6 @@ static void __exit acpi_processor_exit(v
 	acpi_bus_unregister_driver(&acpi_processor_driver);
 
 	remove_proc_entry(ACPI_PROCESSOR_CLASS, acpi_root_dir);
-
-	return_VOID;
 }
 
 module_init(acpi_processor_init);

drivers/acpi/sony_acpi.c +3 -0

Index: linux-2.6.16/drivers/acpi/sony_acpi.c
===================================================================
@@ -366,6 +366,9 @@ static int __init sony_acpi_init(void)
 {
 	int result;
 
+	if (acpi_disabled)
+		return -ENODEV;
+
 	sony_acpi_dir = proc_mkdir("sony", acpi_root_dir);
 	if (!sony_acpi_dir) {
 		printk(LOG_PFX "unable to create /proc entry\n");

drivers/acpi/thermal.c +6 -7

Index: linux-2.6.16/drivers/acpi/thermal.c
===================================================================
@@ -1400,20 +1400,21 @@ static int __init acpi_thermal_init(void
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_thermal_init");
-
+	if (acpi_disabled)
+		return -ENODEV;
+		
 	acpi_thermal_dir = proc_mkdir(ACPI_THERMAL_CLASS, acpi_root_dir);
 	if (!acpi_thermal_dir)
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	acpi_thermal_dir->owner = THIS_MODULE;
 
 	result = acpi_bus_register_driver(&acpi_thermal_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_THERMAL_CLASS, acpi_root_dir);
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	}
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_thermal_exit(void)

@@ -1423,8 +1424,6 @@ static void __exit acpi_thermal_exit(voi
 	acpi_bus_unregister_driver(&acpi_thermal_driver);
 
 	remove_proc_entry(ACPI_THERMAL_CLASS, acpi_root_dir);
-
-	return_VOID;
 }
 
 module_init(acpi_thermal_init);

drivers/acpi/video.c +6 -9

Index: linux-2.6.16/drivers/acpi/video.c
===================================================================
@@ -1850,36 +1850,33 @@ static int __init acpi_video_init(void)
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_video_init");
-
 	/*
 	   acpi_dbg_level = 0xFFFFFFFF;
 	   acpi_dbg_layer = 0x08000000;
 	 */
 
+	if (acpi_disabled)
+		return -ENODEV;
+
 	acpi_video_dir = proc_mkdir(ACPI_VIDEO_CLASS, acpi_root_dir);
 	if (!acpi_video_dir)
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	acpi_video_dir->owner = THIS_MODULE;
 
 	result = acpi_bus_register_driver(&acpi_video_bus);
 	if (result < 0) {
 		remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	}
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_video_exit(void)
 {
-	ACPI_FUNCTION_TRACE("acpi_video_exit");
-
 	acpi_bus_unregister_driver(&acpi_video_bus);
 
 	remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
-
-	return_VOID;
 }
 
 module_init(acpi_video_init);

Subject: Exit ACPI modules if acpi is disabled (via acpi=off)

 drivers/acpi/ac.c              |   13 +++++--------
 drivers/acpi/acpi_memhotplug.c |   13 +++++--------
 drivers/acpi/battery.c         |   13 +++++--------
 drivers/acpi/button.c          |   12 +++++-------
 drivers/acpi/container.c       |    3 +++
 drivers/acpi/fan.c             |   11 +++++------
 drivers/acpi/processor_core.c  |   13 +++++--------
 drivers/acpi/sony_acpi.c       |    3 +++
 drivers/acpi/thermal.c         |   13 ++++++-------
 drivers/acpi/video.c           |   15 ++++++---------
 10 files changed, 48 insertions(+), 61 deletions(-)

Signed-off-by Thomas Renninger <[email protected]>

drivers/acpi/ac.c +5 -8

Index: linux-2.6.16/drivers/acpi/ac.c
===================================================================
@@ -289,31 +289,28 @@ static int __init acpi_ac_init(void)
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_ac_init");
+	if (acpi_disabled)
+		return -ENODEV;
 
 	acpi_ac_dir = proc_mkdir(ACPI_AC_CLASS, acpi_root_dir);
 	if (!acpi_ac_dir)
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	acpi_ac_dir->owner = THIS_MODULE;
 
 	result = acpi_bus_register_driver(&acpi_ac_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_AC_CLASS, acpi_root_dir);
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	}
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_ac_exit(void)
 {
-	ACPI_FUNCTION_TRACE("acpi_ac_exit");
-
 	acpi_bus_unregister_driver(&acpi_ac_driver);
 
 	remove_proc_entry(ACPI_AC_CLASS, acpi_root_dir);
-
-	return_VOID;
 }
 
 module_init(acpi_ac_init);

drivers/acpi/acpi_memhotplug.c +5 -8

Index: linux-2.6.16/drivers/acpi/acpi_memhotplug.c
===================================================================
@@ -489,12 +489,13 @@ static int __init acpi_memory_device_ini
 	int result;
 	acpi_status status;
 
-	ACPI_FUNCTION_TRACE("acpi_memory_device_init");
+	if (acpi_disabled)
+		return -ENODEV;
 
 	result = acpi_bus_register_driver(&acpi_memory_device_driver);
 
 	if (result < 0)
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
 				     ACPI_UINT32_MAX,

@@ -504,18 +505,16 @@ static int __init acpi_memory_device_ini
 	if (ACPI_FAILURE(status)) {
 		ACPI_EXCEPTION((AE_INFO, status, "walk_namespace failed"));
 		acpi_bus_unregister_driver(&acpi_memory_device_driver);
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	}
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_memory_device_exit(void)
 {
 	acpi_status status;
 
-	ACPI_FUNCTION_TRACE("acpi_memory_device_exit");
-
 	/*
 	 * Adding this to un-install notification handlers for all the device
 	 * handles.

@@ -529,8 +528,6 @@ static void __exit acpi_memory_device_ex
 		ACPI_EXCEPTION((AE_INFO, status, "walk_namespace failed"));
 
 	acpi_bus_unregister_driver(&acpi_memory_device_driver);
-
-	return_VOID;
 }
 
 module_init(acpi_memory_device_init);

drivers/acpi/battery.c +5 -8

Index: linux-2.6.16/drivers/acpi/battery.c
===================================================================
@@ -768,31 +768,28 @@ static int __init acpi_battery_init(void
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_battery_init");
+	if (acpi_disabled)
+		return -ENODEV;
 
 	acpi_battery_dir = proc_mkdir(ACPI_BATTERY_CLASS, acpi_root_dir);
 	if (!acpi_battery_dir)
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	acpi_battery_dir->owner = THIS_MODULE;
 
 	result = acpi_bus_register_driver(&acpi_battery_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_BATTERY_CLASS, acpi_root_dir);
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	}
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_battery_exit(void)
 {
-	ACPI_FUNCTION_TRACE("acpi_battery_exit");
-
 	acpi_bus_unregister_driver(&acpi_battery_driver);
 
 	remove_proc_entry(ACPI_BATTERY_CLASS, acpi_root_dir);
-
-	return_VOID;
 }
 
 module_init(acpi_battery_init);

drivers/acpi/button.c +5 -7

Index: linux-2.6.16/drivers/acpi/button.c
===================================================================
@@ -445,25 +445,24 @@ static int __init acpi_button_init(void)
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_button_init");
+	if (acpi_disabled)
+		return -ENODEV;
 
 	acpi_button_dir = proc_mkdir(ACPI_BUTTON_CLASS, acpi_root_dir);
 	if (!acpi_button_dir)
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	acpi_button_dir->owner = THIS_MODULE;
 	result = acpi_bus_register_driver(&acpi_button_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_BUTTON_CLASS, acpi_root_dir);
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	}
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_button_exit(void)
 {
-	ACPI_FUNCTION_TRACE("acpi_button_exit");
-
 	acpi_bus_unregister_driver(&acpi_button_driver);
 
 	if (acpi_power_dir)

@@ -474,7 +473,6 @@ static void __exit acpi_button_exit(void
 		remove_proc_entry(ACPI_BUTTON_SUBCLASS_LID, acpi_button_dir);
 	remove_proc_entry(ACPI_BUTTON_CLASS, acpi_root_dir);
 
-	return_VOID;
 }
 
 module_init(acpi_button_init);

drivers/acpi/container.c +3 -0

Index: linux-2.6.16/drivers/acpi/container.c
===================================================================
@@ -251,6 +251,9 @@ static int __init acpi_container_init(vo
 	int result = 0;
 	int action = INSTALL_NOTIFY_HANDLER;
 
+	if (acpi_disabled)
+		return -ENODEV;
+
 	result = acpi_bus_register_driver(&acpi_container_driver);
 	if (result < 0) {
 		return (result);

drivers/acpi/fan.c +5 -6

Index: linux-2.6.16/drivers/acpi/fan.c
===================================================================
@@ -240,26 +240,25 @@ static int __init acpi_fan_init(void)
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_fan_init");
+	if (acpi_disabled)
+		return -ENODEV;
 
 	acpi_fan_dir = proc_mkdir(ACPI_FAN_CLASS, acpi_root_dir);
 	if (!acpi_fan_dir)
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	acpi_fan_dir->owner = THIS_MODULE;
 
 	result = acpi_bus_register_driver(&acpi_fan_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_FAN_CLASS, acpi_root_dir);
-		return_VALUE(-ENODEV);
+		return ENODEV;
 	}
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_fan_exit(void)
 {
-	ACPI_FUNCTION_TRACE("acpi_fan_exit");
-
 	acpi_bus_unregister_driver(&acpi_fan_driver);
 
 	remove_proc_entry(ACPI_FAN_CLASS, acpi_root_dir);

drivers/acpi/processor_core.c +5 -8

Index: linux-2.6.16/drivers/acpi/processor_core.c
===================================================================
@@ -910,20 +910,21 @@ static int __init acpi_processor_init(vo
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_processor_init");
+	if (acpi_disabled)
+		return -ENODEV;
 
 	memset(&processors, 0, sizeof(processors));
 	memset(&errata, 0, sizeof(errata));
 
 	acpi_processor_dir = proc_mkdir(ACPI_PROCESSOR_CLASS, acpi_root_dir);
 	if (!acpi_processor_dir)
-		return_VALUE(0);
+		return 0;
 	acpi_processor_dir->owner = THIS_MODULE;
 
 	result = acpi_bus_register_driver(&acpi_processor_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_PROCESSOR_CLASS, acpi_root_dir);
-		return_VALUE(0);
+		return 0;
 	}
 
 	acpi_processor_install_hotplug_notify();

@@ -932,13 +933,11 @@ static int __init acpi_processor_init(vo
 
 	acpi_processor_ppc_init();
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_processor_exit(void)
 {
-	ACPI_FUNCTION_TRACE("acpi_processor_exit");
-
 	acpi_processor_ppc_exit();
 
 	acpi_thermal_cpufreq_exit();

@@ -948,8 +947,6 @@ static void __exit acpi_processor_exit(v
 	acpi_bus_unregister_driver(&acpi_processor_driver);
 
 	remove_proc_entry(ACPI_PROCESSOR_CLASS, acpi_root_dir);
-
-	return_VOID;
 }
 
 module_init(acpi_processor_init);

drivers/acpi/sony_acpi.c +3 -0

Index: linux-2.6.16/drivers/acpi/sony_acpi.c
===================================================================
@@ -366,6 +366,9 @@ static int __init sony_acpi_init(void)
 {
 	int result;
 
+	if (acpi_disabled)
+		return -ENODEV;
+
 	sony_acpi_dir = proc_mkdir("sony", acpi_root_dir);
 	if (!sony_acpi_dir) {
 		printk(LOG_PFX "unable to create /proc entry\n");

drivers/acpi/thermal.c +6 -7

Index: linux-2.6.16/drivers/acpi/thermal.c
===================================================================
@@ -1400,20 +1400,21 @@ static int __init acpi_thermal_init(void
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_thermal_init");
-
+	if (acpi_disabled)
+		return -ENODEV;
+		
 	acpi_thermal_dir = proc_mkdir(ACPI_THERMAL_CLASS, acpi_root_dir);
 	if (!acpi_thermal_dir)
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	acpi_thermal_dir->owner = THIS_MODULE;
 
 	result = acpi_bus_register_driver(&acpi_thermal_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_THERMAL_CLASS, acpi_root_dir);
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	}
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_thermal_exit(void)

@@ -1423,8 +1424,6 @@ static void __exit acpi_thermal_exit(voi
 	acpi_bus_unregister_driver(&acpi_thermal_driver);
 
 	remove_proc_entry(ACPI_THERMAL_CLASS, acpi_root_dir);
-
-	return_VOID;
 }
 
 module_init(acpi_thermal_init);

drivers/acpi/video.c +6 -9

Index: linux-2.6.16/drivers/acpi/video.c
===================================================================
@@ -1850,36 +1850,33 @@ static int __init acpi_video_init(void)
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_video_init");
-
 	/*
 	   acpi_dbg_level = 0xFFFFFFFF;
 	   acpi_dbg_layer = 0x08000000;
 	 */
 
+	if (acpi_disabled)
+		return -ENODEV;
+
 	acpi_video_dir = proc_mkdir(ACPI_VIDEO_CLASS, acpi_root_dir);
 	if (!acpi_video_dir)
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	acpi_video_dir->owner = THIS_MODULE;
 
 	result = acpi_bus_register_driver(&acpi_video_bus);
 	if (result < 0) {
 		remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
-		return_VALUE(-ENODEV);
+		return -ENODEV;
 	}
 
-	return_VALUE(0);
+	return 0;
 }
 
 static void __exit acpi_video_exit(void)
 {
-	ACPI_FUNCTION_TRACE("acpi_video_exit");
-
 	acpi_bus_unregister_driver(&acpi_video_bus);
 
 	remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
-
-	return_VOID;
 }
 
 module_init(acpi_video_init);

Bjorn Helgaas

2006-04-21 15:18 UTC #1 lore raw

drivers/acpi/ac.c static int __init acpi_ac_init(void) · line 293

Quoted 11 lines · Thomas Renninger

If booting with acpi=off and loading the processor module, it may happen that the kernel oopses when trying to unload the module again. Better exit all ACPI modules immediately if booted with acpi=off.
...
drivers/acpi/ac.c +2 -1
@@ -289,31 +289,28 @@ static int __init acpi_ac_init(void)
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_ac_init");
+	if (acpi_disabled)
+		return -ENODEV;

This doesn't feel like the right solution to me. acpi_bus_register_driver() already returns -ENODEV if acpi_disabled, and that ought to be enough so the drivers can handle this correctly.

I don't think we should clutter every ACPI driver with the same check for acpi_disabled.

Did you investigate the oops when unloading the processor module? I wonder if that driver has a bug there. It has a mysterious comment that "we keep the driver loaded even when ACPI is not
running" for powernow-k8. I don't understand that at all, but
it sounds like it requires extra care to make sure everything works right.

Thomas Renninger

author

2006-04-21 16:53 UTC #2 lore raw

Quoted 18 lines · Bjorn Helgaas

Review static int __init acpi_ac_init(void) · line 293
Quoted 11 lines · Thomas Renninger
If booting with acpi=off and loading the processor module, it may happen that the kernel oopses when trying to unload the module again. Better exit all ACPI modules immediately if booted with acpi=off.
...
(quoted diff) +2 -1
@@ -289,31 +289,28 @@ static int __init acpi_ac_init(void)
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_ac_init");
+	if (acpi_disabled)
+		return -ENODEV;
This doesn't feel like the right solution to me. acpi_bus_register_driver() already returns -ENODEV if acpi_disabled, and that ought to be enough so the drivers can handle this correctly.

Quoted 2 lines

I don't think we should clutter every ACPI driver with the same check for acpi_disabled.

Yes, you are right.

Quoted 2 lines

Did you investigate the oops when unloading the processor module?

It's in unregister_driver.

Quoted 5 lines

I wonder if that driver has a bug there. It has a mysterious comment that "we keep the driver loaded even when ACPI is not
running" for powernow-k8. I don't understand that at all, but
it sounds like it requires extra care to make sure everything works right.

AFAIK processor module has no functions if acpi=off. Some cpufreq drivers still work. The ones which have the steps for each CPU model compiled in and that should still work.

There should be no functions missing when booting acpi=off and processor module cannot be loaded.
But I might be wrong here...

Hmm, as even proc is deleted, the module probably should not be loaded...

I think this one should go in then:

Subject: Exit ACPI processor module if acpi is disabled

 drivers/acpi/processor_core.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Signed-off-by Thomas Renninger <[email protected]>

drivers/acpi/processor_core.c +1 -1

Index: linux-2.6.16/drivers/acpi/processor_core.c
===================================================================
@@ -923,7 +923,7 @@ static int __init acpi_processor_init(vo
 	result = acpi_bus_register_driver(&acpi_processor_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_PROCESSOR_CLASS, acpi_root_dir);
-		return_VALUE(0);
+		return_VALUE(-EINVAL);
 	}
 
 	acpi_processor_install_hotplug_notify();

Subject: Exit ACPI processor module if acpi is disabled

 drivers/acpi/processor_core.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Signed-off-by Thomas Renninger <[email protected]>

drivers/acpi/processor_core.c +1 -1

Index: linux-2.6.16/drivers/acpi/processor_core.c
===================================================================
@@ -923,7 +923,7 @@ static int __init acpi_processor_init(vo
 	result = acpi_bus_register_driver(&acpi_processor_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_PROCESSOR_CLASS, acpi_root_dir);
-		return_VALUE(0);
+		return_VALUE(-EINVAL);
 	}
 
 	acpi_processor_install_hotplug_notify();

Dominik Brodowski

2006-04-21 16:58 UTC #3 lore raw

Quoted 38 lines · Thomas Renninger

Quoted 18 lines · Bjorn Helgaas
Review static int __init acpi_ac_init(void) · line 293
Quoted 11 lines · Thomas Renninger
If booting with acpi=off and loading the processor module, it may happen that the kernel oopses when trying to unload the module again. Better exit all ACPI modules immediately if booted with acpi=off.
...
(quoted diff) +2 -1
@@ -289,31 +289,28 @@ static int __init acpi_ac_init(void)
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_ac_init");
+	if (acpi_disabled)
+		return -ENODEV;
This doesn't feel like the right solution to me. acpi_bus_register_driver() already returns -ENODEV if acpi_disabled, and that ought to be enough so the drivers can handle this correctly.
Quoted 2 lines
I don't think we should clutter every ACPI driver with the same check for acpi_disabled.
Yes, you are right.
Quoted 2 lines
Did you investigate the oops when unloading the processor module?
It's in unregister_driver.
Quoted 5 lines
I wonder if that driver has a bug there. It has a mysterious comment that "we keep the driver loaded even when ACPI is not
running" for powernow-k8. I don't understand that at all, but
it sounds like it requires extra care to make sure everything works right.
AFAIK processor module has no functions if acpi=off. Some cpufreq drivers still work. The ones which have the steps for each CPU model compiled in and that should still work.
There should be no functions missing when booting acpi=off and processor module cannot be loaded.
But I might be wrong here...

Quoted 5 lines

Hmm, as even proc is deleted, the module probably should not be loaded...
I think this one should go in then:
Subject: Exit ACPI processor module if acpi is disabled

No. For example powernow-k8 and speedstep-centrino do have a dependency on a function which is exported by processor_perflib.c. THis means you cannot load these modules (which can run without ACPI support) if loading the processor module fails. Therefore its init function must not fail.

Dominik

Thomas Renninger

author

2006-04-21 17:18 UTC #4 lore raw

In reply to Thomas Renninger

Quoted 19 lines · Thomas Renninger

Quoted 18 lines · Bjorn Helgaas
Review static int __init acpi_ac_init(void) · line 293
Quoted 11 lines · Thomas Renninger
If booting with acpi=off and loading the processor module, it may happen that the kernel oopses when trying to unload the module again. Better exit all ACPI modules immediately if booted with acpi=off.
...
(quoted diff) +2 -1
@@ -289,31 +289,28 @@ static int __init acpi_ac_init(void)
 {
 	int result = 0;
 
-	ACPI_FUNCTION_TRACE("acpi_ac_init");
+	if (acpi_disabled)
+		return -ENODEV;
This doesn't feel like the right solution to me. acpi_bus_register_driver() already returns -ENODEV if acpi_disabled, and that ought to be enough so the drivers can handle this correctly.

Quoted 9 lines

Quoted 2 lines
I don't think we should clutter every ACPI driver with the same check for acpi_disabled.
Yes, you are right.
Quoted 2 lines
Did you investigate the oops when unloading the processor module?
It's in unregister_driver.
Quoted 3 lines
I wonder if that driver has a bug there. It has a mysterious comment that "we keep the driver loaded even when ACPI is not running" for powernow-k8.

Arggh, I didn't see this.
Next try:

Subject: Exit ACPI processor module gracefully if acpi is disabled

Signed-off-by Thomas Renninger <[email protected]>

 drivers/acpi/processor_core.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletion(-)

drivers/acpi/processor_core.c +6 -1

Index: linux-2.6.16/drivers/acpi/processor_core.c
===================================================================
@@ -906,6 +906,8 @@ void acpi_processor_uninstall_hotplug_no
  * ACPI, but needs symbols from this driver
  */
 
+static int driver_registered = 0;
+
 static int __init acpi_processor_init(void)
 {
 	int result = 0;

@@ -926,6 +928,8 @@ static int __init acpi_processor_init(vo
 		return_VALUE(0);
 	}
 
+	driver_registered = 1;
+
 	acpi_processor_install_hotplug_notify();
 
 	acpi_thermal_cpufreq_init();

@@ -943,12 +947,13 @@ static void __exit acpi_processor_exit(v
 
 	acpi_thermal_cpufreq_exit();
 
+	if (driver_registered){
 	acpi_processor_uninstall_hotplug_notify();
 
 	acpi_bus_unregister_driver(&acpi_processor_driver);
 
 	remove_proc_entry(ACPI_PROCESSOR_CLASS, acpi_root_dir);
-
+	}
 	return_VOID;
 }

Subject: Exit ACPI processor module gracefully if acpi is disabled

Signed-off-by Thomas Renninger <[email protected]>

 drivers/acpi/processor_core.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletion(-)

drivers/acpi/processor_core.c +6 -1

Index: linux-2.6.16/drivers/acpi/processor_core.c
===================================================================
@@ -906,6 +906,8 @@ void acpi_processor_uninstall_hotplug_no
  * ACPI, but needs symbols from this driver
  */
 
+static int driver_registered = 0;
+
 static int __init acpi_processor_init(void)
 {
 	int result = 0;

@@ -926,6 +928,8 @@ static int __init acpi_processor_init(vo
 		return_VALUE(0);
 	}
 
+	driver_registered = 1;
+
 	acpi_processor_install_hotplug_notify();
 
 	acpi_thermal_cpufreq_init();

@@ -943,12 +947,13 @@ static void __exit acpi_processor_exit(v
 
 	acpi_thermal_cpufreq_exit();
 
+	if (driver_registered){
 	acpi_processor_uninstall_hotplug_notify();
 
 	acpi_bus_unregister_driver(&acpi_processor_driver);
 
 	remove_proc_entry(ACPI_PROCESSOR_CLASS, acpi_root_dir);
-
+	}
 	return_VOID;
 }

Thomas Renninger

author

2006-04-24 10:52 UTC #5 lore raw

Rethinking about this: driver_registered is a bit too general variable?

Is this OK, now?

Subject: Exit ACPI processor module gracefully if acpi is disabled

Signed-off-by Thomas Renninger <[email protected]>

 drivers/acpi/processor_core.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletion(-)

drivers/acpi/processor_core.c +6 -1

Index: linux-2.6.16/drivers/acpi/processor_core.c
===================================================================
@@ -906,6 +906,8 @@ void acpi_processor_uninstall_hotplug_no
  * ACPI, but needs symbols from this driver
  */
 
+static int processor_driver_registered = 0;
+
 static int __init acpi_processor_init(void)
 {
 	int result = 0;

@@ -926,6 +928,8 @@ static int __init acpi_processor_init(vo
 		return_VALUE(0);
 	}
 
+	processor_driver_registered = 1;
+
 	acpi_processor_install_hotplug_notify();
 
 	acpi_thermal_cpufreq_init();

@@ -943,12 +947,13 @@ static void __exit acpi_processor_exit(v
 
 	acpi_thermal_cpufreq_exit();
 
+	if (processor_driver_registered){
 	acpi_processor_uninstall_hotplug_notify();
 
 	acpi_bus_unregister_driver(&acpi_processor_driver);
 
 	remove_proc_entry(ACPI_PROCESSOR_CLASS, acpi_root_dir);
-
+	}
 	return_VOID;
 }

Bjorn Helgaas

2006-04-24 14:59 UTC #6 lore raw

Quoted 3 lines · Thomas Renninger

Rethinking about this: driver_registered is a bit too general variable?
Is this OK, now?

It's static, so I don't care either way.

The powernow/speedstep/acpi code sharing seems a little strange, but I don't know anything about it. And you're not changing that part of things anyway.

drivers/acpi/processor_core.c static void __exit acpi_processor_exit(v · line 956

Quoted 12 lines

drivers/acpi/processor_core.c +2 -1

@@ -943,12 +947,13 @@ static void __exit acpi_processor_exit(v
 
 	acpi_thermal_cpufreq_exit();
 
+	if (processor_driver_registered){
 	acpi_processor_uninstall_hotplug_notify();
 
 	acpi_bus_unregister_driver(&acpi_processor_driver);
 
 	remove_proc_entry(ACPI_PROCESSOR_CLASS, acpi_root_dir);
-
+	}

Shouldn't the remove_proc_entry() be outside the "if (processor_driver_registered)" block? The directory was created unconditionally, before the acpi_bus_register_driver() call.

Thomas Renninger

author

2006-04-24 15:56 UTC #7 lore raw

Quoted 20 lines · Bjorn Helgaas

Quoted 3 lines · Thomas Renninger
Rethinking about this: driver_registered is a bit too general variable?
Is this OK, now?
It's static, so I don't care either way.
The powernow/speedstep/acpi code sharing seems a little strange, but I don't know anything about it. And you're not changing that part of things anyway.
Quoted 9 lines
drivers/acpi/processor_core.c +1 -0
@@ -943,12 +947,13 @@ static void __exit acpi_processor_exit(v
 
 	acpi_thermal_cpufreq_exit();
 
+	if (processor_driver_registered){
 	acpi_processor_uninstall_hotplug_notify();
 
 	acpi_bus_unregister_driver(&acpi_processor_driver);
 

Quoted 7 lines

drivers/acpi/processor_core.c static void __exit acpi_processor_exit(v · line 956
Quoted 3 lines
drivers/acpi/processor_core.c +1 -1
 	remove_proc_entry(ACPI_PROCESSOR_CLASS, acpi_root_dir);
-
+	}
Shouldn't the remove_proc_entry() be outside the "if (processor_driver_registered)" block? The directory was created unconditionally, before the acpi_bus_register_driver() call.

That should be OK as it is conditionally created in the way that init func returns,
if proc_mkdir returns NULL:

init():

	acpi_processor_dir = proc_mkdir(ACPI_PROCESSOR_CLASS, acpi_root_dir);
	if (!acpi_processor_dir)
/* -> proc_mkdir failed -> remove_proc_entry not needed */
		return_VALUE(0);

	acpi_processor_dir->owner = THIS_MODULE;

	result = acpi_bus_register_driver(&acpi_processor_driver);
	if (result < 0) {
		remove_proc_entry(ACPI_PROCESSOR_CLASS, acpi_root_dir);
/* here it is already removed ... */
		return_VALUE(0);
	}
/*        else ... here it must be removed... */
	processor_driver_registered = 1;

Thanks,

Thomas