Hi,

On 09-08-18 12:03, Bartlomiej Zolnierkiewicz wrote:

On Monday, August 06, 2018 05:54:16 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 when the notifier runs from an atomic context.

Signed-off-by: Hans de Goede hdegoede@redhat.com

checkpatch.pl complains about in_atomic use:

ERROR: do not use in_atomic in drivers #51: FILE: drivers/video/fbdev/core/fbcon.c:3623:

•   if (in_atomic() || irqs_disabled()) {
  
  

please also see the comment in preempt.h:

/*

• Are we running in atomic context? WARNING: this macro cannot
• always detect atomic context; in particular, it cannot know about
• held spinlocks in non-preemptible kernels. Thus it should not be
• used in the general case to determine whether sleeping is possible.
• Do not use in_atomic() in driver code.

*/ #define in_atomic() (preempt_count() != 0)

Therefore please explain why it is fine to use in_atomic in fbcon's case.

Ok I will send a v2 which adds a comment why we need to use in_atomic here.

Regards,

Hans

...

---

drivers/video/fbdev/core/fbcon.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index ef8b2d0b7071..4e5997d53fc4 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,12 @@ 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]);
  

• if (in_atomic() || irqs_disabled()) {

• schedule_work(&fbcon_deferred_takeover_work);
  

• } else {

• for_each_registered_fb(i)
  

• 	fbcon_fb_registered(registered_fb[i]);
  

• }

  return NOTIFY_OK; }

Best regards,

Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics