[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 6 Jun 2014 09:24:35 +0200
From: David Herrmann <dh.herrmann@...il.com>
To: Daniel Vetter <daniel.vetter@...ll.ch>
Cc: Intel Graphics Development <intel-gfx@...ts.freedesktop.org>,
Jiri Slaby <jslaby@...e.cz>,
LKML <linux-kernel@...r.kernel.org>,
DRI Development <dri-devel@...ts.freedesktop.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH 2/5] vt: Fix up unregistration of vt drivers
Hi
On Thu, Jun 5, 2014 at 4:58 PM, Daniel Vetter <daniel.vetter@...ll.ch> wrote:
> A bunch of issues:
> - We should not kick out the default console (which is tracked in
> conswitchp), so check for that.
> - Add better error codes so callers can differentiate between "something
> went wrong" and "your driver isn't registered already". i915 needs
> that so it doesn't fall over when reloading the driver and hence
> vga_con is already unregistered.
> - There's a mess with the driver flags: What we need to check for is
> that the driver isn't used any more, i.e. unbound completely (FLAG_INIT).
> And not whether it's the boot console or not (which is the only one
> which doesn't have FLAG_MODULE). Otherwise there's no way to kick
> out the boot console, which i915 wants to do to prevent havoc with
> vga_con interferring (which tends to hang machines).
>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: Jiri Slaby <jslaby@...e.cz>
> Signed-off-by: Daniel Vetter <daniel.vetter@...ll.ch>
> ---
> drivers/tty/vt/vt.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index ea600f482eeb..5077fe87324d 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -3573,17 +3573,20 @@ err:
> */
> int do_unregister_con_driver(const struct consw *csw)
> {
> - int i, retval = -ENODEV;
> + int i;
>
> /* cannot unregister a bound driver */
> if (con_is_bound(csw))
> - goto err;
> + return -EBUSY;
> +
> + if (csw == conswitchp)
> + return -EINVAL;
Ugh, that fix is correct, but I'd rather like to see
do_unbind_con_driver() do the right thing. It currently resets
conswitchp _only_ if the new fallback is unbound. Why not _always_ set
conswitchp to defcsw _iff_ conswitchp == csw there?
This way, you _know_ here that if !con_is_bound(csw), then csw != conswitchp.
>
> for (i = 0; i < MAX_NR_CON_DRIVER; i++) {
> struct con_driver *con_driver = ®istered_con_driver[i];
>
> if (con_driver->con == csw &&
> - con_driver->flag & CON_DRIVER_FLAG_MODULE) {
> + con_driver->flag & CON_DRIVER_FLAG_INIT) {
That makes FLAG_MODULE almost a no-op except for ->unbind(). I wonder
why FLAG_MODULE exists, anyway.
Otherwise looks good.
Thanks
David
> vtconsole_deinit_device(con_driver);
> device_destroy(vtconsole_class,
> MKDEV(0, con_driver->node));
> @@ -3594,12 +3597,11 @@ int do_unregister_con_driver(const struct consw *csw)
> con_driver->flag = 0;
> con_driver->first = 0;
> con_driver->last = 0;
> - retval = 0;
> - break;
> + return 0;
> }
> }
> -err:
> - return retval;
> +
> + return -ENODEV;
> }
> EXPORT_SYMBOL_GPL(do_unregister_con_driver);
>
> --
> 1.8.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists