Re: [RFC PATCH] printk: console: Allow each console to have its own loglevel

[Chris Down]

Greg Kroah-Hartman writes:
> The two stanzas in my reply do NOT refer to the same thing.
>
> The first one is for the device that is assigned to the class.  That
> must be freed and properly reference counted and handled as that is a
> dynamic object that can come and go as people add and remove consoles.

Ah, I see. So to be clear, this change would fix what you're concerned about, 
right? :-)

If so I'll do this in v2. Thanks!

---

diff --git include/linux/console.h include/linux/console.h
index ce5ba405285a..408dd86be8eb 100644
--- include/linux/console.h
+++ include/linux/console.h
@@ -156,12 +156,6 @@ static inline int con_debug_leave(void)
  */
 #define CON_LOGLEVEL	(128) /* Level set locally for this console */
 
-/*
- * Console has active class device, so may have active readers/writers from
- * /sys/class hierarchy.
- */
-#define CON_CLASSDEV_ACTIVE	(256)
-
 struct console {
 	char	name[16];
 	void	(*write)(struct console *, const char *, unsigned);
@@ -179,9 +173,11 @@ struct console {
 	void	*data;
 	struct	 console *next;
 	int	level;
-	struct	device classdev;
+	struct	device *classdev;
 };
 
+#define classdev_to_console(dev) dev_get_drvdata(dev)
+
 /*
  * for_each_console() allows you to iterate on each console
  */
diff --git kernel/printk/printk.c kernel/printk/printk.c
index ee3328f7b6fb..a60bcf296c25 100644
--- kernel/printk/printk.c
+++ kernel/printk/printk.c
@@ -3076,7 +3076,7 @@ early_param("keep_bootcon", keep_bootcon_setup);
 static ssize_t loglevel_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
-	struct console *con = container_of(dev, struct console, classdev);
+	struct console *con = classdev_to_console(dev);
 
 	if (con->flags & CON_LOGLEVEL)
 		return sprintf(buf, "%d\n", con->level);
@@ -3087,7 +3087,7 @@ static ssize_t loglevel_show(struct device *dev, struct device_attribute *attr,
 static ssize_t loglevel_store(struct device *dev, struct device_attribute *attr,
 			      const char *buf, size_t size)
 {
-	struct console *con = container_of(dev, struct console, classdev);
+	struct console *con = classdev_to_console(dev);
 	ssize_t ret;
 	int tmp;
 
@@ -3115,7 +3115,7 @@ static ssize_t effective_loglevel_source_show(struct device *dev,
 					      struct device_attribute *attr,
 					      char *buf)
 {
-	struct console *con = container_of(dev, struct console, classdev);
+	struct console *con = classdev_to_console(dev);
 	enum loglevel_source source;
 
 	console_effective_loglevel(con, &source);
@@ -3128,7 +3128,7 @@ static ssize_t effective_loglevel_show(struct device *dev,
 				       struct device_attribute *attr,
 				       char *buf)
 {
-	struct console *con = container_of(dev, struct console, classdev);
+	struct console *con = classdev_to_console(dev);
 	enum loglevel_source source;
 
 	return sprintf(buf, "%d\n", console_effective_loglevel(con, &source));
@@ -3139,7 +3139,7 @@ static DEVICE_ATTR_RO(effective_loglevel);
 static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
 			    char *buf)
 {
-	struct console *con = container_of(dev, struct console, classdev);
+	struct console *con = classdev_to_console(dev);
 
 	return sprintf(buf, "%d\n", !!(con->flags & CON_ENABLED));
 }
@@ -3158,15 +3158,7 @@ ATTRIBUTE_GROUPS(console_sysfs);
 
 static void console_classdev_release(struct device *dev)
 {
-	struct console *con = container_of(dev, struct console, classdev);
-
-	/*
-	 * `struct console' objects are statically allocated (or at the very
-	 * least managed outside of our lifecycle), nothing to do. Just set a
-	 * flag so that we know we are no longer waiting for anyone and can
-	 * return control in unregister_console().
-	 */
-	con->flags &= ~CON_CLASSDEV_ACTIVE;
+	kfree(dev);
 }
 
 static void console_register_device(struct console *new)
@@ -3179,13 +3171,17 @@ static void console_register_device(struct console *new)
 	if (IS_ERR_OR_NULL(console_class))
 		return;
 
-	new->flags |= CON_CLASSDEV_ACTIVE;
-	device_initialize(&new->classdev);
-	dev_set_name(&new->classdev, "%s", new->name);
-	new->classdev.release = console_classdev_release;
-	new->classdev.class = console_class;
-	if (WARN_ON(device_add(&new->classdev)))
-		put_device(&new->classdev);
+	new->classdev = kzalloc(sizeof(struct device), GFP_KERNEL);
+	if (!new->classdev)
+		return;
+
+	device_initialize(new->classdev);
+	dev_set_name(new->classdev, "%s", new->name);
+	dev_set_drvdata(new->classdev, new);
+	new->classdev->release = console_classdev_release;
+	new->classdev->class = console_class;
+	if (WARN_ON(device_add(new->classdev)))
+		put_device(new->classdev);
 }
 
 /*
@@ -3462,7 +3458,7 @@ int unregister_console(struct console *console)
 		console_drivers->flags |= CON_CONSDEV;
 
 	console->flags &= ~CON_ENABLED;
-	device_unregister(&console->classdev);
+	device_unregister(console->classdev);
 	console_unlock();
 	console_sysfs_notify();
 
@@ -3475,14 +3471,6 @@ int unregister_console(struct console *console)
 	console->flags &= ~CON_ENABLED;
 	console_unlock();
 
-	/*
-	 * Wait for anyone holding the classdev open to close it so that we
-	 * don't remove the module prematurely. Once classdev refcnt is 0,
-	 * CON_CLASSDEV_ACTIVE will be unset in console_classdev_release.
-	 */
-	while (console->flags & CON_CLASSDEV_ACTIVE)
-		schedule_timeout_uninterruptible(msecs_to_jiffies(1));
-
 	return res;
 }
 EXPORT_SYMBOL(unregister_console);