Patchwork powerpc/pseries: Reduce HVCS driver insanity

login
register
mail settings
Submitter Benjamin Herrenschmidt
Date Feb. 7, 2011, 4:26 a.m.
Message ID <1297052785.14982.39.camel@pasglop>
Download mbox | patch
Permalink /patch/82076/
State Accepted, archived
Commit c7704d352d45de47333f2d9f10aead820b49044c
Delegated to: Benjamin Herrenschmidt
Headers show

Comments

Benjamin Herrenschmidt - Feb. 7, 2011, 4:26 a.m.
The HVCS driver, for those who don't know, is a driver for the "server" side
of the IBM virtual terminal mechanism allowing Linux partitions to act as
terminal servers under IBM PowerVM hypervisor. It's almost never used on
the field at the moment.

However, it's part of our configs, and in its current incarnation, will
allocate the tty driver & major (with 64 minors) and create a kernel thread
whether it's used or not, ie, whether the hypervisor did put a virtual
terminal server device node in the partition or not (or whether running on
a pseries machine or not even).

This in turns causes modern distro's udev's to start trying to open all
those 64 minors at boot, which, since they aren't linked to anything,
causes the driver to spew errors in the kernel log for each of them.

Not nice.

This moves all that initialization to a function which is now only called
the first time a terminal server virtual IO device is actually probed
(that is almost never).

There's still a _LOT_ of cleanup that can be done in this driver, some
simple (almost all printk's statements in there shall either just be
removed or in some case turned into better written & more informative
messages, including using the dev_* variants etc...). This is left as
an exercise for whoever actually cares about that driver.

One could also try to be smart and dispose of all the tty related
resources when the last instance of the VIO server device
is removed (Hotplug anybody ?).

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Patch

diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
index bedc6c1..7e315b7 100644
--- a/drivers/tty/hvc/hvcs.c
+++ b/drivers/tty/hvc/hvcs.c
@@ -309,6 +309,7 @@  struct hvcs_struct {
 
 static LIST_HEAD(hvcs_structs);
 static DEFINE_SPINLOCK(hvcs_structs_lock);
+static DEFINE_MUTEX(hvcs_init_mutex);
 
 static void hvcs_unthrottle(struct tty_struct *tty);
 static void hvcs_throttle(struct tty_struct *tty);
@@ -340,6 +341,7 @@  static int __devinit hvcs_probe(struct vio_dev *dev,
 static int __devexit hvcs_remove(struct vio_dev *dev);
 static int __init hvcs_module_init(void);
 static void __exit hvcs_module_exit(void);
+static int __devinit hvcs_initialize(void);
 
 #define HVCS_SCHED_READ	0x00000001
 #define HVCS_QUICK_READ	0x00000002
@@ -762,7 +764,7 @@  static int __devinit hvcs_probe(
 	const struct vio_device_id *id)
 {
 	struct hvcs_struct *hvcsd;
-	int index;
+	int index, rc;
 	int retval;
 
 	if (!dev || !id) {
@@ -770,6 +772,13 @@  static int __devinit hvcs_probe(
 		return -EPERM;
 	}
 
+	/* Make sure we are properly initialized */
+	rc = hvcs_initialize();
+	if (rc) {
+		pr_err("HVCS: Failed to initialize core driver.\n");
+		return rc;
+	}
+
 	/* early to avoid cleanup on failure */
 	index = hvcs_get_index();
 	if (index < 0) {
@@ -1464,12 +1473,15 @@  static void hvcs_free_index_list(void)
 	hvcs_index_count = 0;
 }
 
-static int __init hvcs_module_init(void)
+static int __devinit hvcs_initialize(void)
 {
-	int rc;
-	int num_ttys_to_alloc;
+	int rc, num_ttys_to_alloc;
 
-	printk(KERN_INFO "Initializing %s\n", hvcs_driver_string);
+	mutex_lock(&hvcs_init_mutex);
+	if (hvcs_task) {
+		mutex_unlock(&hvcs_init_mutex);
+		return 0;
+	}
 
 	/* Has the user specified an overload with an insmod param? */
 	if (hvcs_parm_num_devs <= 0 ||
@@ -1528,35 +1540,13 @@  static int __init hvcs_module_init(void)
 
 	hvcs_task = kthread_run(khvcsd, NULL, "khvcsd");
 	if (IS_ERR(hvcs_task)) {
-		printk(KERN_ERR "HVCS: khvcsd creation failed.  Driver not loaded.\n");
+		printk(KERN_ERR "HVCS: khvcsd creation failed.\n");
 		rc = -EIO;
 		goto kthread_fail;
 	}
-
-	rc = vio_register_driver(&hvcs_vio_driver);
-	if (rc) {
-		printk(KERN_ERR "HVCS: can't register vio driver\n");
-		goto vio_fail;
-	}
-
-	/*
-	 * This needs to be done AFTER the vio_register_driver() call or else
-	 * the kobjects won't be initialized properly.
-	 */
-	rc = driver_create_file(&(hvcs_vio_driver.driver), &driver_attr_rescan);
-	if (rc) {
-		printk(KERN_ERR "HVCS: sysfs attr create failed\n");
-		goto attr_fail;
-	}
-
-	printk(KERN_INFO "HVCS: driver module inserted.\n");
-
+	mutex_unlock(&hvcs_init_mutex);
 	return 0;
 
-attr_fail:
-	vio_unregister_driver(&hvcs_vio_driver);
-vio_fail:
-	kthread_stop(hvcs_task);
 kthread_fail:
 	kfree(hvcs_pi_buff);
 buff_alloc_fail:
@@ -1566,15 +1556,39 @@  register_fail:
 index_fail:
 	put_tty_driver(hvcs_tty_driver);
 	hvcs_tty_driver = NULL;
+	mutex_unlock(&hvcs_init_mutex);
 	return rc;
 }
 
+static int __init hvcs_module_init(void)
+{
+	int rc = vio_register_driver(&hvcs_vio_driver);
+	if (rc) {
+		printk(KERN_ERR "HVCS: can't register vio driver\n");
+		return rc;
+	}
+
+	pr_info("HVCS: Driver registered.\n");
+
+	/* This needs to be done AFTER the vio_register_driver() call or else
+	 * the kobjects won't be initialized properly.
+	 */
+	rc = driver_create_file(&(hvcs_vio_driver.driver), &driver_attr_rescan);
+	if (rc)
+		pr_warning(KERN_ERR "HVCS: Failed to create rescan file (err %d)\n", rc);
+
+	return 0;
+}
+
 static void __exit hvcs_module_exit(void)
 {
 	/*
 	 * This driver receives hvcs_remove callbacks for each device upon
 	 * module removal.
 	 */
+	vio_unregister_driver(&hvcs_vio_driver);
+	if (!hvcs_task)
+		return;
 
 	/*
 	 * This synchronous operation  will wake the khvcsd kthread if it is
@@ -1589,8 +1603,6 @@  static void __exit hvcs_module_exit(void)
 
 	driver_remove_file(&hvcs_vio_driver.driver, &driver_attr_rescan);
 
-	vio_unregister_driver(&hvcs_vio_driver);
-
 	tty_unregister_driver(hvcs_tty_driver);
 
 	hvcs_free_index_list();