[Cbe-oss-dev] [PATCH 2/2] ps3fb: fix deadlock on kexec()

[Geert Uytterhoeven]

On Thu, 10 Jan 2008, Jeremy Kerr wrote:
> 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()
>   -> set_con2fb_map()
> 	[ acquires console_sem ]
>   -> con2fb_release_oldinfo()
>   -> fbops->fb_release(newinfo, 0)
>   -> ps3fb_release()
>   -> ps3fb_sync()
> 	[ acquires console_sem ]

The actual trace is slightly different:

    .ps3fb_release+0x128/0x198
    .fbcon_deinit+0x1a8/0x250
    .bind_con_driver+0x310/0x4bc
    .unbind_con_driver+0x264/0x36c
    .fbcon_event_notify+0x2e4/0x898
    .notifier_call_chain+0x5c/0xcc
    .__blocking_notifier_call_chain+0x68/0xa4
    .fb_notifier_call_chain+0x8c/0xcc
    .unregister_framebuffer+0xa0/0x170
    .ps3fb_shutdown+0xb0/0x188

because:

  - If you have only one active frame buffer device, fbcon_fb_unbind() won't
    call set_con2fb_map(), as there's no other frame buffer device to switch
    to.
  - ps3fb_release() calls ps3fb_sync() only when /dev/fb0 is open.

On normal shutdown, it doesn't happen because all applications are killed
first.

> 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.

The patch indeed fixes the problem.

But I'm wondering whether it's always safe to call ps3fb_sync() from
ps3fb_release() without acquiring the console semaphore:
  - On console unbind, ps3fb_release() is called while holding the console
    semaphore (cfr. the deadlock),
  - On normal close of /dev/fb0, ps3fb_release() is called without holding the
    console semaphore, i.e. there's no protection against concurreny anymore.

Perhaps ps3fb_release() should do

	if (!try_acquire_console_sem()) {
		ps3fb_sync();
		release_console_sem();
	}

This would do the sync on a normal close, but not on special cases (console
unbind, e.g. shutdown and kexec).

What do you think?

BTW, if we want these fixes in 2.6.24, we probably have to send it ASAP
(tomorrow?).

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven at sonycom.com
Internet: http://www.sony-europe.com/

Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619