* [PATCH]: ACPI: Add the reference count to avoid unloading ACPI video bus twice
@ 2009-06-16 3:23 yakui_zhao
2009-06-16 14:50 ` Alex Chiang
2009-06-24 3:36 ` Zhang Rui
0 siblings, 2 replies; 6+ messages in thread
From: yakui_zhao @ 2009-06-16 3:23 UTC (permalink / raw)
To: lenb; +Cc: linux-acpi
From: Zhao Yakui <yakui.zhao@intel.com>
Sometimes both acpi video and i915 driver are compiled as modules.
And there exists the strict dependency between the two drivers.
The acpi video bus will be unloaded in course of unloading the i915 driver.
If we unload the acpi video driver, then the kernel oops will be triggered.
Add the reference count to avoid unloading the ACPI video bus twice.
The reference count should be checked before unregistering the acpi video bus.
If the reference count is already zero, it won't unregister it again.
And after the acpi video bus is already unregistered, the reference count
will be set to zero.
http://bugzilla.kernel.org/show_bug.cgi?id=13396
Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
---
drivers/acpi/video.c | 41 +++++++++++++++++++++++++++++------
drivers/gpu/drm/i915/i915_opregion.c | 2 -
include/acpi/video.h | 4 +--
3 files changed, 38 insertions(+), 9 deletions(-)
Index: linux-2.6/drivers/acpi/video.c
===================================================================
--- linux-2.6.orig/drivers/acpi/video.c 2009-06-16 10:25:12.000000000 +0800
+++ linux-2.6/drivers/acpi/video.c 2009-06-16 11:09:57.000000000 +0800
@@ -76,6 +76,7 @@
static int brightness_switch_enabled = 1;
module_param(brightness_switch_enabled, bool, 0644);
+static int register_count = 0;
static int acpi_video_bus_add(struct acpi_device *device);
static int acpi_video_bus_remove(struct acpi_device *device, int type);
static int acpi_video_resume(struct acpi_device *device);
@@ -2318,6 +2319,13 @@
int acpi_video_register(void)
{
int result = 0;
+ if (register_count) {
+ /*
+ * if the function of acpi_video_register is already called,
+ * don't register the acpi_vide_bus again and return no error.
+ */
+ return 0;
+ }
acpi_video_dir = proc_mkdir(ACPI_VIDEO_CLASS, acpi_root_dir);
if (!acpi_video_dir)
@@ -2329,10 +2337,35 @@
return -ENODEV;
}
+ /*
+ * When the acpi_video_bus is loaded successfully, increase
+ * the counter reference.
+ */
+ register_count = 1;
+
return 0;
}
EXPORT_SYMBOL(acpi_video_register);
+void acpi_video_unregister(void)
+{
+ if (!register_count) {
+ /*
+ * If the acpi video bus is already unloaded, don't
+ * unload it again and return directly.
+ */
+ return;
+ }
+ acpi_bus_unregister_driver(&acpi_video_bus);
+
+ remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
+
+ register_count = 0;
+
+ return;
+}
+EXPORT_SYMBOL(acpi_video_unregister);
+
/*
* This is kind of nasty. Hardware using Intel chipsets may require
* the video opregion code to be run first in order to initialise
@@ -2350,16 +2383,12 @@
return acpi_video_register();
}
-void acpi_video_exit(void)
+static void __exit acpi_video_exit(void)
{
-
- acpi_bus_unregister_driver(&acpi_video_bus);
-
- remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
+ acpi_video_unregister();
return;
}
-EXPORT_SYMBOL(acpi_video_exit);
module_init(acpi_video_init);
module_exit(acpi_video_exit);
Index: linux-2.6/drivers/gpu/drm/i915/i915_opregion.c
===================================================================
--- linux-2.6.orig/drivers/gpu/drm/i915/i915_opregion.c 2009-06-16 10:25:12.000000000 +0800
+++ linux-2.6/drivers/gpu/drm/i915/i915_opregion.c 2009-06-16 10:29:50.000000000 +0800
@@ -419,7 +419,7 @@
return;
if (!suspend)
- acpi_video_exit();
+ acpi_video_unregister();
opregion->acpi->drdy = 0;
Index: linux-2.6/include/acpi/video.h
===================================================================
--- linux-2.6.orig/include/acpi/video.h 2009-06-16 10:25:12.000000000 +0800
+++ linux-2.6/include/acpi/video.h 2009-06-16 11:07:43.000000000 +0800
@@ -3,10 +3,10 @@
#if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
extern int acpi_video_register(void);
-extern int acpi_video_exit(void);
+extern void acpi_video_unregister(void);
#else
static inline int acpi_video_register(void) { return 0; }
-static inline void acpi_video_exit(void) { return; }
+static inline void acpi_video_unregister(void) { return; }
#endif
#endif
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH]: ACPI: Add the reference count to avoid unloading ACPI video bus twice
2009-06-16 3:23 [PATCH]: ACPI: Add the reference count to avoid unloading ACPI video bus twice yakui_zhao
@ 2009-06-16 14:50 ` Alex Chiang
2009-06-17 3:09 ` yakui_zhao
2009-06-24 3:36 ` Zhang Rui
1 sibling, 1 reply; 6+ messages in thread
From: Alex Chiang @ 2009-06-16 14:50 UTC (permalink / raw)
To: yakui_zhao; +Cc: lenb, linux-acpi
Hi Yakui,
* yakui_zhao <yakui.zhao@intel.com>:
> From: Zhao Yakui <yakui.zhao@intel.com>
>
> Sometimes both acpi video and i915 driver are compiled as modules.
> And there exists the strict dependency between the two drivers.
> The acpi video bus will be unloaded in course of unloading the i915 driver.
> If we unload the acpi video driver, then the kernel oops will be triggered.
>
> Add the reference count to avoid unloading the ACPI video bus twice.
> The reference count should be checked before unregistering the acpi video bus.
> If the reference count is already zero, it won't unregister it again.
> And after the acpi video bus is already unregistered, the reference count
> will be set to zero.
Your implementation below isn't really a reference count, so I
don't think you should call it that in your changelog.
Since you are talking about refcounts, have you tried using
try_module_get()?
Thanks.
/ac
>
> http://bugzilla.kernel.org/show_bug.cgi?id=13396
>
> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> ---
> drivers/acpi/video.c | 41 +++++++++++++++++++++++++++++------
> drivers/gpu/drm/i915/i915_opregion.c | 2 -
> include/acpi/video.h | 4 +--
> 3 files changed, 38 insertions(+), 9 deletions(-)
>
> Index: linux-2.6/drivers/acpi/video.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/video.c 2009-06-16 10:25:12.000000000 +0800
> +++ linux-2.6/drivers/acpi/video.c 2009-06-16 11:09:57.000000000 +0800
> @@ -76,6 +76,7 @@
> static int brightness_switch_enabled = 1;
> module_param(brightness_switch_enabled, bool, 0644);
>
> +static int register_count = 0;
> static int acpi_video_bus_add(struct acpi_device *device);
> static int acpi_video_bus_remove(struct acpi_device *device, int type);
> static int acpi_video_resume(struct acpi_device *device);
> @@ -2318,6 +2319,13 @@
> int acpi_video_register(void)
> {
> int result = 0;
> + if (register_count) {
> + /*
> + * if the function of acpi_video_register is already called,
> + * don't register the acpi_vide_bus again and return no error.
> + */
> + return 0;
> + }
>
> acpi_video_dir = proc_mkdir(ACPI_VIDEO_CLASS, acpi_root_dir);
> if (!acpi_video_dir)
> @@ -2329,10 +2337,35 @@
> return -ENODEV;
> }
>
> + /*
> + * When the acpi_video_bus is loaded successfully, increase
> + * the counter reference.
> + */
> + register_count = 1;
> +
> return 0;
> }
> EXPORT_SYMBOL(acpi_video_register);
>
> +void acpi_video_unregister(void)
> +{
> + if (!register_count) {
> + /*
> + * If the acpi video bus is already unloaded, don't
> + * unload it again and return directly.
> + */
> + return;
> + }
> + acpi_bus_unregister_driver(&acpi_video_bus);
> +
> + remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
> +
> + register_count = 0;
> +
> + return;
> +}
> +EXPORT_SYMBOL(acpi_video_unregister);
> +
> /*
> * This is kind of nasty. Hardware using Intel chipsets may require
> * the video opregion code to be run first in order to initialise
> @@ -2350,16 +2383,12 @@
> return acpi_video_register();
> }
>
> -void acpi_video_exit(void)
> +static void __exit acpi_video_exit(void)
> {
> -
> - acpi_bus_unregister_driver(&acpi_video_bus);
> -
> - remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
> + acpi_video_unregister();
>
> return;
> }
> -EXPORT_SYMBOL(acpi_video_exit);
>
> module_init(acpi_video_init);
> module_exit(acpi_video_exit);
> Index: linux-2.6/drivers/gpu/drm/i915/i915_opregion.c
> ===================================================================
> --- linux-2.6.orig/drivers/gpu/drm/i915/i915_opregion.c 2009-06-16 10:25:12.000000000 +0800
> +++ linux-2.6/drivers/gpu/drm/i915/i915_opregion.c 2009-06-16 10:29:50.000000000 +0800
> @@ -419,7 +419,7 @@
> return;
>
> if (!suspend)
> - acpi_video_exit();
> + acpi_video_unregister();
>
> opregion->acpi->drdy = 0;
>
> Index: linux-2.6/include/acpi/video.h
> ===================================================================
> --- linux-2.6.orig/include/acpi/video.h 2009-06-16 10:25:12.000000000 +0800
> +++ linux-2.6/include/acpi/video.h 2009-06-16 11:07:43.000000000 +0800
> @@ -3,10 +3,10 @@
>
> #if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
> extern int acpi_video_register(void);
> -extern int acpi_video_exit(void);
> +extern void acpi_video_unregister(void);
> #else
> static inline int acpi_video_register(void) { return 0; }
> -static inline void acpi_video_exit(void) { return; }
> +static inline void acpi_video_unregister(void) { return; }
> #endif
>
> #endif
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH]: ACPI: Add the reference count to avoid unloading ACPI video bus twice
2009-06-16 14:50 ` Alex Chiang
@ 2009-06-17 3:09 ` yakui_zhao
0 siblings, 0 replies; 6+ messages in thread
From: yakui_zhao @ 2009-06-17 3:09 UTC (permalink / raw)
To: Alex Chiang; +Cc: lenb@kernel.org, linux-acpi@vger.kernel.org
On Tue, 2009-06-16 at 22:50 +0800, Alex Chiang wrote:
> Hi Yakui,
>
> * yakui_zhao <yakui.zhao@intel.com>:
> > From: Zhao Yakui <yakui.zhao@intel.com>
> >
> > Sometimes both acpi video and i915 driver are compiled as modules.
> > And there exists the strict dependency between the two drivers.
> > The acpi video bus will be unloaded in course of unloading the i915 driver.
> > If we unload the acpi video driver, then the kernel oops will be triggered.
> >
> > Add the reference count to avoid unloading the ACPI video bus twice.
> > The reference count should be checked before unregistering the acpi video bus.
> > If the reference count is already zero, it won't unregister it again.
> > And after the acpi video bus is already unregistered, the reference count
> > will be set to zero.
>
> Your implementation below isn't really a reference count, so I
> don't think you should call it that in your changelog.
It is only a register count, which indicates whether the acpi video bus
is registered or not.
And it is not a the reference count for module.
Thanks.
>
> Since you are talking about refcounts, have you tried using
> try_module_get()?
>
> Thanks.
>
> /ac
>
> >
> > http://bugzilla.kernel.org/show_bug.cgi?id=13396
> >
> > Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> > ---
> > drivers/acpi/video.c | 41 +++++++++++++++++++++++++++++------
> > drivers/gpu/drm/i915/i915_opregion.c | 2 -
> > include/acpi/video.h | 4 +--
> > 3 files changed, 38 insertions(+), 9 deletions(-)
> >
> > Index: linux-2.6/drivers/acpi/video.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/video.c 2009-06-16 10:25:12.000000000 +0800
> > +++ linux-2.6/drivers/acpi/video.c 2009-06-16 11:09:57.000000000 +0800
> > @@ -76,6 +76,7 @@
> > static int brightness_switch_enabled = 1;
> > module_param(brightness_switch_enabled, bool, 0644);
> >
> > +static int register_count = 0;
> > static int acpi_video_bus_add(struct acpi_device *device);
> > static int acpi_video_bus_remove(struct acpi_device *device, int type);
> > static int acpi_video_resume(struct acpi_device *device);
> > @@ -2318,6 +2319,13 @@
> > int acpi_video_register(void)
> > {
> > int result = 0;
> > + if (register_count) {
> > + /*
> > + * if the function of acpi_video_register is already called,
> > + * don't register the acpi_vide_bus again and return no error.
> > + */
> > + return 0;
> > + }
> >
> > acpi_video_dir = proc_mkdir(ACPI_VIDEO_CLASS, acpi_root_dir);
> > if (!acpi_video_dir)
> > @@ -2329,10 +2337,35 @@
> > return -ENODEV;
> > }
> >
> > + /*
> > + * When the acpi_video_bus is loaded successfully, increase
> > + * the counter reference.
> > + */
> > + register_count = 1;
> > +
> > return 0;
> > }
> > EXPORT_SYMBOL(acpi_video_register);
> >
> > +void acpi_video_unregister(void)
> > +{
> > + if (!register_count) {
> > + /*
> > + * If the acpi video bus is already unloaded, don't
> > + * unload it again and return directly.
> > + */
> > + return;
> > + }
> > + acpi_bus_unregister_driver(&acpi_video_bus);
> > +
> > + remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
> > +
> > + register_count = 0;
> > +
> > + return;
> > +}
> > +EXPORT_SYMBOL(acpi_video_unregister);
> > +
> > /*
> > * This is kind of nasty. Hardware using Intel chipsets may require
> > * the video opregion code to be run first in order to initialise
> > @@ -2350,16 +2383,12 @@
> > return acpi_video_register();
> > }
> >
> > -void acpi_video_exit(void)
> > +static void __exit acpi_video_exit(void)
> > {
> > -
> > - acpi_bus_unregister_driver(&acpi_video_bus);
> > -
> > - remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
> > + acpi_video_unregister();
> >
> > return;
> > }
> > -EXPORT_SYMBOL(acpi_video_exit);
> >
> > module_init(acpi_video_init);
> > module_exit(acpi_video_exit);
> > Index: linux-2.6/drivers/gpu/drm/i915/i915_opregion.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/gpu/drm/i915/i915_opregion.c 2009-06-16 10:25:12.000000000 +0800
> > +++ linux-2.6/drivers/gpu/drm/i915/i915_opregion.c 2009-06-16 10:29:50.000000000 +0800
> > @@ -419,7 +419,7 @@
> > return;
> >
> > if (!suspend)
> > - acpi_video_exit();
> > + acpi_video_unregister();
> >
> > opregion->acpi->drdy = 0;
> >
> > Index: linux-2.6/include/acpi/video.h
> > ===================================================================
> > --- linux-2.6.orig/include/acpi/video.h 2009-06-16 10:25:12.000000000 +0800
> > +++ linux-2.6/include/acpi/video.h 2009-06-16 11:07:43.000000000 +0800
> > @@ -3,10 +3,10 @@
> >
> > #if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
> > extern int acpi_video_register(void);
> > -extern int acpi_video_exit(void);
> > +extern void acpi_video_unregister(void);
> > #else
> > static inline int acpi_video_register(void) { return 0; }
> > -static inline void acpi_video_exit(void) { return; }
> > +static inline void acpi_video_unregister(void) { return; }
> > #endif
> >
> > #endif
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH]: ACPI: Add the reference count to avoid unloading ACPI video bus twice
2009-06-16 3:23 [PATCH]: ACPI: Add the reference count to avoid unloading ACPI video bus twice yakui_zhao
2009-06-16 14:50 ` Alex Chiang
@ 2009-06-24 3:36 ` Zhang Rui
1 sibling, 0 replies; 6+ messages in thread
From: Zhang Rui @ 2009-06-24 3:36 UTC (permalink / raw)
To: yakui_zhao; +Cc: lenb@kernel.org, linux-acpi@vger.kernel.org
On Tue, 2009-06-16 at 11:23 +0800, yakui_zhao wrote:
> From: Zhao Yakui <yakui.zhao@intel.com>
>
> Sometimes both acpi video and i915 driver are compiled as modules.
> And there exists the strict dependency between the two drivers.
> The acpi video bus will be unloaded in course of unloading the i915 driver.
> If we unload the acpi video driver, then the kernel oops will be triggered.
>
> Add the reference count to avoid unloading the ACPI video bus twice.
> The reference count should be checked before unregistering the acpi video bus.
> If the reference count is already zero, it won't unregister it again.
> And after the acpi video bus is already unregistered, the reference count
> will be set to zero.
>
> http://bugzilla.kernel.org/show_bug.cgi?id=13396
>
> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
Acked-by: Zhang Rui <rui.zhang@intel.com>
But we'd better rename the patch to something like "unregister ACPI
video driver only once".
thanks,
rui
> ---
> drivers/acpi/video.c | 41 +++++++++++++++++++++++++++++------
> drivers/gpu/drm/i915/i915_opregion.c | 2 -
> include/acpi/video.h | 4 +--
> 3 files changed, 38 insertions(+), 9 deletions(-)
>
> Index: linux-2.6/drivers/acpi/video.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/video.c 2009-06-16 10:25:12.000000000 +0800
> +++ linux-2.6/drivers/acpi/video.c 2009-06-16 11:09:57.000000000 +0800
> @@ -76,6 +76,7 @@
> static int brightness_switch_enabled = 1;
> module_param(brightness_switch_enabled, bool, 0644);
>
> +static int register_count = 0;
> static int acpi_video_bus_add(struct acpi_device *device);
> static int acpi_video_bus_remove(struct acpi_device *device, int type);
> static int acpi_video_resume(struct acpi_device *device);
> @@ -2318,6 +2319,13 @@
> int acpi_video_register(void)
> {
> int result = 0;
> + if (register_count) {
> + /*
> + * if the function of acpi_video_register is already called,
> + * don't register the acpi_vide_bus again and return no error.
> + */
> + return 0;
> + }
>
> acpi_video_dir = proc_mkdir(ACPI_VIDEO_CLASS, acpi_root_dir);
> if (!acpi_video_dir)
> @@ -2329,10 +2337,35 @@
> return -ENODEV;
> }
>
> + /*
> + * When the acpi_video_bus is loaded successfully, increase
> + * the counter reference.
> + */
> + register_count = 1;
> +
> return 0;
> }
> EXPORT_SYMBOL(acpi_video_register);
>
> +void acpi_video_unregister(void)
> +{
> + if (!register_count) {
> + /*
> + * If the acpi video bus is already unloaded, don't
> + * unload it again and return directly.
> + */
> + return;
> + }
> + acpi_bus_unregister_driver(&acpi_video_bus);
> +
> + remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
> +
> + register_count = 0;
> +
> + return;
> +}
> +EXPORT_SYMBOL(acpi_video_unregister);
> +
> /*
> * This is kind of nasty. Hardware using Intel chipsets may require
> * the video opregion code to be run first in order to initialise
> @@ -2350,16 +2383,12 @@
> return acpi_video_register();
> }
>
> -void acpi_video_exit(void)
> +static void __exit acpi_video_exit(void)
> {
> -
> - acpi_bus_unregister_driver(&acpi_video_bus);
> -
> - remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
> + acpi_video_unregister();
>
> return;
> }
> -EXPORT_SYMBOL(acpi_video_exit);
>
> module_init(acpi_video_init);
> module_exit(acpi_video_exit);
> Index: linux-2.6/drivers/gpu/drm/i915/i915_opregion.c
> ===================================================================
> --- linux-2.6.orig/drivers/gpu/drm/i915/i915_opregion.c 2009-06-16 10:25:12.000000000 +0800
> +++ linux-2.6/drivers/gpu/drm/i915/i915_opregion.c 2009-06-16 10:29:50.000000000 +0800
> @@ -419,7 +419,7 @@
> return;
>
> if (!suspend)
> - acpi_video_exit();
> + acpi_video_unregister();
>
> opregion->acpi->drdy = 0;
>
> Index: linux-2.6/include/acpi/video.h
> ===================================================================
> --- linux-2.6.orig/include/acpi/video.h 2009-06-16 10:25:12.000000000 +0800
> +++ linux-2.6/include/acpi/video.h 2009-06-16 11:07:43.000000000 +0800
> @@ -3,10 +3,10 @@
>
> #if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
> extern int acpi_video_register(void);
> -extern int acpi_video_exit(void);
> +extern void acpi_video_unregister(void);
> #else
> static inline int acpi_video_register(void) { return 0; }
> -static inline void acpi_video_exit(void) { return; }
> +static inline void acpi_video_unregister(void) { return; }
> #endif
>
> #endif
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH]: ACPI: Add the reference count to avoid unloading ACPI video bus twice
@ 2009-06-12 0:51 yakui_zhao
0 siblings, 0 replies; 6+ messages in thread
From: yakui_zhao @ 2009-06-12 0:51 UTC (permalink / raw)
To: lenb; +Cc: linux-acpi
From: Zhao Yakui <yakui.zhao@intel.com>
Sometimes both acpi video and i915 driver are compiled as modules.
And there exists the strict dependency between the two drivers.
The acpi video bus will be unloaded in course of unloading the i915 driver.
If we unload the acpi video driver, then the kernel oops will be triggered.
Add the reference count to avoid unloading the ACPI video bus twice.
The reference count should be checked before unregistering the acpi video bus.
If the reference count is already zero, it won't unregister it again.
And after the acpi video bus is already unregistered, the reference count
will be set to zero.
At the same time the reference count is also checked when registering
the acpi video bus.
http://bugzilla.kernel.org/show_bug.cgi?id=13396
Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
---
drivers/acpi/video.c | 42 ++++++++++++++++++++++++++++-------
drivers/gpu/drm/i915/i915_opregion.c | 2 -
include/acpi/video.h | 4 +--
3 files changed, 37 insertions(+), 11 deletions(-)
Index: linux-2.6/drivers/acpi/video.c
===================================================================
--- linux-2.6.orig/drivers/acpi/video.c 2009-05-31 09:33:17.000000000 +0800
+++ linux-2.6/drivers/acpi/video.c 2009-06-01 13:29:41.000000000 +0800
@@ -76,6 +76,7 @@
static int brightness_switch_enabled = 1;
module_param(brightness_switch_enabled, bool, 0644);
+static int video_atomic = 0;
static int acpi_video_bus_add(struct acpi_device *device);
static int acpi_video_bus_remove(struct acpi_device *device, int type);
static int acpi_video_resume(struct acpi_device *device);
@@ -2302,7 +2303,13 @@
int acpi_video_register(void)
{
int result = 0;
-
+ if (video_atomic) {
+ /*
+ * if the function of acpi_video_register is already called,
+ * don't register the acpi_vide_bus again and return no error.
+ */
+ return 0;
+ }
acpi_video_dir = proc_mkdir(ACPI_VIDEO_CLASS, acpi_root_dir);
if (!acpi_video_dir)
return -ENODEV;
@@ -2312,11 +2319,34 @@
remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
return -ENODEV;
}
-
+ /*
+ * When the acpi_video_bus is loaded successfully, increase
+ * the counter reference.
+ */
+ video_atomic = 1;
return 0;
}
EXPORT_SYMBOL(acpi_video_register);
+void acpi_video_unregister(void)
+{
+ if (!video_atomic) {
+ /*
+ * If the acpi video bus is already unloaded, don't
+ * unload it again and return directly.
+ */
+ return;
+ }
+ acpi_bus_unregister_driver(&acpi_video_bus);
+
+ remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
+
+ video_atomic = 0;
+
+ return;
+}
+EXPORT_SYMBOL(acpi_video_unregister);
+
/*
* This is kind of nasty. Hardware using Intel chipsets may require
* the video opregion code to be run first in order to initialise
@@ -2334,16 +2364,12 @@
return acpi_video_register();
}
-void __exit acpi_video_exit(void)
+static void __exit acpi_video_exit(void)
{
-
- acpi_bus_unregister_driver(&acpi_video_bus);
-
- remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
+ acpi_video_unregister();
return;
}
-EXPORT_SYMBOL(acpi_video_exit);
module_init(acpi_video_init);
module_exit(acpi_video_exit);
Index: linux-2.6/include/acpi/video.h
===================================================================
--- linux-2.6.orig/include/acpi/video.h 2009-04-29 09:32:00.000000000 +0800
+++ linux-2.6/include/acpi/video.h 2009-06-01 13:34:19.000000000 +0800
@@ -3,10 +3,10 @@
#if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
extern int acpi_video_register(void);
-extern int acpi_video_exit(void);
+extern void acpi_video_unregister(void);
#else
static inline int acpi_video_register(void) { return 0; }
-static inline void acpi_video_exit(void) { return; }
+static inline void acpi_video_unregister(void) { return; }
#endif
#endif
Index: linux-2.6/drivers/gpu/drm/i915/i915_opregion.c
===================================================================
--- linux-2.6.orig/drivers/gpu/drm/i915/i915_opregion.c 2009-04-29 09:32:00.000000000 +0800
+++ linux-2.6/drivers/gpu/drm/i915/i915_opregion.c 2009-06-01 12:02:00.000000000 +0800
@@ -419,7 +419,7 @@
return;
if (!suspend)
- acpi_video_exit();
+ acpi_video_unregister();
opregion->acpi->drdy = 0;
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH]: ACPI: Add the reference count to avoid unloading ACPI video bus twice
@ 2009-06-01 5:44 yakui_zhao
0 siblings, 0 replies; 6+ messages in thread
From: yakui_zhao @ 2009-06-01 5:44 UTC (permalink / raw)
To: lenb; +Cc: linux-acpi
Sometimes both acpi video and i915 driver are compiled as modules.
And there exists the strict dependency between the two drivers.
The acpi video bus will be unloaded in course of unloading the i915 driver.
If we unload the acpi video driver, then the kernel oops will be triggered.
Add the reference count to avoid unloading the ACPI video bus twice.
The reference count should be checked before unregistering the acpi video bus.
If the reference count is already zero, it won't unregister it again.
And after the acpi video bus is already unregistered, the reference count
will be set to zero.
At the same time the reference count is also checked when registering
the acpi video bus.
http://bugzilla.kernel.org/show_bug.cgi?id=13396
Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
---
drivers/acpi/video.c | 42 ++++++++++++++++++++++++++++-------
drivers/gpu/drm/i915/i915_opregion.c | 2 -
include/acpi/video.h | 4 +--
3 files changed, 37 insertions(+), 11 deletions(-)
Index: linux-2.6/drivers/acpi/video.c
===================================================================
--- linux-2.6.orig/drivers/acpi/video.c 2009-05-31 09:33:17.000000000 +0800
+++ linux-2.6/drivers/acpi/video.c 2009-06-01 13:29:41.000000000 +0800
@@ -76,6 +76,7 @@
static int brightness_switch_enabled = 1;
module_param(brightness_switch_enabled, bool, 0644);
+static int video_atomic = 0;
static int acpi_video_bus_add(struct acpi_device *device);
static int acpi_video_bus_remove(struct acpi_device *device, int type);
static int acpi_video_resume(struct acpi_device *device);
@@ -2302,7 +2303,13 @@
int acpi_video_register(void)
{
int result = 0;
-
+ if (video_atomic) {
+ /*
+ * if the function of acpi_video_register is already called,
+ * don't register the acpi_vide_bus again and return no error.
+ */
+ return 0;
+ }
acpi_video_dir = proc_mkdir(ACPI_VIDEO_CLASS, acpi_root_dir);
if (!acpi_video_dir)
return -ENODEV;
@@ -2312,11 +2319,34 @@
remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
return -ENODEV;
}
-
+ /*
+ * When the acpi_video_bus is loaded successfully, increase
+ * the counter reference.
+ */
+ video_atomic = 1;
return 0;
}
EXPORT_SYMBOL(acpi_video_register);
+void acpi_video_unregister(void)
+{
+ if (!video_atomic) {
+ /*
+ * If the acpi video bus is already unloaded, don't
+ * unload it again and return directly.
+ */
+ return;
+ }
+ acpi_bus_unregister_driver(&acpi_video_bus);
+
+ remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
+
+ video_atomic = 0;
+
+ return;
+}
+EXPORT_SYMBOL(acpi_video_unregister);
+
/*
* This is kind of nasty. Hardware using Intel chipsets may require
* the video opregion code to be run first in order to initialise
@@ -2334,16 +2364,12 @@
return acpi_video_register();
}
-void __exit acpi_video_exit(void)
+static void __exit acpi_video_exit(void)
{
-
- acpi_bus_unregister_driver(&acpi_video_bus);
-
- remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
+ acpi_video_unregister();
return;
}
-EXPORT_SYMBOL(acpi_video_exit);
module_init(acpi_video_init);
module_exit(acpi_video_exit);
Index: linux-2.6/include/acpi/video.h
===================================================================
--- linux-2.6.orig/include/acpi/video.h 2009-04-29 09:32:00.000000000 +0800
+++ linux-2.6/include/acpi/video.h 2009-06-01 13:34:19.000000000 +0800
@@ -3,10 +3,10 @@
#if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
extern int acpi_video_register(void);
-extern int acpi_video_exit(void);
+extern void acpi_video_unregister(void);
#else
static inline int acpi_video_register(void) { return 0; }
-static inline void acpi_video_exit(void) { return; }
+static inline void acpi_video_unregister(void) { return; }
#endif
#endif
Index: linux-2.6/drivers/gpu/drm/i915/i915_opregion.c
===================================================================
--- linux-2.6.orig/drivers/gpu/drm/i915/i915_opregion.c 2009-04-29 09:32:00.000000000 +0800
+++ linux-2.6/drivers/gpu/drm/i915/i915_opregion.c 2009-06-01 12:02:00.000000000 +0800
@@ -419,7 +419,7 @@
return;
if (!suspend)
- acpi_video_exit();
+ acpi_video_unregister();
opregion->acpi->drdy = 0;
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-06-24 3:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-06-16 3:23 [PATCH]: ACPI: Add the reference count to avoid unloading ACPI video bus twice yakui_zhao
2009-06-16 14:50 ` Alex Chiang
2009-06-17 3:09 ` yakui_zhao
2009-06-24 3:36 ` Zhang Rui
-- strict thread matches above, loose matches on Subject: below --
2009-06-12 0:51 yakui_zhao
2009-06-01 5:44 yakui_zhao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox