All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Antonino A. Daplas" <adaplas@gmail.com>
To: linux-fbdev-devel@lists.sourceforge.net
Cc: Dave Airlie <airlied@linux.ie>, Jakob Bornecrantz <wallbraker@gmail.com>
Subject: Re: Console unregistration questions
Date: Wed, 25 Apr 2007 07:19:47 +0800	[thread overview]
Message-ID: <1177456787.10465.29.camel@daplas> (raw)
In-Reply-To: <200704241459.29297.jbarnes@virtuousgeek.org>

On Tue, 2007-04-24 at 14:59 -0700, Jesse Barnes wrote:
> In doing the DRM modesetting work, we've been using the FB layer to manage 
> regular FB devices and provide a DRM based console to the user (iow DRM 
> owns the PCI device but the FB layer is used as a console and to provide 
> the familiar /dev/fb interface to existing software).
> 
> However, in doing development we noticed that when we remove the new DRM 
> based fb driver, our machines tend to hang.  This appears to be due to 
> bugs in console unregistration.
> 
> The DRM fb driver's cleanup routine is normal, I think:
> 
> int drmfb_remove(struct drm_device *dev, struct drm_framebuffer *fb)
> {
> 	struct fb_info *info = fb->fbdev;
> 	
> 	unregister_framebuffer(info);
> 	framebuffer_release(info);
> 	/* Unmap framebuffer */
> 	drm_mem_reg_iounmap(dev, &fb->bo->mem, fb->virtual_base);
> 
> 	return 0;
> }
> 
> But since we're using drmfb as a console, the unregister_framebuffer call 
> silently fails.  fbcon is builtin to the kernel, so when drmfb exits, 
> unregister_framebuffer will end up calling fbcon's fbcon_fb_unregistered() 
> function via the notifier chain.  However, when fbcon_fb_unregistered 
> calls unregister_con_driver(&fb_con) it ignores the return value, which is 
> bad since unregister_con_driver left fbcon's routines active (due to fbcon 
> still being bound to the console) even while unregister_framebuffer freed 
> their underlying structures.

Okay, we can do that. This was not done before because
unregister_framebuffer was only called on rmmod <module>.  But checking
for the return error and have it propagate to the driver should not be a
problem.

I'll send a patch to you and to akpm soon.

>   So when unregister_framebuffer returns, the 
> very next console operation causes an oops or worse (I usually see 
> hide_cursor->fbcon_cursor die when it tries to get at info->fbcon_par).
> 
> However, it doesn't appear that there's a way for fb drivers to unbind 
> themselves from the console at unload time, so we have to do it manually.  

You cannot unbind framebuffer drivers independently from the console
layer because the console is holding a reference count on them. 

FYI: this was what I did initially, but akpm objected :-).

> Should there be a way to unbind it from driver code?  Maybe by exporting a 
> wrapper to unbind_con_driver? 

Yes, you can expose unbind_con_driver(), I don't mind. You can do it
first in your tree, then when you need it for mainline, let me know.

(The kernel has someone who hunts for unused EXPORT_SYMBOLs and removes
them without remorse :-)


>  Also, shouldn't callers of 
> unregister_con_driver be checking return values?  That would have made 
> this bug a lot easier to find at least. :)
> 

Okay.

Tony




-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

  reply	other threads:[~2007-04-24 23:20 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-04-24 21:59 Console unregistration questions Jesse Barnes
2007-04-24 23:19 ` Antonino A. Daplas [this message]
2007-04-24 23:42   ` Antonino A. Daplas
2007-04-24 23:46     ` Jesse Barnes
2007-04-25  0:07       ` Antonino A. Daplas
2007-04-25  1:17       ` Antonino A. Daplas
2007-04-26 17:00         ` Jesse Barnes
2007-04-27  1:30         ` Jesse Barnes
2007-04-27  6:10           ` Antonino A. Daplas
2007-04-27 15:14             ` Jesse Barnes
2007-04-27 16:10               ` Antonino A. Daplas
2007-04-24 23:42   ` Jesse Barnes
2007-04-25  0:04     ` Antonino A. Daplas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1177456787.10465.29.camel@daplas \
    --to=adaplas@gmail.com \
    --cc=airlied@linux.ie \
    --cc=linux-fbdev-devel@lists.sourceforge.net \
    --cc=wallbraker@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.