From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bartlomiej Zolnierkiewicz Date: Thu, 09 Aug 2018 10:03:09 +0000 Subject: Re: [PATCH fix for 4.19] fbcon: Do not takeover the console from atomic context Message-Id: <1667652.5c9CXM81DN@amdc3058> List-Id: References: <20180806155416.15752-1-hdegoede@redhat.com> In-Reply-To: <20180806155416.15752-1-hdegoede@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Hans de Goede Cc: linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org 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 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. > --- > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bartlomiej Zolnierkiewicz Subject: Re: [PATCH fix for 4.19] fbcon: Do not takeover the console from atomic context Date: Thu, 09 Aug 2018 12:03:09 +0200 Message-ID: <1667652.5c9CXM81DN@amdc3058> References: <20180806155416.15752-1-hdegoede@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) by gabe.freedesktop.org (Postfix) with ESMTPS id F16C08970B for ; Thu, 9 Aug 2018 10:03:13 +0000 (UTC) In-reply-to: <20180806155416.15752-1-hdegoede@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Hans de Goede Cc: linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org T24gTW9uZGF5LCBBdWd1c3QgMDYsIDIwMTggMDU6NTQ6MTYgUE0gSGFucyBkZSBHb2VkZSB3cm90 ZToKPiBUYWtpbmcgb3ZlciB0aGUgY29uc29sZSBpbnZvbHZlcyBhbGxvY2F0aW5nIG1lbSB3aXRo IEdGUF9LRVJORUwsIHRhbGtpbmcKPiB0byBkcm0gZHJpdmVycywgZXRjLiBTbyB0aGlzIHNob3Vs ZCBub3QgYmUgZG9uZSBmcm9tIGFuIGF0b21pYyBjb250ZXh0Lgo+IAo+IEJ1dCB0aGUgY29uc29s ZS1vdXRwdXQgdHJpZ2dlciBkZWZlcnJlZCBjb25zb2xlIHRha2VvdmVyIG1heSBoYXBwZW4gZnJv bSBhbgo+IGF0b21pYyBjb250ZXh0LCB3aGljaCBsZWFkcyB0byAiQlVHOiBzbGVlcGluZyBmdW5j dGlvbiBjYWxsZWQgZnJvbSBpbnZhbGlkCj4gY29udGV4dCIgZXJyb3JzLgo+IAo+IFRoaXMgY29t bWl0IGZpeGVzIHRoZXNlIGVycm9ycyBieSBkb2luZyB0aGUgZGVmZXJyZWQgdGFrZW92ZXIgZnJv bSBhCj4gd29ya3F1ZXVlIHdoZW4gdGhlIG5vdGlmaWVyIHJ1bnMgZnJvbSBhbiBhdG9taWMgY29u dGV4dC4KPiAKPiBTaWduZWQtb2ZmLWJ5OiBIYW5zIGRlIEdvZWRlIDxoZGVnb2VkZUByZWRoYXQu Y29tPgoKY2hlY2twYXRjaC5wbCBjb21wbGFpbnMgYWJvdXQgaW5fYXRvbWljIHVzZToKCkVSUk9S OiBkbyBub3QgdXNlIGluX2F0b21pYyBpbiBkcml2ZXJzCiM1MTogRklMRTogZHJpdmVycy92aWRl by9mYmRldi9jb3JlL2ZiY29uLmM6MzYyMzoKKyAgICAgICBpZiAoaW5fYXRvbWljKCkgfHwgaXJx c19kaXNhYmxlZCgpKSB7CgpwbGVhc2UgYWxzbyBzZWUgdGhlIGNvbW1lbnQgaW4gcHJlZW1wdC5o OgoKLyoKICogQXJlIHdlIHJ1bm5pbmcgaW4gYXRvbWljIGNvbnRleHQ/ICBXQVJOSU5HOiB0aGlz IG1hY3JvIGNhbm5vdAogKiBhbHdheXMgZGV0ZWN0IGF0b21pYyBjb250ZXh0OyBpbiBwYXJ0aWN1 bGFyLCBpdCBjYW5ub3Qga25vdyBhYm91dAogKiBoZWxkIHNwaW5sb2NrcyBpbiBub24tcHJlZW1w dGlibGUga2VybmVscy4gIFRodXMgaXQgc2hvdWxkIG5vdCBiZQogKiB1c2VkIGluIHRoZSBnZW5l cmFsIGNhc2UgdG8gZGV0ZXJtaW5lIHdoZXRoZXIgc2xlZXBpbmcgaXMgcG9zc2libGUuCiAqIERv IG5vdCB1c2UgaW5fYXRvbWljKCkgaW4gZHJpdmVyIGNvZGUuCiAqLwojZGVmaW5lIGluX2F0b21p YygpCShwcmVlbXB0X2NvdW50KCkgIT0gMCkKClRoZXJlZm9yZSBwbGVhc2UgZXhwbGFpbiB3aHkg aXQgaXMgZmluZSB0byB1c2UgaW5fYXRvbWljIGluIGZiY29uJ3MgY2FzZS4KCj4gLS0tCj4gIGRy aXZlcnMvdmlkZW8vZmJkZXYvY29yZS9mYmNvbi5jIHwgMjEgKysrKysrKysrKysrKysrKysrKy0t Cj4gIDEgZmlsZSBjaGFuZ2VkLCAxOSBpbnNlcnRpb25zKCspLCAyIGRlbGV0aW9ucygtKQo+IAo+ IGRpZmYgLS1naXQgYS9kcml2ZXJzL3ZpZGVvL2ZiZGV2L2NvcmUvZmJjb24uYyBiL2RyaXZlcnMv dmlkZW8vZmJkZXYvY29yZS9mYmNvbi5jCj4gaW5kZXggZWY4YjJkMGI3MDcxLi40ZTU5OTdkNTNm YzQgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy92aWRlby9mYmRldi9jb3JlL2ZiY29uLmMKPiArKysg Yi9kcml2ZXJzL3ZpZGVvL2ZiZGV2L2NvcmUvZmJjb24uYwo+IEBAIC0zNTkyLDcgKzM1OTIsMjAg QEAgc3RhdGljIGludCBmYmNvbl9pbml0X2RldmljZSh2b2lkKQo+ICB9Cj4gIAo+ICAjaWZkZWYg Q09ORklHX0ZSQU1FQlVGRkVSX0NPTlNPTEVfREVGRVJSRURfVEFLRU9WRVIKPiArc3RhdGljIHZv aWQgZmJjb25fcmVnaXN0ZXJfZXhpc3RpbmdfZmJzKHN0cnVjdCB3b3JrX3N0cnVjdCAqd29yaykK PiArewo+ICsJaW50IGk7Cj4gKwo+ICsJY29uc29sZV9sb2NrKCk7Cj4gKwo+ICsJZm9yX2VhY2hf cmVnaXN0ZXJlZF9mYihpKQo+ICsJCWZiY29uX2ZiX3JlZ2lzdGVyZWQocmVnaXN0ZXJlZF9mYltp XSk7Cj4gKwo+ICsJY29uc29sZV91bmxvY2soKTsKPiArfQo+ICsKPiAgc3RhdGljIHN0cnVjdCBu b3RpZmllcl9ibG9jayBmYmNvbl9vdXRwdXRfbmI7Cj4gK3N0YXRpYyBERUNMQVJFX1dPUksoZmJj b25fZGVmZXJyZWRfdGFrZW92ZXJfd29yaywgZmJjb25fcmVnaXN0ZXJfZXhpc3RpbmdfZmJzKTsK PiAgCj4gIHN0YXRpYyBpbnQgZmJjb25fb3V0cHV0X25vdGlmaWVyKHN0cnVjdCBub3RpZmllcl9i bG9jayAqbmIsCj4gIAkJCQkgdW5zaWduZWQgbG9uZyBhY3Rpb24sIHZvaWQgKmRhdGEpCj4gQEAg LTM2MDcsOCArMzYyMCwxMiBAQCBzdGF0aWMgaW50IGZiY29uX291dHB1dF9ub3RpZmllcihzdHJ1 Y3Qgbm90aWZpZXJfYmxvY2sgKm5iLAo+ICAJZGVmZXJyZWRfdGFrZW92ZXIgPSBmYWxzZTsKPiAg CWxvZ29fc2hvd24gPSBGQkNPTl9MT0dPX0RPTlRTSE9XOwo+ICAKPiAtCWZvcl9lYWNoX3JlZ2lz dGVyZWRfZmIoaSkKPiAtCQlmYmNvbl9mYl9yZWdpc3RlcmVkKHJlZ2lzdGVyZWRfZmJbaV0pOwo+ ICsJaWYgKGluX2F0b21pYygpIHx8IGlycXNfZGlzYWJsZWQoKSkgewo+ICsJCXNjaGVkdWxlX3dv cmsoJmZiY29uX2RlZmVycmVkX3Rha2VvdmVyX3dvcmspOwo+ICsJfSBlbHNlIHsKPiArCQlmb3Jf ZWFjaF9yZWdpc3RlcmVkX2ZiKGkpCj4gKwkJCWZiY29uX2ZiX3JlZ2lzdGVyZWQocmVnaXN0ZXJl ZF9mYltpXSk7Cj4gKwl9Cj4gIAo+ICAJcmV0dXJuIE5PVElGWV9PSzsKPiAgfQoKQmVzdCByZWdh cmRzLAotLQpCYXJ0bG9taWVqIFpvbG5pZXJraWV3aWN6ClNhbXN1bmcgUiZEIEluc3RpdHV0ZSBQ b2xhbmQKU2Ftc3VuZyBFbGVjdHJvbmljcwoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMu ZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlz dGluZm8vZHJpLWRldmVsCg==