From: Samo Pogacnik on
On 21.06.2010 (Mon) at 15:38 +0100 Alan Cox wrote:
> > I'm thinking to leave the ratelimit support in for the time being. I had
> > in mind cases, when someone does
> > "cat /proc/kmsg > dev/ttyprintk" or
> > suppose the console is redirected to ttyprintk (which i would like to be
> > able to do from user program)
>
> Console as in the printk sense would then loop.
>
> If you are going to do the flow control you should probably do something
> like
>
>
> write_room()
> {
> if (!flow_controlled)
> space = 8192;
> return space;
> }
>
> write()
> {
> space -= len;
> }
>
> then your flow control will behave properly and slow down users rather
> than losing data (except stuff sent without blocking)
>
For correct flow control, i suppose current empty space of the real
(final) printk buffer is needed. On the other hand my intermediate
pre-formatting buffer is only "one line" long, but serialized on access
in a way that it is always completely available (except for the time of
tpk_printk() function running). As such intermediate buffer only defines
maximum write_room space.

Now there are two ideas. The first one is to dig out current real printk
buffer space (smells like hacking?) and adapt write_room to that space
in some logical manner. And the other would be to use ratelimit support
to switch between max and zero in write_room answer and remove other
retelimit response?

What do you suggest, do i miss something?

regards, Samo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on
> For correct flow control, i suppose current empty space of the real
> (final) printk buffer is needed. On the other hand my intermediate

Console drivers have their own buffering so that doesn't really work. If
you want to just avoid explosions then you don't need to be quite so
clever

> in some logical manner. And the other would be to use ratelimit support
> to switch between max and zero in write_room answer and remove other
> retelimit response?

Yes - except that a driver isn't permitted to reduce the write room space
by more than bytes written. That is if you offer 512 bytes you can't
offer 0 until 512 bytes have been written - hence the design of the pseudo
code I posted in the previous message.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Samo Pogacnik on

> > in some logical manner. And the other would be to use ratelimit support
> > to switch between max and zero in write_room answer and remove other
> > retelimit response?
>
> Yes - except that a driver isn't permitted to reduce the write room space
> by more than bytes written. That is if you offer 512 bytes you can't
> offer 0 until 512 bytes have been written - hence the design of the pseudo
> code I posted in the previous message.

Now flow control is provided in a way that each write ratelimit false
detection shrinks available space for its output lenth. On the other
hand write returns available space back to maximum on true ratelimit
detection result. Additionaly each next write_room returns at least one
character space under high load, if zero space is reached.

Additionally a private ttyprintk_ratelimit function is provided to
suppress "missed callbacks" printks, because we do not really miss
anything.

regards, Samo
----
Signed-off-by: Samo Pogacnik <samo_pogacnik(a)t-2.net>
diff --git a_linux/Documentation/devices.txt b_linux/Documentation/devices.txt
index 53d64d3..71aef33 100644
--- a_linux/Documentation/devices.txt
+++ b_linux/Documentation/devices.txt
@@ -239,6 +239,7 @@ Your cooperation is appreciated.
0 = /dev/tty Current TTY device
1 = /dev/console System console
2 = /dev/ptmx PTY master multiplex
+ 3 = /dev/ttyprintk User messages via printk TTY device
64 = /dev/cua0 Callout device for ttyS0
...
255 = /dev/cua191 Callout device for ttyS191
diff --git a_linux/drivers/char/Kconfig b_linux/drivers/char/Kconfig
index 3141dd3..5c38a06 100644
--- a_linux/drivers/char/Kconfig
+++ b_linux/drivers/char/Kconfig
@@ -485,6 +485,20 @@ config LEGACY_PTY_COUNT
When not in use, each legacy PTY occupies 12 bytes on 32-bit
architectures and 24 bytes on 64-bit architectures.

+config TTY_PRINTK
+ bool "TTY driver to output user messages via printk"
+ default n
+ ---help---
+ If you say Y here, the support for writing user messages (i.e.
+ console messages) via printk is available.
+
+ The feature is useful to inline user messages with kernel
+ messages.
+ In order to use this feature, you should output user messages
+ to /dev/ttyprintk or redirect console to this TTY.
+
+ If unsure, say N.
+
config BRIQ_PANEL
tristate 'Total Impact briQ front panel driver'
depends on PPC_CHRP
diff --git a_linux/drivers/char/Makefile b_linux/drivers/char/Makefile
index f957edf..ed60f45 100644
--- a_linux/drivers/char/Makefile
+++ b_linux/drivers/char/Makefile
@@ -11,6 +11,7 @@ obj-y += mem.o random.o tty_io.o n_tty.o tty_ioctl.o tty_ldisc.o tty_buffer.o t

