ps3fb: fix deadlock on kexec()
authorJeremy Kerr <jk@ozlabs.org>
Fri, 11 Jan 2008 13:28:04 +0000 (14:28 +0100)
committerLinus Torvalds <torvalds@woody.linux-foundation.org>
Fri, 11 Jan 2008 19:47:43 +0000 (11:47 -0800)
Since the introduction of the acquire_console_sem calls in
0333d83509c7d8496c8965b5ba9bc0c98e83c259, kexecing can cause the
kernel to deadlock:

 ps3fb_shutdown()
  -> unregister_framebuffer()
  -> fb_notifier_call_chain(FB_EVENT_FB_UNBIND)
  -> fbcon_fb_unbind()
  -> unbind_con_driver()
  -> bind_con_driver()
[ acquires console_sem ]
  -> fbcon_deinit()
  -> fbops->fb_release(newinfo, 0)
  -> ps3fb_release()
  -> ps3fb_sync()
[ acquires console_sem ]

This change avoids the deadlock by moving the acquire_console_sem()
out of ps3fb_sync(), and puts it into the two other callsites, leaving
ps3fb_release() to call ps3fb_sync() without the console semaphore.

[Geert]
  - Corrected call sequence above
  - ps3fb_release() may be called with and without console_sem held. This is an
    inconsistency that should be fixed at the fb level, but for now, try to
    acquire console_sem in ps3fb_release().

    I think it's safer to let ps3fb_release() try to acquire console_sem and
    not refresh the screen if it fails, than to call ps3fb_sync() without
    holding console_sem, as ps3fb_par may be modified at the same time, causing
    crashes or lockups.

    Besides, ps3fb_release() only calls ps3fb_sync() to refresh the screen
    when display flipping is disabled, which is an uncommon case (except during
    shutdown/kexec).

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
drivers/video/ps3fb.c

index ae07e02943d86946aba598c610dcb3927acd2a5f..044a423a72cbe406e0a5811796b246cb788f1ec3 100644 (file)
@@ -443,8 +443,6 @@ static int ps3fb_sync(struct fb_info *info, u32 frame)
        u32 ddr_line_length, xdr_line_length;
        u64 ddr_base, xdr_base;
 
-       acquire_console_sem();
-
        if (frame > par->num_frames - 1) {
                dev_dbg(info->device, "%s: invalid frame number (%u)\n",
                        __func__, frame);
@@ -464,7 +462,6 @@ static int ps3fb_sync(struct fb_info *info, u32 frame)
                         xdr_line_length);
 
 out:
-       release_console_sem();
        return error;
 }
 
@@ -479,7 +476,10 @@ static int ps3fb_release(struct fb_info *info, int user)
        if (atomic_dec_and_test(&ps3fb.f_count)) {
                if (atomic_read(&ps3fb.ext_flip)) {
                        atomic_set(&ps3fb.ext_flip, 0);
-                       ps3fb_sync(info, 0);    /* single buffer */
+                       if (!try_acquire_console_sem()) {
+                               ps3fb_sync(info, 0);    /* single buffer */
+                               release_console_sem();
+                       }
                }
        }
        return 0;
@@ -865,7 +865,9 @@ static int ps3fb_ioctl(struct fb_info *info, unsigned int cmd,
                        break;
 
                dev_dbg(info->device, "PS3FB_IOCTL_FSEL:%d\n", val);
+               acquire_console_sem();
                retval = ps3fb_sync(info, val);
+               release_console_sem();
                break;
 
        default:
@@ -885,7 +887,9 @@ static int ps3fbd(void *arg)
                set_current_state(TASK_INTERRUPTIBLE);
                if (ps3fb.is_kicked) {
                        ps3fb.is_kicked = 0;
+                       acquire_console_sem();
                        ps3fb_sync(info, 0);    /* single buffer */
+                       release_console_sem();
                }
                schedule();
        }