khvcd

khvcd

[Dave Jones]

Hi Rusty,
 In profiling bootup time, we noticed something new appeared
on the graph at http://pjones.fedorapeople.org/bootchart-preinit.png
in the name of khvcd.

"wtf is this" was my first reaction.
Followed shortly by much headscratching after finding it's
created by drivers/char/hvc_console.c whose Kconfig yeilds..

config HVC_DRIVER
        bool
        help
          Users of pSeries machines that want to utilize the hvc console front-end
          module for their backend console driver should select this option.

Which is a little confusing for those of us not actually using a pSeries.
The above profile came from a x86 box.
A little more grepping around revealed that lguest select's HVC_DRIVER,
thus sneaking this config option into the build.

asides from the needed Kconfig clarification, is it necessary for khvcd
to run at all if the lguest module hasn't been loaded ?  Is it possible
we can somehow delay creation of the kthread ?

	Dave

-- 
http://www.codemonkey.org.uk

[PATCH] Only initialize hvc_console if needed, cleanup Kconfig help

[Rusty Russell]

On Thu, 2007-08-16 at 16:16 -0400, Dave Jones wrote:
> asides from the needed Kconfig clarification, is it necessary for khvcd
> to run at all if the lguest module hasn't been loaded ?  Is it possible
> we can somehow delay creation of the kthread ?

It's for lguest guests, so it kind of needs to be built-in.  But yes, it
should be made to suck less.  Xen will want it too.

How's this?
==
This changes hvc_init() to be called only when someone actually uses
the hvc_console driver.

hvc_console used to only be for Power aka pSeries: now lguest and Xen
both want it built-in in case the kernel is a guest under one of
those, even though usually it will be a native boot.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff -r 0730da2377be drivers/char/Kconfig
--- a/drivers/char/Kconfig	Tue Aug 14 12:46:08 2007 +1000
+++ b/drivers/char/Kconfig	Fri Aug 17 09:05:12 2007 +1000
@@ -568,10 +568,10 @@ config HVC_DRIVER
 config HVC_DRIVER
 	bool
 	help
-	  Users of pSeries machines that want to utilize the hvc console front-end
-	  module for their backend console driver should select this option.
-	  It will automatically be selected if one of the back-end console drivers
-	  is selected.
+	  Generic "hypervisor virtual console" infrastructure for various
+	  hypervisors (pSeries, Xen, lguest).
+	  It will automatically be selected if one of those back-end
+	  console drivers is selected.
 
 
 config HVC_CONSOLE
diff -r 0730da2377be drivers/char/hvc_console.c
--- a/drivers/char/hvc_console.c	Tue Aug 14 12:46:08 2007 +1000
+++ b/drivers/char/hvc_console.c	Fri Aug 17 09:15:24 2007 +1000
@@ -68,6 +68,8 @@ static struct task_struct *hvc_task;
 
 /* Picks up late kicks after list walk but before schedule() */
 static int hvc_kicked;
+
+static int hvc_init(void);
 
 #ifdef CONFIG_MAGIC_SYSRQ
 static int sysrq_pressed;
@@ -754,6 +756,13 @@ struct hvc_struct __devinit *hvc_alloc(u
 	struct hvc_struct *hp;
 	int i;
 
+	/* We wait until a driver actually comes along */
+	if (!hvc_driver) {
+		int err = hvc_init();
+		if (err)
+			return ERR_PTR(err);
+	}
+
 	hp = kmalloc(ALIGN(sizeof(*hp), sizeof(long)) + outbuf_size,
 			GFP_KERNEL);
 	if (!hp)
@@ -829,16 +838,18 @@ int __devexit hvc_remove(struct hvc_stru
 	return 0;
 }
 
-/* Driver initialization.  Follow console initialization.  This is where the TTY
- * interfaces start to become available. */
-static int __init hvc_init(void)
+/* Driver initialization: called as soon as someone uses hvc_alloc(). */
+static int hvc_init(void)
 {
 	struct tty_driver *drv;
+	int err;
 
 	/* We need more than hvc_count adapters due to hotplug additions. */
 	drv = alloc_tty_driver(HVC_ALLOC_TTY_ADAPTERS);
-	if (!drv)
-		return -ENOMEM;
+	if (!drv) {
+		err = -ENOMEM;
+		goto out;
+	}
 
 	drv->owner = THIS_MODULE;
 	drv->driver_name = "hvc";
@@ -854,30 +865,43 @@ static int __init hvc_init(void)
 	 * added later. */
 	hvc_task = kthread_run(khvcd, NULL, "khvcd");
 	if (IS_ERR(hvc_task)) {
-		panic("Couldn't create kthread for console.\n");
-		put_tty_driver(drv);
-		return -EIO;
-	}
-
-	if (tty_register_driver(drv))
-		panic("Couldn't register hvc console driver\n");
-
+		printk(KERN_ERR "Couldn't create kthread for console.\n");
+		err = PTR_ERR(hvc_task);
+		goto put_tty;
+	}
+
+	err = tty_register_driver(drv);
+	if (err) {
+		printk(KERN_ERR "Couldn't register hvc console driver\n");
+		goto stop_thread;
+	}
+
+	/* FIXME: This mb() seems completely random.  Remove it. */
 	mb();
 	hvc_driver = drv;
 	return 0;
-}
-module_init(hvc_init);
+
+put_tty:
+	put_tty_driver(hvc_driver);
+stop_thread:
+	kthread_stop(hvc_task);
+	hvc_task = NULL;
+out:
+	return err;
+}
 
 /* This isn't particularly necessary due to this being a console driver
  * but it is nice to be thorough.
  */
 static void __exit hvc_exit(void)
 {
-	kthread_stop(hvc_task);
-
-	tty_unregister_driver(hvc_driver);
-	/* return tty_struct instances allocated in hvc_init(). */
-	put_tty_driver(hvc_driver);
-	unregister_console(&hvc_con_driver);
+	if (hvc_driver) {
+		kthread_stop(hvc_task);
+
+		tty_unregister_driver(hvc_driver);
+		/* return tty_struct instances allocated in hvc_init(). */
+		put_tty_driver(hvc_driver);
+		unregister_console(&hvc_con_driver);
+	}
 }
 module_exit(hvc_exit);

Re: [PATCH] Only initialize hvc_console if needed, cleanup Kconfig help

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]

On Fri, 17 Aug 2007 11:25:46 +1000 Rusty Russell <rusty@rustcorp.com.au>
wrote:
>
> diff -r 0730da2377be drivers/char/Kconfig
> --- a/drivers/char/Kconfig	Tue Aug 14 12:46:08 2007 +1000
> +++ b/drivers/char/Kconfig	Fri Aug 17 09:05:12 2007 +1000
> @@ -568,10 +568,10 @@ config HVC_DRIVER
>  config HVC_DRIVER
>  	bool
>  	help
> -	  Users of pSeries machines that want to utilize the hvc console front-end
> -	  module for their backend console driver should select this option.
> -	  It will automatically be selected if one of the back-end console drivers
> -	  is selected.
> +	  Generic "hypervisor virtual console" infrastructure for various
> +	  hypervisors (pSeries, Xen, lguest).
                      ^^^^^^^^^^^^^^^^^^^^^^^
You left out iSeries.  Probably better to just not enumerate them as they
may increase over time.

Otherwise looks good (though I haven't actually built/booted it).
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]