2014-02-26 14:41:33

by Peter Hurley

[permalink] [raw]
Subject: [PATCH] tty: Add sysfs symlink for console name->tty device

Enable a user-space process to discover the underlying tty device
for a console, if one exists, and when the tty device is later
created or destroyed.

Add sysfs symlinks for registered consoles to their respective
devices in [sys/class,sys/devices/virtual]/tty/console.
Scan consoles at tty device (un)registration to handle deferred
console<->device (un)binding.

Reported-by: Hannes Reinecke <[email protected]>
Cc: Ray Strode <[email protected]>
Cc: Kay Sievers <[email protected]>
Cc: David Herrmann <[email protected]>
Cc: Lennart Poettering <[email protected]>
Cc: Werner Fink <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/tty_io.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++-
include/linux/console.h | 8 +++-
kernel/printk/printk.c | 4 +-
3 files changed, 129 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index a7cdbc5..53bb141 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -169,6 +169,12 @@ static void release_tty(struct tty_struct *tty, int idx);
static void __proc_set_tty(struct task_struct *tsk, struct tty_struct *tty);
static void proc_set_tty(struct task_struct *tsk, struct tty_struct *tty);

+#define TTY_SYSFS_NOTIFY_REGISTER 0
+#define TTY_SYSFS_NOTIFY_UNREGISTER 1
+static void tty_sysfs_notify(struct tty_driver *driver, int index,
+ struct device *dev, int event);
+
+
/**
* alloc_tty_struct - allocate a tty object
*
@@ -3172,6 +3178,8 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
if (retval)
goto error;

+ tty_sysfs_notify(driver, index, dev, TTY_SYSFS_NOTIFY_REGISTER);
+
return dev;

error:
@@ -3195,6 +3203,8 @@ EXPORT_SYMBOL_GPL(tty_register_device_attr);

void tty_unregister_device(struct tty_driver *driver, unsigned index)
{
+ tty_sysfs_notify(driver, index, NULL, TTY_SYSFS_NOTIFY_UNREGISTER);
+
device_destroy(tty_class,
MKDEV(driver->major, driver->minor_start) + index);
if (!(driver->flags & TTY_DRIVER_DYNAMIC_ALLOC))
@@ -3544,12 +3554,118 @@ static DEVICE_ATTR(active, S_IRUGO, show_cons_active, NULL);

static struct device *consdev;

-void console_sysfs_notify(void)
+static void console_name(struct console *c, size_t n, char *name)
{
- if (consdev)
+ snprintf(name, n, "%s%d", c->name, c->index);
+}
+
+static struct device *console_to_tty_device(struct console *c)
+{
+ struct tty_driver *driver;
+ dev_t devt;
+ int index;
+
+ if (!c->device)
+ return NULL;
+ driver = c->device(c, &index);
+ if (!driver)
+ return NULL;
+ devt = MKDEV(driver->major, driver->minor_start) + index;
+ return class_find_device(tty_class, NULL, &devt, dev_match_devt);
+}
+
+static void symlink_console_tty_device(struct console *c)
+{
+ struct device *tty_dev;
+ char link[64];
+
+ tty_dev = console_to_tty_device(c);
+ if (tty_dev) {
+ console_name(c, sizeof(link), link);
+ sysfs_create_link_nowarn(&consdev->kobj, &tty_dev->kobj, link);
+ put_device(tty_dev);
+ }
+}
+
+static void unlink_console_tty_device(struct console *c)
+{
+ char link[64];
+
+ console_name(c, sizeof(link), link);
+ sysfs_remove_link(&consdev->kobj, link);
+}
+
+void console_sysfs_notify(struct console *console, int event)
+{
+ if (!consdev)
+ return;
+
+ switch (event) {
+ case CON_SYSFS_NOTIFY_REGISTER:
+ symlink_console_tty_device(console);
+ break;
+ case CON_SYSFS_NOTIFY_UNREGISTER:
+ unlink_console_tty_device(console);
+ break;
+ }
+
+ sysfs_notify(&consdev->kobj, NULL, "active");
+}
+
+static void tty_sysfs_notify(struct tty_driver *driver, int index,
+ struct device *dev, int event)
+{
+ struct console *c;
+ bool found_one = false;
+
+ if (!consdev)
+ return;
+
+ /* check if this tty device is a console device and if so
+ * update the sysfs console links
+ */
+ console_lock();
+ for_each_console(c) {
+ char link[64];
+ struct tty_driver *c_driver;
+ int c_index;
+
+ if (!c->device)
+ continue;
+ c_driver = c->device(c, &c_index);
+
+ if (driver == c_driver && index == c_index) {
+ console_name(c, sizeof(link), link);
+
+ switch (event) {
+ case TTY_SYSFS_NOTIFY_REGISTER:
+ sysfs_create_link_nowarn(&consdev->kobj,
+ &dev->kobj, link);
+ break;
+ case TTY_SYSFS_NOTIFY_UNREGISTER:
+ sysfs_remove_link(&consdev->kobj, link);
+ break;
+ }
+ found_one = true;
+ }
+ }
+ console_unlock();
+
+ if (found_one)
sysfs_notify(&consdev->kobj, NULL, "active");
}

