On Tue, 2009-06-16 at 22:50 +0800, Alex Chiang wrote: > Hi Yakui, > > * yakui_zhao <yakui.zhao@xxxxxxxxx>: > > From: Zhao Yakui <yakui.zhao@xxxxxxxxx> > > > > 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@xxxxxxxxx> > > --- > > 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@xxxxxxxxxxxxxxx > > 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@xxxxxxxxxxxxxxx > 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
