[PATCH fix for 4.19] fbcon: Do not takeover the console from atomic context
Hans de Goede
hdegoede at redhat.com
Thu Aug 9 11:28:25 UTC 2018
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 at 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
>
More information about the dri-devel
mailing list