+
+static void symlink_console_tty_devices(void)
+{
+ struct console *c;
+
+ console_lock();
+ for_each_console(c)
+ symlink_console_tty_device(c);
+ console_unlock();
+}
+
/*
* Ok, now we can initialize the rest of the tty devices and can count
* on memory allocations, interrupts etc..
@@ -3573,6 +3689,9 @@ int __init tty_init(void)
else
WARN_ON(device_create_file(consdev, &dev_attr_active) < 0);

+ if (consdev)
+ symlink_console_tty_devices();
+
#ifdef CONFIG_VT
vty_init(&console_fops);
#endif
diff --git a/include/linux/console.h b/include/linux/console.h
index 7571a16..e8c4c04 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -157,10 +157,14 @@ extern int is_console_locked(void);
extern int braille_register_console(struct console *, int index,
char *console_options, char *braille_options);
extern int braille_unregister_console(struct console *);
+
+/* event values for console_sysfs_notify() */
+#define CON_SYSFS_NOTIFY_REGISTER 0
+#define CON_SYSFS_NOTIFY_UNREGISTER 1
#ifdef CONFIG_TTY
-extern void console_sysfs_notify(void);
+extern void console_sysfs_notify(struct console *, int event);
#else
-static inline void console_sysfs_notify(void)
+static inline void console_sysfs_notify(struct console *, int event)
{ }
#endif
extern bool console_suspend_enabled;
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index b1d255f..1e8dbe2 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2348,7 +2348,7 @@ void register_console(struct console *newcon)
exclusive_console = newcon;
}
console_unlock();
- console_sysfs_notify();
+ console_sysfs_notify(newcon, CON_SYSFS_NOTIFY_REGISTER);

/*
* By unregistering the bootconsoles after we enable the real console
@@ -2410,7 +2410,7 @@ int unregister_console(struct console *console)
console_drivers->flags |= CON_CONSDEV;

console_unlock();
- console_sysfs_notify();
+ console_sysfs_notify(console, CON_SYSFS_NOTIFY_UNREGISTER);
return res;
}
EXPORT_SYMBOL(unregister_console);
--
1.8.1.2


2014-02-27 11:13:33

by Kay Sievers

[permalink] [raw]
Subject: Re: [PATCH] tty: Add sysfs symlink for console name->tty device

On Wed, Feb 26, 2014 at 3:40 PM, Peter Hurley <[email protected]> wrote:
> Enable a user-space process to discover the underlying tty device
> for a console, if one exists, and when the tty device is later
> created or destroyed.
>
> Add sysfs symlinks for registered consoles to their respective
> devices in [sys/class,sys/devices/virtual]/tty/console.
> Scan consoles at tty device (un)registration to handle deferred
> console<->device (un)binding.

What tool is supposed to read that? I can't think of anything
interested in this, as soon as we have fixed the "active" console
output.

Thanks,
Kay

2014-02-27 13:31:59

by Peter Hurley

[permalink] [raw]
Subject: Re: [PATCH] tty: Add sysfs symlink for console name->tty device

On 02/27/2014 06:13 AM, Kay Sievers wrote:
> On Wed, Feb 26, 2014 at 3:40 PM, Peter Hurley <[email protected]> wrote:
>> Enable a user-space process to discover the underlying tty device
>> for a console, if one exists, and when the tty device is later
>> created or destroyed.
>>
>> Add sysfs symlinks for registered consoles to their respective
>> devices in [sys/class,sys/devices/virtual]/tty/console.
>> Scan consoles at tty device (un)registration to handle deferred
>> console<->device (un)binding.
>
> What tool is supposed to read that? I can't think of anything
> interested in this, as soon as we have fixed the "active" console
> output.

Kay,

With all due respect, that "fix" is a ridiculous hack, being done
for self-serving expedience. It already caused one user-space breakage
which you did not expect.

This sysfs interface is superior in every way.

Regards,
Peter Hurley

2014-02-27 13:37:17

by Kay Sievers

[permalink] [raw]
Subject: Re: [PATCH] tty: Add sysfs symlink for console name->tty device

On Thu, Feb 27, 2014 at 2:31 PM, Peter Hurley <[email protected]> wrote:
> On 02/27/2014 06:13 AM, Kay Sievers wrote:
>>
>> On Wed, Feb 26, 2014 at 3:40 PM, Peter Hurley <[email protected]>
>> wrote:
>>>
>>> Enable a user-space process to discover the underlying tty device
>>> for a console, if one exists, and when the tty device is later
>>> created or destroyed.
>>>
>>> Add sysfs symlinks for registered consoles to their respective
>>> devices in [sys/class,sys/devices/virtual]/tty/console.
>>> Scan consoles at tty device (un)registration to handle deferred
>>> console<->device (un)binding.
>>
>> What tool is supposed to read that? I can't think of anything
>> interested in this, as soon as we have fixed the "active" console
>> output.

> With all due respect, that "fix" is a ridiculous hack,

No, it is not. It's fine to handle tty0 special, as it is special.

> being done
> for self-serving expedience.

I don't see the problem.

> It already caused one user-space breakage
> which you did not expect.

That is normal way to do things, only people who don't do things don't
break things. And broken things get fixed, and the "active" file is still
fixable, and that is what we should do.

We don't need to invent new things because we did not get things right
with the first try.

> This sysfs interface is superior in every way.

But nothing uses it now, and probably never will, so I don't see the
need for it at this moment.

Kay

2014-02-27 14:00:23

by Peter Hurley

[permalink] [raw]
Subject: Re: [PATCH] tty: Add sysfs symlink for console name->tty device

On 02/27/2014 08:36 AM, Kay Sievers wrote:
> On Thu, Feb 27, 2014 at 2:31 PM, Peter Hurley <[email protected]> wrote:
>> On 02/27/2014 06:13 AM, Kay Sievers wrote:
>>>
>>> On Wed, Feb 26, 2014 at 3:40 PM, Peter Hurley <[email protected]>
>>> wrote:
>>>>
>>>> Enable a user-space process to discover the underlying tty device
>>>> for a console, if one exists, and when the tty device is later
>>>> created or destroyed.
>>>>
>>>> Add sysfs symlinks for registered consoles to their respective
>>>> devices in [sys/class,sys/devices/virtual]/tty/console.
>>>> Scan consoles at tty device (un)registration to handle deferred
>>>> console<->device (un)binding.
>>>
>>> What tool is supposed to read that? I can't think of anything
>>> interested in this, as soon as we have fixed the "active" console
>>> output.
>
>> With all due respect, that "fix" is a ridiculous hack,
>
> No, it is not. It's fine to handle tty0 special, as it is special.

I wasn't just referring to the plymouth workaround.

>> being done
>> for self-serving expedience.
>
> I don't see the problem.
>
>> It already caused one user-space breakage
>> which you did not expect.
>
> That is normal way to do things, only people who don't do things don't
> break things. And broken things get fixed, and the "active" file is still
> fixable, and that is what we should do.

But what's the plan when more user-space breakage is uncovered after that
change has been in 10 kernel releases?

Then everyone will point to how much user-space breakage reverting it
will cause.

> We don't need to invent new things because we did not get things right
> with the first try.
>
>> This sysfs interface is superior in every way.
>
> But nothing uses it now, and probably never will, so I don't see the
> need for it at this moment.

Let's just avoid the mess right up-front.

Regards,
Peter Hurley

2014-03-01 00:34:12

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] tty: Add sysfs symlink for console name->tty device

On Wed, Feb 26, 2014 at 09:40:51AM -0500, Peter Hurley wrote:
> Enable a user-space process to discover the underlying tty device
> for a console, if one exists, and when the tty device is later
> created or destroyed.

What userspace code has been tested with this change?

> Add sysfs symlinks for registered consoles to their respective
> devices in [sys/class,sys/devices/virtual]/tty/console.
> Scan consoles at tty device (un)registration to handle deferred
> console<->device (un)binding.

I don't understand, what does userspace now look like in sysfs? Do we
need Documentation/ABI/ updates here?

And David has fixed up his original patch, which doesn't break plymouth,
and I'll be taking that, so I don't see why this patch is needed.

thanks,

greg k-h

2014-03-01 00:41:10

by Peter Hurley

[permalink] [raw]
Subject: Re: [PATCH] tty: Add sysfs symlink for console name->tty device

On 02/28/2014 07:35 PM, Greg Kroah-Hartman wrote:
> On Wed, Feb 26, 2014 at 09:40:51AM -0500, Peter Hurley wrote:
>> Enable a user-space process to discover the underlying tty device
>> for a console, if one exists, and when the tty device is later
>> created or destroyed.
>
> What userspace code has been tested with this change?

Every existing distro + personal copies going back 42 versions.
No breakage. ;)

>> Add sysfs symlinks for registered consoles to their respective
>> devices in [sys/class,sys/devices/virtual]/tty/console.
>> Scan consoles at tty device (un)registration to handle deferred
>> console<->device (un)binding.
>
> I don't understand, what does userspace now look like in sysfs? Do we
> need Documentation/ABI/ updates here?
>
> And David has fixed up his original patch, which doesn't break plymouth,
> and I'll be taking that, so I don't see why this patch is needed.

Ok. I tried.

Regards,
Peter Hurley