Re: [PATCH 2/3] printk: Add /sys/consoles/ interface

From
Kroah-Hartman <hidden>
Date
2017-11-03 15:57:50
Also in
lkml
Reply to
Petr Mladek, 2017-11-03
On Fri, Nov 03, 2017 at 04:46:51PM +0100, Petr Mladek wrote:
On Fri 2017-11-03 15:32:34, Kroah-Hartman wrote:
quoted
quoted
quoted
diff --git a/Documentation/ABI/testing/sysfs-consoles b/Documentation/ABI/testing/sysfs-consoles
new file mode 100644
index 0000000..6a1593e
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-consoles
@@ -0,0 +1,13 @@
+What:		/sys/consoles/
Eeek, what!
quoted
I rather add Greg in CC. I am not 100% sure that the top level
directory is the right thing to do.
Neither do I.
quoted
Alternative might be to hide this under /sys/kernel/consoles/.
No no no.
quoted
quoted
diff --git a/include/linux/console.h b/include/linux/console.h
index a5b5d79..76840be 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -148,6 +148,7 @@ struct console {
 	void	*data;
 	struct	 console *next;
 	int	level;
+	struct kobject *kobj;
Why are you using "raw" kobjects and not a "real" struct device?  This
is a device, use that interface instead please.
Hmm, struct console has a member

	struct tty_driver *(*device)(struct console *, int *);

but it is set only when the console has tty binding.
I don't understand what you are referring to here, sorry.
quoted
If you need a console 'bus' to place them on, fine, but the virtual bus
is probably best and simpler to use.

That is if you _really_ feel you need sysfs interaction with the console
layer (hint, I am not yet convinced...)
The purpose of this patch is to make a userfriendly interface
for setting console-specific loglevel (message filtering).

It curretnly uses kobject for creating a simple directory
structure under /sys/. It is inspired by /sys/power/, /sys/kernel/mm/,
/sys/kernel/debug/tracing/ stuff.

There are ideas to add more files that would allow to modify even the
global setting. This is currectly possible by the four numbers in
/proc/sys/kernel/printk. Nobody knows what the four numbers mean.
IMHO, the following interface would be easier to work with:

       /sys/console/loglevel
       /sys/console/min_loglevel
       /sys/condole/default_loglevel
No, please do not polute the /sys/* namespace with stuff like this.  If
this is associated with a specific device, hang the sysfs files off of
it.  Your console is in the /sys/device/ tree, right?
quoted
quoted
	/*
	 * Find the related struct console a safe way. The kobject
	 * desctruction is asynchronous.
	 */
quoted
+	console_lock();
+	for_each_console(con) {
+		if (con->kobj == kobj) {
You are doing something wrong, go from kobj to your console directly,
the fact that you can not do that here is a _huge_ hint that your
structure is not correct.

Hint, it's not correct at all :)
I know that we are not following the original purpose of sysfs.
But there are more (mis)users.
Sure, and I will gladly work to fix those.  But do not add known-broken
code to the tree :)

thanks,

greg k-h
Thread (18 messages) 18 messages, 5 authors, last reply 2018-10-22
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help