[patch] fbcon: Fix workqueue shutdown

[patch] fbcon: Fix workqueue shutdown

[Geoff Levand]

Add a call to cancel_work_sync() in fbcon_exit() to cancel
any pending work in the fbcon workqueue.  Also, change the
assignment of the workqueue function info->queue.func from
NULL to fb_flashcursor(), and add a corresponding check in
fb_flashcursor() to exit immediately if the variable
fbcon_has_exited is set.

The current implementation of fbcon_exit() sets the fbcon
workqueue function info->queue.func to NULL, but does not
assure that there is no work pending when it does so.  On
occasion, depending on system timing, there will still be
pending work in the queue when fbcon_exit() is called.
This results in a null pointer deference when
run_workqueue() tries to call the queue's work function.

Fixes errors on shutdown similar to these:

  Console: switching to colour dummy device 80x25
  Unable to handle kernel paging request for data at address 0x00000000

Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
 drivers/video/console/fbcon.c |   19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -394,13 +394,19 @@ static void fbcon_update_softback(struct
 
 static void fb_flashcursor(struct work_struct *work)
 {
-	struct fb_info *info = container_of(work, struct fb_info, queue);
-	struct fbcon_ops *ops = info->fbcon_par;
+	struct fb_info *info;
+	struct fbcon_ops *ops;
 	struct display *p;
 	struct vc_data *vc = NULL;
 	int c;
 	int mode;
 
+	if (fbcon_has_exited)
+		return;
+
+	info = container_of(work, struct fb_info, queue);
+	ops = info->fbcon_par;
+
 	acquire_console_sem();
 	if (ops && ops->currcon != -1)
 		vc = vc_cons[ops->currcon].d;
@@ -3534,12 +3540,18 @@ static void fbcon_exit(void)
 	softback_buf = 0UL;
 
 	for (i = 0; i < FB_MAX; i++) {
+		int pending;
+
 		mapped = 0;
 		info = registered_fb[i];
 
 		if (info == NULL)
 			continue;
 
+		pending = cancel_work_sync(&info->queue);
+		DPRINTK("fbcon: %s pending work\n", (pending ? "canceled" :
+			"no"));
+
 		for (j = first_fb_vc; j <= last_fb_vc; j++) {
 			if (con2fb_map[j] == i)
 				mapped = 1;
@@ -3558,9 +3570,6 @@ static void fbcon_exit(void)
 				kfree(info->fbcon_par);
 				info->fbcon_par = NULL;
 			}
-
-			if (info->queue.func == fb_flashcursor)
-				info->queue.func = NULL;
 		}
 	}
 


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[patch] fbcon: Fix workqueue shutdown

[Geoff Levand]

Add a call to cancel_work_sync() in fbcon_exit() to cancel
any pending work in the fbcon workqueue.  Also, change the
assignment of the workqueue function info->queue.func from
NULL to fb_flashcursor(), and add a corresponding check in
fb_flashcursor() to exit immediately if the variable
fbcon_has_exited is set.

The current implementation of fbcon_exit() sets the fbcon
workqueue function info->queue.func to NULL, but does not
assure that there is no work pending when it does so.  On
occasion, depending on system timing, there will still be
pending work in the queue when fbcon_exit() is called.
This results in a null pointer deference when
run_workqueue() tries to call the queue's work function.

Fixes errors on shutdown similar to these:

  Console: switching to colour dummy device 80x25
  Unable to handle kernel paging request for data at address 0x00000000

Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
Hi,

This is a resend of my 10/30/2008 patch.  I didn't
get any feed back.

I was hoping to get this bug fixed for 2.6.29.

-Geoff
 
 drivers/video/console/fbcon.c |   19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -394,13 +394,19 @@ static void fbcon_update_softback(struct
 
 static void fb_flashcursor(struct work_struct *work)
 {
-	struct fb_info *info = container_of(work, struct fb_info, queue);
-	struct fbcon_ops *ops = info->fbcon_par;
+	struct fb_info *info;
+	struct fbcon_ops *ops;
 	struct display *p;
 	struct vc_data *vc = NULL;
 	int c;
 	int mode;
 
+	if (fbcon_has_exited)
+		return;
+
+	info = container_of(work, struct fb_info, queue);
+	ops = info->fbcon_par;
+
 	acquire_console_sem();
 	if (ops && ops->currcon != -1)
 		vc = vc_cons[ops->currcon].d;
@@ -3534,12 +3540,18 @@ static void fbcon_exit(void)
 	softback_buf = 0UL;
 
 	for (i = 0; i < FB_MAX; i++) {
+		int pending;
+
 		mapped = 0;
 		info = registered_fb[i];
 
 		if (info == NULL)
 			continue;
 
+		pending = cancel_work_sync(&info->queue);
+		DPRINTK("fbcon: %s pending work\n", (pending ? "canceled" :
+			"no"));
+
 		for (j = first_fb_vc; j <= last_fb_vc; j++) {
 			if (con2fb_map[j] == i)
 				mapped = 1;
@@ -3558,9 +3570,6 @@ static void fbcon_exit(void)
 				kfree(info->fbcon_par);
 				info->fbcon_par = NULL;
 			}
-
-			if (info->queue.func == fb_flashcursor)
-				info->queue.func = NULL;
 		}
 	}
 



-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [patch] fbcon: Fix workqueue shutdown

[Andrew Morton]

On Wed, 3 Dec 2008 16:23:12 -0800 Geoff Levand <geoffrey.levand@am.sony.com> wrote:

> Add a call to cancel_work_sync() in fbcon_exit() to cancel
> any pending work in the fbcon workqueue.  Also, change the
> assignment of the workqueue function info->queue.func from
> NULL to fb_flashcursor(),

OK.

> and add a corresponding check in
> fb_flashcursor() to exit immediately if the variable
> fbcon_has_exited is set.

Why was this second part added?

> The current implementation of fbcon_exit() sets the fbcon
> workqueue function info->queue.func to NULL, but does not
> assure that there is no work pending when it does so.  On
> occasion, depending on system timing, there will still be
> pending work in the queue when fbcon_exit() is called.
> This results in a null pointer deference when
> run_workqueue() tries to call the queue's work function.
> 
> Fixes errors on shutdown similar to these:
> 
>   Console: switching to colour dummy device 80x25
>   Unable to handle kernel paging request for data at address 0x00000000
> 
> ...
>
> --- a/drivers/video/console/fbcon.c
> +++ b/drivers/video/console/fbcon.c
> @@ -394,13 +394,19 @@ static void fbcon_update_softback(struct
>  
>  static void fb_flashcursor(struct work_struct *work)
>  {
> -	struct fb_info *info = container_of(work, struct fb_info, queue);
> -	struct fbcon_ops *ops = info->fbcon_par;
> +	struct fb_info *info;
> +	struct fbcon_ops *ops;
>  	struct display *p;
>  	struct vc_data *vc = NULL;
>  	int c;
>  	int mode;
>  
> +	if (fbcon_has_exited)
> +		return;

This whole fbcon_has_exited thing has big red "wrong" lights flashing
all over it.

If drivers get all their refcounting, locking and synchronisation
correct, such things are unneeded.


------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

Re: [patch] fbcon: Fix workqueue shutdown

[Geoff Levand]

Andrew Morton wrote:
> On Wed, 3 Dec 2008 16:23:12 -0800 Geoff Levand <geoffrey.levand@am.sony.com> wrote:
> 
>> Add a call to cancel_work_sync() in fbcon_exit() to cancel
>> any pending work in the fbcon workqueue.  Also, change the
>> assignment of the workqueue function info->queue.func from
>> NULL to fb_flashcursor(),
> 
> OK.
> 
>> and add a corresponding check in
>> fb_flashcursor() to exit immediately if the variable
>> fbcon_has_exited is set.
> 
> Why was this second part added?

Paranoia.  cancel_work_sync() should assure fb_flashcursor()
is not called after fbcon_exit().

I'll post an updated patch.

-Geoff



------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

[patch v2] fbcon: Fix workqueue shutdown

[Geoff Levand]

Add a call to cancel_work_sync() in fbcon_exit() to cancel
any pending work in the fbcon workqueue.

The current implementation of fbcon_exit() sets the fbcon
workqueue function info->queue.func to NULL, but does not
assure that there is no work pending when it does so.  On
occasion, depending on system timing, there will still be
pending work in the queue when fbcon_exit() is called.
This results in a null pointer deference when
run_workqueue() tries to call the queue's work function.

Fixes errors on shutdown similar to these:

  Console: switching to colour dummy device 80x25
  Unable to handle kernel paging request for data at address 0x00000000

Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
v2: Remove use of the fbcon_has_exited variable.

 drivers/video/console/fbcon.c |    6 ++++++
 1 file changed, 6 insertions(+)

--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -3531,12 +3531,18 @@ static void fbcon_exit(void)
 	softback_buf = 0UL;
 
 	for (i = 0; i < FB_MAX; i++) {
+		int pending;
+
 		mapped = 0;
 		info = registered_fb[i];
 
 		if (info == NULL)
 			continue;
 
+		pending = cancel_work_sync(&info->queue);
+		DPRINTK("fbcon: %s pending work\n", (pending ? "canceled" :
+			"no"));
+
 		for (j = first_fb_vc; j <= last_fb_vc; j++) {
 			if (con2fb_map[j] == i)
 				mapped = 1;


------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/