obj-$(CONFIG_LEGACY_PTYS) += pty.o
obj-$(CONFIG_UNIX98_PTYS) += pty.o
+obj-$(CONFIG_TTY_PRINTK) += ttyprintk.o
obj-y += misc.o
obj-$(CONFIG_VT) += vt_ioctl.o vc_screen.o selection.o keyboard.o
obj-$(CONFIG_BFIN_JTAG_COMM) += bfin_jtag_comm.o
diff --git a_linux/drivers/char/ttyprintk.c b_linux/drivers/char/ttyprintk.c
new file mode 100644
index 0000000..344f2d3
--- /dev/null
+++ b_linux/drivers/char/ttyprintk.c
@@ -0,0 +1,264 @@
+/*
+ * linux/drivers/char/ttyprintk.c
+ *
+ * Copyright (C) 2010 Samo Pogacnik
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the smems of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+
+/*
+ * This pseudo device allows user to make printk messages. It is possible
+ * to store "console" messages inline with kernel messages for better analyses
+ * of the boot process, for example.
+ */
+
+#include <linux/sched.h>
+#include <linux/ratelimit.h>
+#include <linux/device.h>
+#include <linux/serial.h>
+#include <linux/tty.h>
+
+struct ttyprintk_port {
+ struct tty_port port;
+ struct mutex port_write_mutex;
+};
+
+static struct ttyprintk_port tpk_port;
+
+/*
+ * Ratelimiting support to handle to much output to this device,
+ * because of explicit writes or because of unintentional loop
+ * setup (caught printks again sent to this device).
+ */
+static struct ratelimit_state ttyprintk_rs = {
+ .interval = DEFAULT_RATELIMIT_INTERVAL,
+ .burst = DEFAULT_RATELIMIT_BURST,
+};
+
+#define ttyprintk_printk(fmt, ...) \
+{ \
+ printk(KERN_INFO fmt, ##__VA_ARGS__); \
+}
+
+/*
+ * Our private ratelimit function, to suppress its printk warnings about
+ * missed callbacks, which are irrelevant in a flow control mechanism.
+ */
+static int ttyprintk_ratelimit(struct ratelimit_state *rs,
+ struct ttyprintk_port *tpkp)
+{
+ int ret;
+
+ /* exclusive call of ttyprintk_ratelimit within this tty */
+ mutex_lock(&tpkp->port_write_mutex);
+ /* clear ratelimit missed callbacks counter */
+ rs->missed = 0;
+ ret = __ratelimit(rs);
+ mutex_unlock(&tpkp->port_write_mutex);
+ return ret;
+}
+
+/*
+ * Our simple preformatting supports transparent output of (time-stamped)
+ * printk messages (also suitable for logging service):
+ * - any cr is replaced by nl
+ * - adds a ttyprintk source tag in front of each line
+ * - too long message is fragmeted, with '\'nl between fragments
+ */
+#define TTY_PRINTK_STR_SIZE 508
+static int tpk_space = TTY_PRINTK_STR_SIZE;
+static const char *tpk_tag = "[U] "; /* U for User */
+
+static int tpk_printk(const unsigned char *buf, int count)
+{
+ static char tmp[TTY_PRINTK_STR_SIZE + 4];
+ static int curr;
+ int i;
+
+ for (i = 0; i < count; i++) {
+ tmp[curr] = buf[i];
+ if (curr < TTY_PRINTK_STR_SIZE) {
+ switch (buf[i]) {
+ case '\r':
+ /* replace cr with nl */
+ tmp[curr + 0] = '\n';
+ tmp[curr + 1] = '\0';
+ ttyprintk_printk("%s%s", tpk_tag, tmp);
+ curr = 0;
+ if (buf[i + 1] == '\n')
+ i++;
+ break;
+ case '\n':
+ tmp[curr + 1] = '\0';
+ ttyprintk_printk("%s%s", tpk_tag, tmp);
+ curr = 0;
+ break;
+ default:
+ curr++;
+ }
+ } else {
+ /* end of tmp buffer reached: cut the message in two */
+ tmp[curr + 1] = '\\';
+ tmp[curr + 2] = '\n';
+ tmp[curr + 3] = '\0';
+ ttyprintk_printk("%s%s", tpk_tag, tmp);
+ curr = 0;
+ }
+ }
+ if ((tpk_space == TTY_PRINTK_STR_SIZE) && (curr > 0)) {
+ /* non nl or cr terminated message - add nl */
+ tmp[curr + 0] = '\n';
+ tmp[curr + 1] = '\0';
+ ttyprintk_printk("%s%s", tpk_tag, tmp);
+ curr = 0;
+ }
+
+ return count;
+}
+
+/*
+ * TTY operations open function.
+ */
+static int tpk_open(struct tty_struct *tty, struct file *filp)
+{
+ tty->driver_data = &tpk_port;
+
+ return tty_port_open(&tpk_port.port, tty, filp);
+}
+
+/*
+ * TTY operations close function.
+ */
+static void tpk_close(struct tty_struct *tty, struct file *filp)
+{
+ struct ttyprintk_port *tpkp = tty->driver_data;
+
+ tty_port_close(&tpkp->port, tty, filp);
+}
+
+/*
+ * TTY operations write function.
+ */
+static int tpk_write(struct tty_struct *tty,
+ const unsigned char *buf, int count)
+{
+ struct ttyprintk_port *tpkp = tty->driver_data;
+ int ret;
+
+ if (ttyprintk_ratelimit(&ttyprintk_rs, tpkp))
+ tpk_space = TTY_PRINTK_STR_SIZE;
+ else {
+ if (tpk_space > count)
+ tpk_space -= count;
+ else
+ tpk_space = 0;
+ }
+
+ /* exclusive use of tpk_printk within this tty */
+ mutex_lock(&tpkp->port_write_mutex);
+ ret = tpk_printk(buf, count);
+ mutex_unlock(&tpkp->port_write_mutex);
+
+ return ret;
+}
+
+/*
+ * TTY operations write_room function.
+ */
+static int tpk_write_room(struct tty_struct *tty)
+{
+ int ret = tpk_space;
+
+ /* allow char by char under max pressure */
+ if (tpk_space == 0)
+ tpk_space = 1;
+
+ return ret;
+}
+
+/*
+ * TTY operations ioctl function.
+ */
+static int tpk_ioctl(struct tty_struct *tty, struct file *file,
+ unsigned int cmd, unsigned long arg)
+{
+ struct ttyprintk_port *port;
+
+ port = tty->driver_data;
+
+ if (!port)
+ return -EINVAL;
+
+ switch (cmd) {
+ /* Stop TIOCCONS */
+ case TIOCCONS:
+ return -EOPNOTSUPP;
+ default:
+ return -ENOIOCTLCMD;
+ }
+ return 0;
+}
+
+static const struct tty_operations ttyprintk_ops = {
+ .open = tpk_open,
+ .close = tpk_close,
+ .write = tpk_write,
+ .write_room = tpk_write_room,
+ .ioctl = tpk_ioctl,
+};
+
+struct tty_port_operations null_ops = { };
+
+static struct tty_driver *ttyprintk_driver;
+
+static int __init ttyprintk_init(void)
+{
+ int ret = -ENOMEM;
+ void *rp;
+
+ ttyprintk_driver = alloc_tty_driver(1);
+ if (!ttyprintk_driver)
+ return ret;
+
+ ttyprintk_driver->owner = THIS_MODULE;
+ ttyprintk_driver->driver_name = "ttyprintk";
+ ttyprintk_driver->name = "ttyprintk";
+ ttyprintk_driver->major = TTYAUX_MAJOR;
+ ttyprintk_driver->minor_start = 3;
+ ttyprintk_driver->num = 1;
+ ttyprintk_driver->type = TTY_DRIVER_TYPE_CONSOLE;
+ ttyprintk_driver->init_termios = tty_std_termios;
+ ttyprintk_driver->init_termios.c_oflag = OPOST | OCRNL | ONOCR | ONLRET;
+ ttyprintk_driver->flags = TTY_DRIVER_RESET_TERMIOS |
+ TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
+ tty_set_operations(ttyprintk_driver, &ttyprintk_ops);
+
+ ret = tty_register_driver(ttyprintk_driver);
+ if (ret < 0) {
+ printk(KERN_ERR "Couldn't register ttyprintk driver\n");
+ goto error;
+ }
+
+ /* create our unnumbered device */
+ rp = device_create(tty_class, NULL, MKDEV(TTYAUX_MAJOR, 3), NULL,
+ ttyprintk_driver->name);
+ if (IS_ERR(rp)) {
+ printk(KERN_ERR "Couldn't create ttyprintk device\n");
+ ret = PTR_ERR(rp);
+ goto error;
+ }
+
+ tty_port_init(&tpk_port.port);
+ tpk_port.port.ops = &null_ops;
+ mutex_init(&tpk_port.port_write_mutex);
+
+ return 0;
+
+error:
+ put_tty_driver(ttyprintk_driver);
+ ttyprintk_driver = NULL;
+ return ret;
+}
+module_init(ttyprintk_init);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on
> +static int tpk_write_room(struct tty_struct *tty)
> +{
> + int ret = tpk_space;
> +
> + /* allow char by char under max pressure */
> + if (tpk_space == 0)
> + tpk_space = 1;

That won't do what you think, the ldisc will keep seeing progress and
generate millions of 1 byte I/Os in a loop !

Otherwise looks excellent.


> + switch (cmd) {
> + /* Stop TIOCCONS */
> + case TIOCCONS:
> + return -EOPNOTSUPP;

And I'll fix this bit up to work properly in the core code.


With my devices.txt owner hat on I'll allocate the minor as you suggest
(and double check this causes no problems), with my tty hat on can you
send it to GregKH for merging into the tree.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Samo Pogacnik on
> > +static int tpk_write_room(struct tty_struct *tty)
> > +{
> > + int ret = tpk_space;
> > +
> > + /* allow char by char under max pressure */
> > + if (tpk_space == 0)
> > + tpk_space = 1;
>
> That won't do what you think, the ldisc will keep seeing progress and
> generate millions of 1 byte I/Os in a loop !

I thought that this would automatically reduce processor load, which is
obviously not the case. Sorry for the delay, but i am trying to figure
out how to slow down write method when under pressure.

And that setting tpk_space to 1 would then be just in case we reach 0 to
enable further processing.

Samo


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/