From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bartlomiej Zolnierkiewicz Date: Wed, 25 Jul 2018 10:56:14 +0000 Subject: Re: [PATCH] fb: fix lost console when the user unplugs a USB adapter Message-Id: <6583210.oaMISO8Dcr@amdc3058> List-Id: References: <7861c063-6494-7cd8-683d-02c61f9faa1f@tronnes.org> In-Reply-To: <7861c063-6494-7cd8-683d-02c61f9faa1f@tronnes.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: Noralf =?ISO-8859-1?Q?Tr=F8nnes?= Cc: linux-fbdev@vger.kernel.org, Ladislav Michl , Bernie Thompson , dri-devel@lists.freedesktop.org, Mikulas Patocka , Dave Airlie On Wednesday, July 04, 2018 05:31:19 PM Noralf Tr=F8nnes wrote: >=20 > Den 03.07.2018 19.18, skrev Mikulas Patocka: > > > > On Tue, 3 Jul 2018, Bartlomiej Zolnierkiewicz wrote: > > > >> Hi, > >> > >> On Sunday, June 03, 2018 11:46:29 AM Mikulas Patocka wrote: > >>> I have a USB display adapter using the udlfb driver and I use it on a= n ARM > >>> board that doesn't have any graphics card. When I plug the adapter in= , the > >>> console is properly displayed, however when I unplug and re-plug the > >>> adapter, the console is not displayed and I can't access it until I r= eboot > >>> the board. > >>> > >>> The reason is this: > >>> When the adapter is unplugged, dlfb_usb_disconnect calls > >>> unlink_framebuffer, then it waits until the reference count drops to = zero > >>> and then it deallocates the framebuffer. However, the console that is > >>> attached to the framebuffer device keeps the reference count non-zero= , so > >>> the framebuffer device is never destroyed. When the USB adapter is pl= ugged > >>> again, it creates a new device /dev/fb1 and the console is not attach= ed to > >>> it. > >>> > >>> This patch fixes the bug by unbinding the console from unlink_framebu= ffer. > >>> The code to unbind the console is moved from do_unregister_framebuffe= r to > >>> a function unbind_console. When the console is unbound, the reference > >>> count drops to zero and the udlfb driver frees the framebuffer. When = the > >>> adapter is plugged back, a new framebuffer is created and the console= is > >>> attached to it. > >>> > >>> Signed-off-by: Mikulas Patocka > >>> Cc: stable@vger.kernel.org > >> After this change unbind_console() will be called twice in the standard > >> framebuffer unregister path: > >> > >> - first time, directly by do_unregister_framebuffer() > >> > >> - second time, indirectly by do_unregister_framebuffer()->unlink_frame= buffer() > >> > >> This doesn't look correctly. > > unbind_console calls the FB_EVENT_FB_UNBIND notifier, FB_EVENT_FB_UNBIND > > goes to the function fbcon_fb_unbind and fbcon_fb_unbind checks if the > > console is bound to the framebuffer for which unbind is requested. So a > > double call won't cause any trouble. > > > >> Also why can't udlfb just use unregister_framebuffer() like all other > >> drivers (it uses unlink_framebuffer() and it is the only user of this > >> helper)? > > It uses unregister_framebuffer() - but - unregister_framebuffer() may o= nly > > be called when the open count of the framebuffer is zero. >=20 > AFAIU calling unregister_framebuffer() with open fd's is just fine as > long as fb_info with buffers stay intact. All it does is to remove the > fbX from userspace. Cleanup can be done in fb_ops->fb_destroy. >=20 > I have been working on generic fbdev emulation for DRM [1] and I did a > test now to see what would happen if I did unbind the driver from the > device. It worked as expected if I didn't have another fbdev present, > but if there is an fb0 and I remove fb1 with a console on it, I would > sometimes get crashes, often with a call to cursor_timer_handler() in > the traceback. >=20 > I think there's index mixup in fbcon_fb_unbind(), at least this change > seems to solve the immediate problem: >=20 > diff --git a/drivers/video/fbdev/core/fbcon.c=20 > b/drivers/video/fbdev/core/fbcon.c > index 5fb156bdcf4e..271b9b988b73 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -3066,7 +3072,7 @@ static int fbcon_fb_unbind(int idx) > for (i =3D first_fb_vc; i <=3D last_fb_vc; i++) { > if (con2fb_map[i] !=3D idx && > con2fb_map[i] !=3D -1) { > - new_idx =3D i; > + new_idx =3D con2fb_map[i]; > break; > } > } Looks fine, could you please submit this as a proper patch (with S-o-b tag and patch description)? Thanks. 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] fb: fix lost console when the user unplugs a USB adapter Date: Wed, 25 Jul 2018 12:56:14 +0200 Message-ID: <6583210.oaMISO8Dcr@amdc3058> References: <7861c063-6494-7cd8-683d-02c61f9faa1f@tronnes.org> 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 BC59C6E07D for ; Wed, 25 Jul 2018 10:56:17 +0000 (UTC) In-reply-to: <7861c063-6494-7cd8-683d-02c61f9faa1f@tronnes.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Noralf =?ISO-8859-1?Q?Tr=F8nnes?= Cc: linux-fbdev@vger.kernel.org, Ladislav Michl , Bernie Thompson , dri-devel@lists.freedesktop.org, Mikulas Patocka , Dave Airlie List-Id: dri-devel@lists.freedesktop.org T24gV2VkbmVzZGF5LCBKdWx5IDA0LCAyMDE4IDA1OjMxOjE5IFBNIE5vcmFsZiBUcsO4bm5lcyB3 cm90ZToKPiAKPiBEZW4gMDMuMDcuMjAxOCAxOS4xOCwgc2tyZXYgTWlrdWxhcyBQYXRvY2thOgo+ ID4KPiA+IE9uIFR1ZSwgMyBKdWwgMjAxOCwgQmFydGxvbWllaiBab2xuaWVya2lld2ljeiB3cm90 ZToKPiA+Cj4gPj4gSGksCj4gPj4KPiA+PiBPbiBTdW5kYXksIEp1bmUgMDMsIDIwMTggMTE6NDY6 MjkgQU0gTWlrdWxhcyBQYXRvY2thIHdyb3RlOgo+ID4+PiBJIGhhdmUgYSBVU0IgZGlzcGxheSBh ZGFwdGVyIHVzaW5nIHRoZSB1ZGxmYiBkcml2ZXIgYW5kIEkgdXNlIGl0IG9uIGFuIEFSTQo+ID4+ PiBib2FyZCB0aGF0IGRvZXNuJ3QgaGF2ZSBhbnkgZ3JhcGhpY3MgY2FyZC4gV2hlbiBJIHBsdWcg dGhlIGFkYXB0ZXIgaW4sIHRoZQo+ID4+PiBjb25zb2xlIGlzIHByb3Blcmx5IGRpc3BsYXllZCwg aG93ZXZlciB3aGVuIEkgdW5wbHVnIGFuZCByZS1wbHVnIHRoZQo+ID4+PiBhZGFwdGVyLCB0aGUg Y29uc29sZSBpcyBub3QgZGlzcGxheWVkIGFuZCBJIGNhbid0IGFjY2VzcyBpdCB1bnRpbCBJIHJl Ym9vdAo+ID4+PiB0aGUgYm9hcmQuCj4gPj4+Cj4gPj4+IFRoZSByZWFzb24gaXMgdGhpczoKPiA+ Pj4gV2hlbiB0aGUgYWRhcHRlciBpcyB1bnBsdWdnZWQsIGRsZmJfdXNiX2Rpc2Nvbm5lY3QgY2Fs bHMKPiA+Pj4gdW5saW5rX2ZyYW1lYnVmZmVyLCB0aGVuIGl0IHdhaXRzIHVudGlsIHRoZSByZWZl cmVuY2UgY291bnQgZHJvcHMgdG8gemVybwo+ID4+PiBhbmQgdGhlbiBpdCBkZWFsbG9jYXRlcyB0 aGUgZnJhbWVidWZmZXIuIEhvd2V2ZXIsIHRoZSBjb25zb2xlIHRoYXQgaXMKPiA+Pj4gYXR0YWNo ZWQgdG8gdGhlIGZyYW1lYnVmZmVyIGRldmljZSBrZWVwcyB0aGUgcmVmZXJlbmNlIGNvdW50IG5v bi16ZXJvLCBzbwo+ID4+PiB0aGUgZnJhbWVidWZmZXIgZGV2aWNlIGlzIG5ldmVyIGRlc3Ryb3ll ZC4gV2hlbiB0aGUgVVNCIGFkYXB0ZXIgaXMgcGx1Z2dlZAo+ID4+PiBhZ2FpbiwgaXQgY3JlYXRl cyBhIG5ldyBkZXZpY2UgL2Rldi9mYjEgYW5kIHRoZSBjb25zb2xlIGlzIG5vdCBhdHRhY2hlZCB0 bwo+ID4+PiBpdC4KPiA+Pj4KPiA+Pj4gVGhpcyBwYXRjaCBmaXhlcyB0aGUgYnVnIGJ5IHVuYmlu ZGluZyB0aGUgY29uc29sZSBmcm9tIHVubGlua19mcmFtZWJ1ZmZlci4KPiA+Pj4gVGhlIGNvZGUg dG8gdW5iaW5kIHRoZSBjb25zb2xlIGlzIG1vdmVkIGZyb20gZG9fdW5yZWdpc3Rlcl9mcmFtZWJ1 ZmZlciB0bwo+ID4+PiBhIGZ1bmN0aW9uIHVuYmluZF9jb25zb2xlLiBXaGVuIHRoZSBjb25zb2xl IGlzIHVuYm91bmQsIHRoZSByZWZlcmVuY2UKPiA+Pj4gY291bnQgZHJvcHMgdG8gemVybyBhbmQg dGhlIHVkbGZiIGRyaXZlciBmcmVlcyB0aGUgZnJhbWVidWZmZXIuIFdoZW4gdGhlCj4gPj4+IGFk YXB0ZXIgaXMgcGx1Z2dlZCBiYWNrLCBhIG5ldyBmcmFtZWJ1ZmZlciBpcyBjcmVhdGVkIGFuZCB0 aGUgY29uc29sZSBpcwo+ID4+PiBhdHRhY2hlZCB0byBpdC4KPiA+Pj4KPiA+Pj4gU2lnbmVkLW9m Zi1ieTogTWlrdWxhcyBQYXRvY2thIDxtcGF0b2NrYUByZWRoYXQuY29tPgo+ID4+PiBDYzogc3Rh YmxlQHZnZXIua2VybmVsLm9yZwo+ID4+IEFmdGVyIHRoaXMgY2hhbmdlIHVuYmluZF9jb25zb2xl KCkgd2lsbCBiZSBjYWxsZWQgdHdpY2UgaW4gdGhlIHN0YW5kYXJkCj4gPj4gZnJhbWVidWZmZXIg dW5yZWdpc3RlciBwYXRoOgo+ID4+Cj4gPj4gLSBmaXJzdCB0aW1lLCBkaXJlY3RseSBieSBkb191 bnJlZ2lzdGVyX2ZyYW1lYnVmZmVyKCkKPiA+Pgo+ID4+IC0gc2Vjb25kIHRpbWUsIGluZGlyZWN0 bHkgYnkgZG9fdW5yZWdpc3Rlcl9mcmFtZWJ1ZmZlcigpLT51bmxpbmtfZnJhbWVidWZmZXIoKQo+ ID4+Cj4gPj4gVGhpcyBkb2Vzbid0IGxvb2sgY29ycmVjdGx5Lgo+ID4gdW5iaW5kX2NvbnNvbGUg Y2FsbHMgdGhlIEZCX0VWRU5UX0ZCX1VOQklORCBub3RpZmllciwgRkJfRVZFTlRfRkJfVU5CSU5E Cj4gPiBnb2VzIHRvIHRoZSBmdW5jdGlvbiBmYmNvbl9mYl91bmJpbmQgYW5kIGZiY29uX2ZiX3Vu YmluZCBjaGVja3MgaWYgdGhlCj4gPiBjb25zb2xlIGlzIGJvdW5kIHRvIHRoZSBmcmFtZWJ1ZmZl ciBmb3Igd2hpY2ggdW5iaW5kIGlzIHJlcXVlc3RlZC4gU28gYQo+ID4gZG91YmxlIGNhbGwgd29u J3QgY2F1c2UgYW55IHRyb3VibGUuCj4gPgo+ID4+IEFsc28gd2h5IGNhbid0IHVkbGZiIGp1c3Qg dXNlIHVucmVnaXN0ZXJfZnJhbWVidWZmZXIoKSBsaWtlIGFsbCBvdGhlcgo+ID4+IGRyaXZlcnMg KGl0IHVzZXMgdW5saW5rX2ZyYW1lYnVmZmVyKCkgYW5kIGl0IGlzIHRoZSBvbmx5IHVzZXIgb2Yg dGhpcwo+ID4+IGhlbHBlcik/Cj4gPiBJdCB1c2VzIHVucmVnaXN0ZXJfZnJhbWVidWZmZXIoKSAt IGJ1dCAtIHVucmVnaXN0ZXJfZnJhbWVidWZmZXIoKSBtYXkgb25seQo+ID4gYmUgY2FsbGVkIHdo ZW4gdGhlIG9wZW4gY291bnQgb2YgdGhlIGZyYW1lYnVmZmVyIGlzIHplcm8uCj4gCj4gQUZBSVUg Y2FsbGluZyB1bnJlZ2lzdGVyX2ZyYW1lYnVmZmVyKCkgd2l0aCBvcGVuIGZkJ3MgaXMganVzdCBm aW5lIGFzCj4gbG9uZyBhcyBmYl9pbmZvIHdpdGggYnVmZmVycyBzdGF5IGludGFjdC4gQWxsIGl0 IGRvZXMgaXMgdG8gcmVtb3ZlIHRoZQo+IGZiWCBmcm9tIHVzZXJzcGFjZS4gQ2xlYW51cCBjYW4g YmUgZG9uZSBpbiBmYl9vcHMtPmZiX2Rlc3Ryb3kuCj4gCj4gSSBoYXZlIGJlZW4gd29ya2luZyBv biBnZW5lcmljIGZiZGV2IGVtdWxhdGlvbiBmb3IgRFJNIFsxXSBhbmQgSSBkaWQgYQo+IHRlc3Qg bm93IHRvIHNlZSB3aGF0IHdvdWxkIGhhcHBlbiBpZiBJIGRpZCB1bmJpbmQgdGhlIGRyaXZlciBm cm9tIHRoZQo+IGRldmljZS4gSXQgd29ya2VkIGFzIGV4cGVjdGVkIGlmIEkgZGlkbid0IGhhdmUg YW5vdGhlciBmYmRldiBwcmVzZW50LAo+IGJ1dCBpZiB0aGVyZSBpcyBhbiBmYjAgYW5kIEkgcmVt b3ZlIGZiMSB3aXRoIGEgY29uc29sZSBvbiBpdCwgSSB3b3VsZAo+IHNvbWV0aW1lcyBnZXQgY3Jh c2hlcywgb2Z0ZW4gd2l0aCBhIGNhbGwgdG8gY3Vyc29yX3RpbWVyX2hhbmRsZXIoKSBpbgo+IHRo ZSB0cmFjZWJhY2suCj4gCj4gSSB0aGluayB0aGVyZSdzIGluZGV4IG1peHVwIGluIGZiY29uX2Zi X3VuYmluZCgpLCBhdCBsZWFzdCB0aGlzIGNoYW5nZQo+IHNlZW1zIHRvIHNvbHZlIHRoZSBpbW1l ZGlhdGUgcHJvYmxlbToKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy92aWRlby9mYmRldi9jb3Jl L2ZiY29uLmMgCj4gYi9kcml2ZXJzL3ZpZGVvL2ZiZGV2L2NvcmUvZmJjb24uYwo+IGluZGV4IDVm YjE1NmJkY2Y0ZS4uMjcxYjliOTg4YjczIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvdmlkZW8vZmJk ZXYvY29yZS9mYmNvbi5jCj4gKysrIGIvZHJpdmVycy92aWRlby9mYmRldi9jb3JlL2ZiY29uLmMK PiBAQCAtMzA2Niw3ICszMDcyLDcgQEAgc3RhdGljIGludCBmYmNvbl9mYl91bmJpbmQoaW50IGlk eCkKPiAgICAgICAgICBmb3IgKGkgPSBmaXJzdF9mYl92YzsgaSA8PSBsYXN0X2ZiX3ZjOyBpKysp IHsKPiAgICAgICAgICAgICAgICAgIGlmIChjb24yZmJfbWFwW2ldICE9IGlkeCAmJgo+ICAgICAg ICAgICAgICAgICAgICAgIGNvbjJmYl9tYXBbaV0gIT0gLTEpIHsKPiAtICAgICAgICAgICAgICAg ICAgICAgICBuZXdfaWR4ID0gaTsKPiArICAgICAgICAgICAgICAgICAgICAgICBuZXdfaWR4ID0g Y29uMmZiX21hcFtpXTsKPiAgICAgICAgICAgICAgICAgICAgICAgICAgYnJlYWs7Cj4gICAgICAg ICAgICAgICAgICB9Cj4gICAgICAgICAgfQoKTG9va3MgZmluZSwgY291bGQgeW91IHBsZWFzZSBz dWJtaXQgdGhpcyBhcyBhIHByb3BlciBwYXRjaAood2l0aCBTLW8tYiB0YWcgYW5kIHBhdGNoIGRl c2NyaXB0aW9uKT8gVGhhbmtzLgoKQmVzdCByZWdhcmRzLAotLQpCYXJ0bG9taWVqIFpvbG5pZXJr aWV3aWN6ClNhbXN1bmcgUiZEIEluc3RpdHV0ZSBQb2xhbmQKU2Ftc3VuZyBFbGVjdHJvbmljcwoK X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg==