Re: Atomicity in KMS panic notifier

[Takashi Iwai <tiwai@suse.de>]

At Mon, 5 May 2014 16:29:37 +0200,
Daniel Vetter wrote:
> 
> On Mon, May 5, 2014 at 3:02 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > Hi,
> >
> > while debugging a few reported bugs, I noticed that
> > drm_fb_helper_force_kernel_mode() that is called in the KMS panic
> > notifier isn't really atomic-safe.  It invokes crtc's set_config(),
> > and all implementations seem to involve with page allocations (kmalloc
> > with GFP_KERNEL, via some ttm ops, etc).  I've actually seen the Oops
> > with cirrus KMS during panic due to this.
> >
> > Does anyone have an idea to fix this?  I thought of re-using
> > drm_fb_helper_debug_enter(), but this won't work with many drivers
> > that don't have crtc->mode_set_base_atomic(), either (yeah, this is
> > another bug).
> 
> David Herrmann has a long-term plan to address this, using a much more
> minimal panic console (so that we can avoid all the fbcon madness) and
> adding a new driver callback to deliver a pointer to whatever
> framebuffers are currently displayed. If it's possible to obtain such
> a pointer in atomic contexts. Then we'd rip out all the existing panic
> notifier and handler stuff in fbcon. We'd also need to disable the
> panic notifier fbcon registers itself (since that ends up calling down
> into our ->set_par which again ends up in ->set_config).
> 
> That should leave us with some very minimal panic code to audit.
> 
> Imo trying to fix the current mess and making ->set_config work in
> atomic contexts is pointless. drm_can_sleep is trying to make that
> possible in some ways, and it's horrible since using it means
> busy-loops in atomic contexts outside of panic handlers won't get
> reported any more. Also the interactions with the console_lock (which
> due to some bonghits is protecting almost everything in fbcon/fbdev
> nowadays) would also be almost completely removed.
> 
> Unfortunately doing all this is a lot of work.

OK, thanks for clarification!

The current problem I see is that the rest of panic notifier chain
won't be called once when we hit the problem in KMS notifier.  So,
this bug in KMS influences on the rest panic behavior.

Maybe a hackish solution would be to keep KMS notifier at the end of
notifier chain so that it crashes at last.  I don't like this either,
but...


Takashi