| Message ID | 20180810112757.3936-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] fbcon: Do not takeover the console from atomic context | expand |
On Friday, August 10, 2018 01:27:57 PM Hans de Goede wrote: > Taking over the console involves allocating mem with GFP_KERNEL, talking > to drm drivers, etc. So this should not be done from an atomic context. > > But the console-output trigger deferred console takeover may happen from an > atomic context, which leads to "BUG: sleeping function called from invalid > context" errors. > > This commit fixes these errors by doing the deferred takeover from a > workqueue. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Patch queued for 4.19, thanks. > @@ -3607,8 +3620,8 @@ static int fbcon_output_notifier(struct notifier_block *nb, > deferred_takeover = false; > logo_shown = FBCON_LOGO_DONTSHOW; > > - for_each_registered_fb(i) > - fbcon_fb_registered(registered_fb[i]); > + /* We may get called in atomic context */ > + schedule_work(&fbcon_deferred_takeover_work); After above change gcc now complains about unused variable: drivers/video/fbdev/core/fbcon.c: In function ‘fbcon_output_notifier’: drivers/video/fbdev/core/fbcon.c:3613:6: warning: unused variable ‘i’ [-Wunused-variable] int i; ^ I fixed this while applying the patch. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Hi, On 10-08-18 17:22, Bartlomiej Zolnierkiewicz wrote: > On Friday, August 10, 2018 01:27:57 PM Hans de Goede wrote: >> Taking over the console involves allocating mem with GFP_KERNEL, talking >> to drm drivers, etc. So this should not be done from an atomic context. >> >> But the console-output trigger deferred console takeover may happen from an >> atomic context, which leads to "BUG: sleeping function called from invalid >> context" errors. >> >> This commit fixes these errors by doing the deferred takeover from a >> workqueue. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > Patch queued for 4.19, thanks. > >> @@ -3607,8 +3620,8 @@ static int fbcon_output_notifier(struct notifier_block *nb, >> deferred_takeover = false; >> logo_shown = FBCON_LOGO_DONTSHOW; >> >> - for_each_registered_fb(i) >> - fbcon_fb_registered(registered_fb[i]); >> + /* We may get called in atomic context */ >> + schedule_work(&fbcon_deferred_takeover_work); > > After above change gcc now complains about unused variable: > > drivers/video/fbdev/core/fbcon.c: In function ‘fbcon_output_notifier’: > drivers/video/fbdev/core/fbcon.c:3613:6: warning: unused variable ‘i’ [-Wunused-variable] > int i; > ^ > > I fixed this while applying the patch. Ah yes, thank you for fixing this, Regards, Hans
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index ef8b2d0b7071..5581dec102aa 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -3592,7 +3592,20 @@ static int fbcon_init_device(void) } #ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER +static void fbcon_register_existing_fbs(struct work_struct *work) +{ + int i; + + console_lock(); + + for_each_registered_fb(i) + fbcon_fb_registered(registered_fb[i]); + + console_unlock(); +} + static struct notifier_block fbcon_output_nb; +static DECLARE_WORK(fbcon_deferred_takeover_work, fbcon_register_existing_fbs); static int fbcon_output_notifier(struct notifier_block *nb, unsigned long action, void *data) @@ -3607,8 +3620,8 @@ static int fbcon_output_notifier(struct notifier_block *nb, deferred_takeover = false; logo_shown = FBCON_LOGO_DONTSHOW; - for_each_registered_fb(i) - fbcon_fb_registered(registered_fb[i]); + /* We may get called in atomic context */ + schedule_work(&fbcon_deferred_takeover_work); return NOTIFY_OK; }
Taking over the console involves allocating mem with GFP_KERNEL, talking to drm drivers, etc. So this should not be done from an atomic context. But the console-output trigger deferred console takeover may happen from an atomic context, which leads to "BUG: sleeping function called from invalid context" errors. This commit fixes these errors by doing the deferred takeover from a workqueue. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v2: -Add a comment fbcon.c and the commit message about why we need to use in_atomic here, no functional changes Changes in v3: -It is not possible to reliable detect if we're running in an atomic context, simply always do the takeover from a worker --- drivers/video/fbdev/core/fbcon.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)