2012-10-18 20:31:56

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 00/21] TTY buffer in tty_port and other stuff

Hi,

this is the fifth series of patches which finally move tty buffers
from tty_struct (present from open to close/hangup) to tty_port
(present as long as the device). This allows us to get rid of the tty
refcounting in the interrupt service routines and other hot paths
after we are done. This is because we do not need to handle races
among ISRs, timers, hangups and others, because tty_port lives as long
as an interrupt/timer tick may occur. Unlike tty_struct.

This set also cleans up devpts handling a bit. Devpts used to play
with tty->driver_data which was a bit ugly. Now devpts returns a node
which we store to driver_data and pass it back when we need devpts to
kill that. As a result, we can do that in the pty code instead of an
ugly hook in tty_release.

Finally, the set moves all the n_tty private stuff from tty_struct to
its own (internal) structure. This was an intention last time ago (at
least here), but the races and undefined ldisc->open/close behavior
did not allow us to do that. Now that we have ldisc kills and waits
and bells and whistles we could finally go ahead.

As usual, standard x86 stuff was runtime-tested. The rest is only
checked to be compilation-errors free.

Jiri Slaby (21):
TTY: devpts, don't care about TTY in devpts_get_tty
TTY: devpts, return created inode from devpts_pty_new
TTY: devpts, do not set driver_data
TTY: devpts, document devpts inode operations
TTY: move devpts kill to pty
TTY: vt, fix paste_selection ldisc handling
TTY: ldisc, wait for idle ldisc in release
TTY: hci_ldisc, remove invalid check in open
TTY: n_tty, simplify read_buf+echo_buf allocation
TTY: n_tty, remove bogus checks
TTY: audit, stop accessing tty->icount
TTY: n_tty, add ldisc data to n_tty
TTY: move ldisc data from tty_struct: simple members
TTY: move ldisc data from tty_struct: bitmaps
TTY: move ldisc data from tty_struct: read_* and echo_* and canon_*
stuff
TTY: move ldisc data from tty_struct: locks
TTY: n_tty, propagate n_tty_data
TTY: move TTY_FLUSH* flags to tty_port
TTY: tty_buffer, cache pointer to tty->buf
TTY: add port -> tty link
TTY: move tty buffers to tty_port

drivers/bluetooth/hci_ldisc.c | 7 +-
drivers/tty/n_tty.c | 752 ++++++++++++++++++++++--------------------
drivers/tty/pty.c | 30 +-
drivers/tty/tty_audit.c | 15 +-
drivers/tty/tty_buffer.c | 224 +++++++------
drivers/tty/tty_io.c | 15 +-
drivers/tty/tty_ldisc.c | 15 +-
drivers/tty/tty_port.c | 2 +
drivers/tty/vt/selection.c | 9 +-
fs/devpts/inode.c | 61 ++--
include/linux/devpts_fs.h | 20 +-
include/linux/tty.h | 44 +--
include/linux/tty_flip.h | 2 +-
13 files changed, 633 insertions(+), 563 deletions(-)

--
1.7.12.3


2012-10-18 20:26:57

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 12/21] TTY: n_tty, add ldisc data to n_tty

All n_tty related members from tty_struct will be moved here.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/n_tty.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 3ebab0c..3d1594e 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -73,6 +73,10 @@
#define ECHO_OP_SET_CANON_COL 0x81
#define ECHO_OP_ERASE_TAB 0x82

+struct n_tty_data {
+ char dummy;
+};
+
static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
unsigned char __user *ptr)
{
@@ -1556,11 +1560,15 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)

static void n_tty_close(struct tty_struct *tty)
{
+ struct n_tty_data *ldata = tty->disc_data;
+
n_tty_flush_buffer(tty);
kfree(tty->read_buf);
kfree(tty->echo_buf);
+ kfree(ldata);
tty->read_buf = NULL;
tty->echo_buf = NULL;
+ tty->disc_data = NULL;
}

/**
@@ -1575,23 +1583,32 @@ static void n_tty_close(struct tty_struct *tty)

static int n_tty_open(struct tty_struct *tty)
{
+ struct n_tty_data *ldata;
+
+ ldata = kzalloc(sizeof(*ldata), GFP_KERNEL);
+ if (!ldata)
+ goto err;
+
/* These are ugly. Currently a malloc failure here can panic */
tty->read_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
tty->echo_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
if (!tty->read_buf || !tty->echo_buf)
goto err_free_bufs;

+ tty->disc_data = ldata;
reset_buffer_flags(tty);
tty_unthrottle(tty);
tty->column = 0;
n_tty_set_termios(tty, NULL);
tty->minimum_to_wake = 1;
tty->closing = 0;
+
return 0;
err_free_bufs:
kfree(tty->read_buf);
kfree(tty->echo_buf);
-
+ kfree(ldata);
+err:
return -ENOMEM;
}

--
1.7.12.3

2012-10-18 20:27:03

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 05/21] TTY: move devpts kill to pty

Now that we have control over tty->driver_data in pty, we can just
kill the /dev/pts/ in pty code too. Namely, in ->shutdown hook of
tty. For pty, this is called only once, for whichever end is closed
last. But we don't care, both driver_data are the inode as it used to
be till now.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/pty.c | 9 +++++++++
drivers/tty/tty_io.c | 5 -----
2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 559e5b2..2728afe 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -363,6 +363,12 @@ err:
return retval;
}

+/* this is called once with whichever end is closed last */
+static void pty_unix98_shutdown(struct tty_struct *tty)
+{
+ devpts_kill_index(tty->driver_data, tty->index);
+}
+
static void pty_cleanup(struct tty_struct *tty)
{
kfree(tty->port);
@@ -578,6 +584,7 @@ static const struct tty_operations ptm_unix98_ops = {
.set_termios = pty_set_termios,
.ioctl = pty_unix98_ioctl,
.resize = pty_resize,
+ .shutdown = pty_unix98_shutdown,
.cleanup = pty_cleanup
};

@@ -593,6 +600,7 @@ static const struct tty_operations pty_unix98_ops = {
.chars_in_buffer = pty_chars_in_buffer,
.unthrottle = pty_unthrottle,
.set_termios = pty_set_termios,
+ .shutdown = pty_unix98_shutdown,
.cleanup = pty_cleanup,
};

@@ -661,6 +669,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
goto err_release;

tty_unlock(tty);
+ tty->driver_data = inode;
tty->link->driver_data = slave_inode;
return 0;
err_release:
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 2ea176b..e835a5b8 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1625,7 +1625,6 @@ int tty_release(struct inode *inode, struct file *filp)
struct tty_struct *tty = file_tty(filp);
struct tty_struct *o_tty;
int pty_master, tty_closing, o_tty_closing, do_sleep;
- int devpts;
int idx;
char buf[64];

@@ -1640,7 +1639,6 @@ int tty_release(struct inode *inode, struct file *filp)
idx = tty->index;
pty_master = (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
tty->driver->subtype == PTY_TYPE_MASTER);
- devpts = (tty->driver->flags & TTY_DRIVER_DEVPTS_MEM) != 0;
/* Review: parallel close */
o_tty = tty->link;

@@ -1799,9 +1797,6 @@ int tty_release(struct inode *inode, struct file *filp)
release_tty(tty, idx);
mutex_unlock(&tty_mutex);

- /* Make this pty number available for reallocation */
- if (devpts)
- devpts_kill_index(inode, idx);
return 0;
}

--
1.7.12.3

2012-10-18 20:27:13

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 17/21] TTY: n_tty, propagate n_tty_data

In some funtions we need only n_tty_data, so pass it down directly in
case tty is not needed there.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/n_tty.c | 171 ++++++++++++++++++++++++----------------------------
1 file changed, 78 insertions(+), 93 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 0a6fcda..531e539 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -153,10 +153,8 @@ static void n_tty_set_room(struct tty_struct *tty)
schedule_work(&tty->buf.work);
}

-static void put_tty_queue_nolock(unsigned char c, struct tty_struct *tty)
+static void put_tty_queue_nolock(unsigned char c, struct n_tty_data *ldata)
{
- struct n_tty_data *ldata = tty->disc_data;
-
if (ldata->read_cnt < N_TTY_BUF_SIZE) {
ldata->read_buf[ldata->read_head] = c;
ldata->read_head = (ldata->read_head + 1) & (N_TTY_BUF_SIZE-1);
@@ -167,23 +165,22 @@ static void put_tty_queue_nolock(unsigned char c, struct tty_struct *tty)
/**
* put_tty_queue - add character to tty
* @c: character
- * @tty: tty device
+ * @ldata: n_tty data
*
* Add a character to the tty read_buf queue. This is done under the
* read_lock to serialize character addition and also to protect us
* against parallel reads or flushes
*/

-static void put_tty_queue(unsigned char c, struct tty_struct *tty)
+static void put_tty_queue(unsigned char c, struct n_tty_data *ldata)
{
- struct n_tty_data *ldata = tty->disc_data;
unsigned long flags;
/*
* The problem of stomping on the buffers ends here.
* Why didn't anyone see this one coming? --AJK
*/
spin_lock_irqsave(&ldata->read_lock, flags);
- put_tty_queue_nolock(c, tty);
+ put_tty_queue_nolock(c, ldata);
spin_unlock_irqrestore(&ldata->read_lock, flags);
}

@@ -699,16 +696,15 @@ static void process_echoes(struct tty_struct *tty)
/**
* add_echo_byte - add a byte to the echo buffer
* @c: unicode byte to echo
- * @tty: terminal device
+ * @ldata: n_tty data
*
* Add a character or operation byte to the echo buffer.
*
* Should be called under the echo lock to protect the echo buffer.
*/

-static void add_echo_byte(unsigned char c, struct tty_struct *tty)
+static void add_echo_byte(unsigned char c, struct n_tty_data *ldata)
{
- struct n_tty_data *ldata = tty->disc_data;
int new_byte_pos;

if (ldata->echo_cnt == N_TTY_BUF_SIZE) {
@@ -746,28 +742,24 @@ static void add_echo_byte(unsigned char c, struct tty_struct *tty)

/**
* echo_move_back_col - add operation to move back a column
- * @tty: terminal device
+ * @ldata: n_tty data
*
* Add an operation to the echo buffer to move back one column.
*
* Locking: echo_lock to protect the echo buffer
*/

-static void echo_move_back_col(struct tty_struct *tty)
+static void echo_move_back_col(struct n_tty_data *ldata)
{
- struct n_tty_data *ldata = tty->disc_data;
-
mutex_lock(&ldata->echo_lock);
-
- add_echo_byte(ECHO_OP_START, tty);
- add_echo_byte(ECHO_OP_MOVE_BACK_COL, tty);
-
+ add_echo_byte(ECHO_OP_START, ldata);
+ add_echo_byte(ECHO_OP_MOVE_BACK_COL, ldata);
mutex_unlock(&ldata->echo_lock);
}

/**
* echo_set_canon_col - add operation to set the canon column
- * @tty: terminal device
+ * @ldata: n_tty data
*
* Add an operation to the echo buffer to set the canon column
* to the current column.
@@ -775,15 +767,11 @@ static void echo_move_back_col(struct tty_struct *tty)
* Locking: echo_lock to protect the echo buffer
*/

-static void echo_set_canon_col(struct tty_struct *tty)
+static void echo_set_canon_col(struct n_tty_data *ldata)
{
- struct n_tty_data *ldata = tty->disc_data;
-
mutex_lock(&ldata->echo_lock);
-
- add_echo_byte(ECHO_OP_START, tty);
- add_echo_byte(ECHO_OP_SET_CANON_COL, tty);
-
+ add_echo_byte(ECHO_OP_START, ldata);
+ add_echo_byte(ECHO_OP_SET_CANON_COL, ldata);
mutex_unlock(&ldata->echo_lock);
}

@@ -791,7 +779,7 @@ static void echo_set_canon_col(struct tty_struct *tty)
* echo_erase_tab - add operation to erase a tab
* @num_chars: number of character columns already used
* @after_tab: true if num_chars starts after a previous tab
- * @tty: terminal device
+ * @ldata: n_tty data
*
* Add an operation to the echo buffer to erase a tab.
*
@@ -805,14 +793,12 @@ static void echo_set_canon_col(struct tty_struct *tty)
*/

static void echo_erase_tab(unsigned int num_chars, int after_tab,
- struct tty_struct *tty)
+ struct n_tty_data *ldata)
{
- struct n_tty_data *ldata = tty->disc_data;
-
mutex_lock(&ldata->echo_lock);

- add_echo_byte(ECHO_OP_START, tty);
- add_echo_byte(ECHO_OP_ERASE_TAB, tty);
+ add_echo_byte(ECHO_OP_START, ldata);
+ add_echo_byte(ECHO_OP_ERASE_TAB, ldata);

/* We only need to know this modulo 8 (tab spacing) */
num_chars &= 7;
@@ -821,7 +807,7 @@ static void echo_erase_tab(unsigned int num_chars, int after_tab,
if (after_tab)
num_chars |= 0x80;

- add_echo_byte(num_chars, tty);
+ add_echo_byte(num_chars, ldata);

mutex_unlock(&ldata->echo_lock);
}
@@ -839,19 +825,15 @@ static void echo_erase_tab(unsigned int num_chars, int after_tab,
* Locking: echo_lock to protect the echo buffer
*/

-static void echo_char_raw(unsigned char c, struct tty_struct *tty)
+static void echo_char_raw(unsigned char c, struct n_tty_data *ldata)
{
- struct n_tty_data *ldata = tty->disc_data;
-
mutex_lock(&ldata->echo_lock);
-
if (c == ECHO_OP_START) {
- add_echo_byte(ECHO_OP_START, tty);
- add_echo_byte(ECHO_OP_START, tty);
+ add_echo_byte(ECHO_OP_START, ldata);
+ add_echo_byte(ECHO_OP_START, ldata);
} else {
- add_echo_byte(c, tty);
+ add_echo_byte(c, ldata);
}
-
mutex_unlock(&ldata->echo_lock);
}

@@ -876,12 +858,12 @@ static void echo_char(unsigned char c, struct tty_struct *tty)
mutex_lock(&ldata->echo_lock);

if (c == ECHO_OP_START) {
- add_echo_byte(ECHO_OP_START, tty);
- add_echo_byte(ECHO_OP_START, tty);
+ add_echo_byte(ECHO_OP_START, ldata);
+ add_echo_byte(ECHO_OP_START, ldata);
} else {
if (L_ECHOCTL(tty) && iscntrl(c) && c != '\t')
- add_echo_byte(ECHO_OP_START, tty);
- add_echo_byte(c, tty);
+ add_echo_byte(ECHO_OP_START, ldata);
+ add_echo_byte(c, ldata);
}

mutex_unlock(&ldata->echo_lock);
@@ -889,14 +871,13 @@ static void echo_char(unsigned char c, struct tty_struct *tty)

/**
* finish_erasing - complete erase
- * @tty: tty doing the erase
+ * @ldata: n_tty data
*/

-static inline void finish_erasing(struct tty_struct *tty)
+static inline void finish_erasing(struct n_tty_data *ldata)
{
- struct n_tty_data *ldata = tty->disc_data;
if (ldata->erasing) {
- echo_char_raw('/', tty);
+ echo_char_raw('/', ldata);
ldata->erasing = 0;
}
}
@@ -944,11 +925,11 @@ static void eraser(unsigned char c, struct tty_struct *tty)
(N_TTY_BUF_SIZE - 1));
ldata->read_head = ldata->canon_head;
spin_unlock_irqrestore(&ldata->read_lock, flags);
- finish_erasing(tty);
+ finish_erasing(ldata);
echo_char(KILL_CHAR(tty), tty);
/* Add a newline if ECHOK is on and ECHOKE is off. */
if (L_ECHOK(tty))
- echo_char_raw('\n', tty);
+ echo_char_raw('\n', ldata);
return;
}
kill_type = KILL;
@@ -984,15 +965,16 @@ static void eraser(unsigned char c, struct tty_struct *tty)
if (L_ECHO(tty)) {
if (L_ECHOPRT(tty)) {
if (!ldata->erasing) {
- echo_char_raw('\\', tty);
+ echo_char_raw('\\', ldata);
ldata->erasing = 1;
}
/* if cnt > 1, output a multi-byte character */
echo_char(c, tty);
while (--cnt > 0) {
head = (head+1) & (N_TTY_BUF_SIZE-1);
- echo_char_raw(ldata->read_buf[head], tty);
- echo_move_back_col(tty);
+ echo_char_raw(ldata->read_buf[head],
+ ldata);
+ echo_move_back_col(ldata);
}
} else if (kill_type == ERASE && !L_ECHOE(tty)) {
echo_char(ERASE_CHAR(tty), tty);
@@ -1021,17 +1003,17 @@ static void eraser(unsigned char c, struct tty_struct *tty)
num_chars++;
}
}
- echo_erase_tab(num_chars, after_tab, tty);
+ echo_erase_tab(num_chars, after_tab, ldata);
} else {
if (iscntrl(c) && L_ECHOCTL(tty)) {
- echo_char_raw('\b', tty);
- echo_char_raw(' ', tty);
- echo_char_raw('\b', tty);
+ echo_char_raw('\b', ldata);
+ echo_char_raw(' ', ldata);
+ echo_char_raw('\b', ldata);
}
if (!iscntrl(c) || L_ECHOCTL(tty)) {
- echo_char_raw('\b', tty);
- echo_char_raw(' ', tty);
- echo_char_raw('\b', tty);
+ echo_char_raw('\b', ldata);
+ echo_char_raw(' ', ldata);
+ echo_char_raw('\b', ldata);
}
}
}
@@ -1039,7 +1021,7 @@ static void eraser(unsigned char c, struct tty_struct *tty)
break;
}
if (ldata->read_head == ldata->canon_head && L_ECHO(tty))
- finish_erasing(tty);
+ finish_erasing(ldata);
}

/**
@@ -1078,6 +1060,8 @@ static inline void isig(int sig, struct tty_struct *tty, int flush)

static inline void n_tty_receive_break(struct tty_struct *tty)
{
+ struct n_tty_data *ldata = tty->disc_data;
+
if (I_IGNBRK(tty))
return;
if (I_BRKINT(tty)) {
@@ -1085,10 +1069,10 @@ static inline void n_tty_receive_break(struct tty_struct *tty)
return;
}
if (I_PARMRK(tty)) {
- put_tty_queue('\377', tty);
- put_tty_queue('\0', tty);
+ put_tty_queue('\377', ldata);
+ put_tty_queue('\0', ldata);
}
- put_tty_queue('\0', tty);
+ put_tty_queue('\0', ldata);
wake_up_interruptible(&tty->read_wait);
}

@@ -1132,16 +1116,18 @@ static inline void n_tty_receive_overrun(struct tty_struct *tty)
static inline void n_tty_receive_parity_error(struct tty_struct *tty,
unsigned char c)
{
+ struct n_tty_data *ldata = tty->disc_data;
+
if (I_IGNPAR(tty))
return;
if (I_PARMRK(tty)) {
- put_tty_queue('\377', tty);
- put_tty_queue('\0', tty);
- put_tty_queue(c, tty);
+ put_tty_queue('\377', ldata);
+ put_tty_queue('\0', ldata);
+ put_tty_queue(c, ldata);
} else if (I_INPCK(tty))
- put_tty_queue('\0', tty);
+ put_tty_queue('\0', ldata);
else
- put_tty_queue(c, tty);
+ put_tty_queue(c, ldata);
wake_up_interruptible(&tty->read_wait);
}

@@ -1162,7 +1148,7 @@ static inline void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
int parmrk;

if (ldata->raw) {
- put_tty_queue(c, tty);
+ put_tty_queue(c, ldata);
return;
}

@@ -1172,7 +1158,7 @@ static inline void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
c = tolower(c);

if (L_EXTPROC(tty)) {
- put_tty_queue(c, tty);
+ put_tty_queue(c, ldata);
return;
}

@@ -1210,16 +1196,16 @@ static inline void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
return;
}
if (L_ECHO(tty)) {
- finish_erasing(tty);
+ finish_erasing(ldata);
/* Record the column of first canon char. */
if (ldata->canon_head == ldata->read_head)
- echo_set_canon_col(tty);
+ echo_set_canon_col(ldata);
echo_char(c, tty);
process_echoes(tty);
}
if (parmrk)
- put_tty_queue(c, tty);
- put_tty_queue(c, tty);
+ put_tty_queue(c, ldata);
+ put_tty_queue(c, ldata);
return;
}

@@ -1285,10 +1271,10 @@ send_signal:
if (c == LNEXT_CHAR(tty) && L_IEXTEN(tty)) {
ldata->lnext = 1;
if (L_ECHO(tty)) {
- finish_erasing(tty);
+ finish_erasing(ldata);
if (L_ECHOCTL(tty)) {
- echo_char_raw('^', tty);
- echo_char_raw('\b', tty);
+ echo_char_raw('^', ldata);
+ echo_char_raw('\b', ldata);
process_echoes(tty);
}
}
@@ -1298,9 +1284,9 @@ send_signal:
L_IEXTEN(tty)) {
unsigned long tail = ldata->canon_head;

- finish_erasing(tty);
+ finish_erasing(ldata);
echo_char(c, tty);
- echo_char_raw('\n', tty);
+ echo_char_raw('\n', ldata);
while (tail != ldata->read_head) {
echo_char(ldata->read_buf[tail], tty);
tail = (tail+1) & (N_TTY_BUF_SIZE-1);
@@ -1315,7 +1301,7 @@ send_signal:
return;
}
if (L_ECHO(tty) || L_ECHONL(tty)) {
- echo_char_raw('\n', tty);
+ echo_char_raw('\n', ldata);
process_echoes(tty);
}
goto handle_newline;
@@ -1343,7 +1329,7 @@ send_signal:
if (L_ECHO(tty)) {
/* Record the column of first canon char. */
if (ldata->canon_head == ldata->read_head)
- echo_set_canon_col(tty);
+ echo_set_canon_col(ldata);
echo_char(c, tty);
process_echoes(tty);
}
@@ -1352,12 +1338,12 @@ send_signal:
* EOL_CHAR and EOL2_CHAR?
*/
if (parmrk)
- put_tty_queue(c, tty);
+ put_tty_queue(c, ldata);

handle_newline:
spin_lock_irqsave(&ldata->read_lock, flags);
set_bit(ldata->read_head, ldata->read_flags);
- put_tty_queue_nolock(c, tty);
+ put_tty_queue_nolock(c, ldata);
ldata->canon_head = ldata->read_head;
ldata->canon_data++;
spin_unlock_irqrestore(&ldata->read_lock, flags);
@@ -1376,22 +1362,22 @@ handle_newline:
return;
}
if (L_ECHO(tty)) {
- finish_erasing(tty);
+ finish_erasing(ldata);
if (c == '\n')
- echo_char_raw('\n', tty);
+ echo_char_raw('\n', ldata);
else {
/* Record the column of first canon char. */
if (ldata->canon_head == ldata->read_head)
- echo_set_canon_col(tty);
+ echo_set_canon_col(ldata);
echo_char(c, tty);
}
process_echoes(tty);
}

if (parmrk)
- put_tty_queue(c, tty);
+ put_tty_queue(c, ldata);

- put_tty_queue(c, tty);
+ put_tty_queue(c, ldata);
}


@@ -2140,9 +2126,8 @@ static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
return mask;
}

-static unsigned long inq_canon(struct tty_struct *tty)
+static unsigned long inq_canon(struct n_tty_data *ldata)
{
- struct n_tty_data *ldata = tty->disc_data;
int nr, head, tail;

if (!ldata->canon_data)
@@ -2173,7 +2158,7 @@ static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
/* FIXME: Locking */
retval = ldata->read_cnt;
if (L_ICANON(tty))
- retval = inq_canon(tty);
+ retval = inq_canon(ldata);
return put_user(retval, (unsigned int __user *) arg);
default:
return n_tty_ioctl_helper(tty, file, cmd, arg);
--
1.7.12.3

2012-10-18 20:27:12

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 21/21] TTY: move tty buffers to tty_port

So this is it. The big step why we did all the work over the past
kernel releases. Now everything is prepared, so nothing protects us
from doing that big step.

| | \ \ nnnn/^l | |
| | \ / / | |
| '-,.__ => \/ ,-` => | '-,.__
| O __.´´) ( .` | O __.´´)
~~~ ~~ `` ~~~ ~~
The buffers are now in the tty_port structure and we can start
teaching the buffer helpers (insert char/string, flip etc.) to use
tty_port instead of tty_struct all around.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/n_tty.c | 7 +++-
drivers/tty/pty.c | 2 +-
drivers/tty/tty_buffer.c | 102 ++++++++++++++++++++++++-----------------------
drivers/tty/tty_io.c | 2 -
drivers/tty/tty_ldisc.c | 10 ++---
drivers/tty/tty_port.c | 2 +
include/linux/tty.h | 6 +--
include/linux/tty_flip.h | 2 +-
8 files changed, 70 insertions(+), 63 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 531e539..60b076c 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -149,8 +149,11 @@ static void n_tty_set_room(struct tty_struct *tty)
tty->receive_room = left;

/* Did this open up the receive buffer? We may need to flip */
- if (left && !old_left)
- schedule_work(&tty->buf.work);
+ if (left && !old_left) {
+ WARN_RATELIMIT(tty->port->itty == NULL,
+ "scheduling with invalid itty");
+ schedule_work(&tty->port->buf.work);
+ }
}

static void put_tty_queue_nolock(unsigned char c, struct n_tty_data *ldata)
diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index c326908..4219f04 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -93,7 +93,7 @@ static void pty_unthrottle(struct tty_struct *tty)

static int pty_space(struct tty_struct *to)
{
- int n = 8192 - to->buf.memory_used;
+ int n = 8192 - to->port->buf.memory_used;
if (n < 0)
return 0;
return n;
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index ffaa441..3876d3b 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -27,9 +27,9 @@
* Locking: none
*/

-void tty_buffer_free_all(struct tty_struct *tty)
+void tty_buffer_free_all(struct tty_port *port)
{
- struct tty_bufhead *buf = &tty->buf;
+ struct tty_bufhead *buf = &port->buf;
struct tty_buffer *thead;

while ((thead = buf->head) != NULL) {
@@ -56,11 +56,11 @@ void tty_buffer_free_all(struct tty_struct *tty)
* Locking: Caller must hold tty->buf.lock
*/

-static struct tty_buffer *tty_buffer_alloc(struct tty_struct *tty, size_t size)
+static struct tty_buffer *tty_buffer_alloc(struct tty_port *port, size_t size)
{
struct tty_buffer *p;

- if (tty->buf.memory_used + size > 65536)
+ if (port->buf.memory_used + size > 65536)
return NULL;
p = kmalloc(sizeof(struct tty_buffer) + 2 * size, GFP_ATOMIC);
if (p == NULL)
@@ -72,7 +72,7 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_struct *tty, size_t size)
p->read = 0;
p->char_buf_ptr = (char *)(p->data);
p->flag_buf_ptr = (unsigned char *)p->char_buf_ptr + size;
- tty->buf.memory_used += size;
+ port->buf.memory_used += size;
return p;
}

@@ -87,9 +87,9 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_struct *tty, size_t size)
* Locking: Caller must hold tty->buf.lock
*/

-static void tty_buffer_free(struct tty_struct *tty, struct tty_buffer *b)
+static void tty_buffer_free(struct tty_port *port, struct tty_buffer *b)
{
- struct tty_bufhead *buf = &tty->buf;
+ struct tty_bufhead *buf = &port->buf;

/* Dumb strategy for now - should keep some stats */
buf->memory_used -= b->size;
@@ -114,14 +114,14 @@ static void tty_buffer_free(struct tty_struct *tty, struct tty_buffer *b)
* Locking: Caller must hold tty->buf.lock
*/

-static void __tty_buffer_flush(struct tty_struct *tty)
+static void __tty_buffer_flush(struct tty_port *port)
{
- struct tty_bufhead *buf = &tty->buf;
+ struct tty_bufhead *buf = &port->buf;
struct tty_buffer *thead;

while ((thead = buf->head) != NULL) {
buf->head = thead->next;
- tty_buffer_free(tty, thead);
+ tty_buffer_free(port, thead);
}
buf->tail = NULL;
}
@@ -140,7 +140,7 @@ static void __tty_buffer_flush(struct tty_struct *tty)
void tty_buffer_flush(struct tty_struct *tty)
{
struct tty_port *port = tty->port;
- struct tty_bufhead *buf = &tty->buf;
+ struct tty_bufhead *buf = &port->buf;
unsigned long flags;

spin_lock_irqsave(&buf->lock, flags);
@@ -155,7 +155,7 @@ void tty_buffer_flush(struct tty_struct *tty)
test_bit(TTYP_FLUSHPENDING, &port->iflags) == 0);
return;
} else
- __tty_buffer_flush(tty);
+ __tty_buffer_flush(port);
spin_unlock_irqrestore(&buf->lock, flags);
}

@@ -171,9 +171,9 @@ void tty_buffer_flush(struct tty_struct *tty)
* Locking: Caller must hold tty->buf.lock
*/

-static struct tty_buffer *tty_buffer_find(struct tty_struct *tty, size_t size)
+static struct tty_buffer *tty_buffer_find(struct tty_port *port, size_t size)
{
- struct tty_buffer **tbh = &tty->buf.free;
+ struct tty_buffer **tbh = &port->buf.free;
while ((*tbh) != NULL) {
struct tty_buffer *t = *tbh;
if (t->size >= size) {
@@ -182,14 +182,14 @@ static struct tty_buffer *tty_buffer_find(struct tty_struct *tty, size_t size)
t->used = 0;
t->commit = 0;
t->read = 0;
- tty->buf.memory_used += t->size;
+ port->buf.memory_used += t->size;
return t;
}
tbh = &((*tbh)->next);
}
/* Round the buffer size out */
size = (size + 0xFF) & ~0xFF;
- return tty_buffer_alloc(tty, size);
+ return tty_buffer_alloc(port, size);
/* Should possibly check if this fails for the largest buffer we
have queued and recycle that ? */
}
@@ -200,11 +200,11 @@ static struct tty_buffer *tty_buffer_find(struct tty_struct *tty, size_t size)
*
* Make at least size bytes of linear space available for the tty
* buffer. If we fail return the size we managed to find.
- * Locking: Caller must hold tty->buf.lock
+ * Locking: Caller must hold port->buf.lock
*/
-static int __tty_buffer_request_room(struct tty_struct *tty, size_t size)
+static int __tty_buffer_request_room(struct tty_port *port, size_t size)
{
- struct tty_bufhead *buf = &tty->buf;
+ struct tty_bufhead *buf = &port->buf;
struct tty_buffer *b, *n;
int left;
/* OPTIMISATION: We could keep a per tty "zero" sized buffer to
@@ -218,7 +218,7 @@ static int __tty_buffer_request_room(struct tty_struct *tty, size_t size)

if (left < size) {
/* This is the slow path - looking for new buffers to use */
- if ((n = tty_buffer_find(tty, size)) != NULL) {
+ if ((n = tty_buffer_find(port, size)) != NULL) {
if (b != NULL) {
b->next = n;
b->commit = b->used;
@@ -241,16 +241,17 @@ static int __tty_buffer_request_room(struct tty_struct *tty, size_t size)
* Make at least size bytes of linear space available for the tty
* buffer. If we fail return the size we managed to find.
*
- * Locking: Takes tty->buf.lock
+ * Locking: Takes port->buf.lock
*/
int tty_buffer_request_room(struct tty_struct *tty, size_t size)
{
+ struct tty_port *port = tty->port;
unsigned long flags;
int length;

- spin_lock_irqsave(&tty->buf.lock, flags);
- length = __tty_buffer_request_room(tty, size);
- spin_unlock_irqrestore(&tty->buf.lock, flags);
+ spin_lock_irqsave(&port->buf.lock, flags);
+ length = __tty_buffer_request_room(port, size);
+ spin_unlock_irqrestore(&port->buf.lock, flags);
return length;
}
EXPORT_SYMBOL_GPL(tty_buffer_request_room);
@@ -265,13 +266,13 @@ EXPORT_SYMBOL_GPL(tty_buffer_request_room);
* Queue a series of bytes to the tty buffering. All the characters
* passed are marked with the supplied flag. Returns the number added.
*
- * Locking: Called functions may take tty->buf.lock
+ * Locking: Called functions may take port->buf.lock
*/

int tty_insert_flip_string_fixed_flag(struct tty_struct *tty,
const unsigned char *chars, char flag, size_t size)
{
- struct tty_bufhead *buf = &tty->buf;
+ struct tty_bufhead *buf = &tty->port->buf;
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
@@ -280,7 +281,7 @@ int tty_insert_flip_string_fixed_flag(struct tty_struct *tty,
struct tty_buffer *tb;

spin_lock_irqsave(&buf->lock, flags);
- space = __tty_buffer_request_room(tty, goal);
+ space = __tty_buffer_request_room(tty->port, goal);
tb = buf->tail;
/* If there is no space then tb may be NULL */
if (unlikely(space == 0)) {
@@ -311,13 +312,13 @@ EXPORT_SYMBOL(tty_insert_flip_string_fixed_flag);
* the flags array indicates the status of the character. Returns the
* number added.
*
- * Locking: Called functions may take tty->buf.lock
+ * Locking: Called functions may take port->buf.lock
*/

int tty_insert_flip_string_flags(struct tty_struct *tty,
const unsigned char *chars, const char *flags, size_t size)
{
- struct tty_bufhead *buf = &tty->buf;
+ struct tty_bufhead *buf = &tty->port->buf;
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
@@ -326,7 +327,7 @@ int tty_insert_flip_string_flags(struct tty_struct *tty,
struct tty_buffer *tb;

spin_lock_irqsave(&buf->lock, __flags);
- space = __tty_buffer_request_room(tty, goal);
+ space = __tty_buffer_request_room(tty->port, goal);
tb = buf->tail;
/* If there is no space then tb may be NULL */
if (unlikely(space == 0)) {
@@ -355,12 +356,12 @@ EXPORT_SYMBOL(tty_insert_flip_string_flags);
* ldisc side of the queue. It then schedules those characters for
* processing by the line discipline.
*
- * Locking: Takes tty->buf.lock
+ * Locking: Takes port->buf.lock
*/

void tty_schedule_flip(struct tty_struct *tty)
{
- struct tty_bufhead *buf = &tty->buf;
+ struct tty_bufhead *buf = &tty->port->buf;
unsigned long flags;

spin_lock_irqsave(&buf->lock, flags);
@@ -383,19 +384,19 @@ EXPORT_SYMBOL(tty_schedule_flip);
* that need their own block copy routines into the buffer. There is no
* guarantee the buffer is a DMA target!
*
- * Locking: May call functions taking tty->buf.lock
+ * Locking: May call functions taking port->buf.lock
*/

int tty_prepare_flip_string(struct tty_struct *tty, unsigned char **chars,
- size_t size)
+ size_t size)
{
- struct tty_bufhead *buf = &tty->buf;
+ struct tty_bufhead *buf = &tty->port->buf;
int space;
unsigned long flags;
struct tty_buffer *tb;

spin_lock_irqsave(&buf->lock, flags);
- space = __tty_buffer_request_room(tty, size);
+ space = __tty_buffer_request_room(tty->port, size);

tb = buf->tail;
if (likely(space)) {
@@ -421,19 +422,19 @@ EXPORT_SYMBOL_GPL(tty_prepare_flip_string);
* that need their own block copy routines into the buffer. There is no
* guarantee the buffer is a DMA target!
*
- * Locking: May call functions taking tty->buf.lock
+ * Locking: May call functions taking port->buf.lock
*/

int tty_prepare_flip_string_flags(struct tty_struct *tty,
unsigned char **chars, char **flags, size_t size)
{
- struct tty_bufhead *buf = &tty->buf;
+ struct tty_bufhead *buf = &tty->port->buf;
int space;
unsigned long __flags;
struct tty_buffer *tb;

spin_lock_irqsave(&buf->lock, __flags);
- space = __tty_buffer_request_room(tty, size);
+ space = __tty_buffer_request_room(tty->port, size);

tb = buf->tail;
if (likely(space)) {
@@ -462,13 +463,16 @@ EXPORT_SYMBOL_GPL(tty_prepare_flip_string_flags);

static void flush_to_ldisc(struct work_struct *work)
{
- struct tty_struct *tty =
- container_of(work, struct tty_struct, buf.work);
- struct tty_port *port = tty->port;
- struct tty_bufhead *buf = &tty->buf;
+ struct tty_port *port = container_of(work, struct tty_port, buf.work);
+ struct tty_bufhead *buf = &port->buf;
+ struct tty_struct *tty;
unsigned long flags;
struct tty_ldisc *disc;

+ tty = port->itty;
+ if (WARN_RATELIMIT(tty == NULL, "tty is NULL"))
+ return;
+
disc = tty_ldisc_ref(tty);
if (disc == NULL) /* !TTY_LDISC */
return;
@@ -487,7 +491,7 @@ static void flush_to_ldisc(struct work_struct *work)
if (head->next == NULL)
break;
buf->head = head->next;
- tty_buffer_free(tty, head);
+ tty_buffer_free(port, head);
continue;
}
/* Ldisc or user is trying to flush the buffers
@@ -513,7 +517,7 @@ static void flush_to_ldisc(struct work_struct *work)
/* We may have a deferred request to flush the input buffer,
if so pull the chain under the lock and empty the queue */
if (test_bit(TTYP_FLUSHPENDING, &port->iflags)) {
- __tty_buffer_flush(tty);
+ __tty_buffer_flush(port);
clear_bit(TTYP_FLUSHPENDING, &port->iflags);
wake_up(&tty->read_wait);
}
@@ -532,7 +536,7 @@ static void flush_to_ldisc(struct work_struct *work)
*/
void tty_flush_to_ldisc(struct tty_struct *tty)
{
- flush_work(&tty->buf.work);
+ flush_work(&tty->port->buf.work);
}

/**
@@ -550,7 +554,7 @@ void tty_flush_to_ldisc(struct tty_struct *tty)

void tty_flip_buffer_push(struct tty_struct *tty)
{
- struct tty_bufhead *buf = &tty->buf;
+ struct tty_bufhead *buf = &tty->port->buf;
unsigned long flags;

spin_lock_irqsave(&buf->lock, flags);
@@ -575,9 +579,9 @@ EXPORT_SYMBOL(tty_flip_buffer_push);
* Locking: none
*/

-void tty_buffer_init(struct tty_struct *tty)
+void tty_buffer_init(struct tty_port *port)
{
- struct tty_bufhead *buf = &tty->buf;
+ struct tty_bufhead *buf = &port->buf;

spin_lock_init(&buf->lock);
buf->head = NULL;
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 202008f..a3eba7f 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -186,7 +186,6 @@ void free_tty_struct(struct tty_struct *tty)
if (tty->dev)
put_device(tty->dev);
kfree(tty->write_buf);
- tty_buffer_free_all(tty);
tty->magic = 0xDEADDEAD;
kfree(tty);
}
@@ -2935,7 +2934,6 @@ void initialize_tty_struct(struct tty_struct *tty,
tty_ldisc_init(tty);
tty->session = NULL;
tty->pgrp = NULL;
- tty_buffer_init(tty);
mutex_init(&tty->legacy_mutex);
mutex_init(&tty->termios_mutex);
mutex_init(&tty->ldisc_mutex);
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 47e3968..f4e6754 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -512,7 +512,7 @@ static void tty_ldisc_restore(struct tty_struct *tty, struct tty_ldisc *old)
static int tty_ldisc_halt(struct tty_struct *tty)
{
clear_bit(TTY_LDISC, &tty->flags);
- return cancel_work_sync(&tty->buf.work);
+ return cancel_work_sync(&tty->port->buf.work);
}

/**
@@ -525,7 +525,7 @@ static void tty_ldisc_flush_works(struct tty_struct *tty)
{
flush_work(&tty->hangup_work);
flush_work(&tty->SAK_work);
- flush_work(&tty->buf.work);
+ flush_work(&tty->port->buf.work);
}

/**
@@ -704,9 +704,9 @@ enable:
/* Restart the work queue in case no characters kick it off. Safe if
already running */
if (work)
- schedule_work(&tty->buf.work);
+ schedule_work(&tty->port->buf.work);
if (o_work)
- schedule_work(&o_tty->buf.work);
+ schedule_work(&o_tty->port->buf.work);
mutex_unlock(&tty->ldisc_mutex);
tty_unlock(tty);
return retval;
@@ -817,7 +817,7 @@ void tty_ldisc_hangup(struct tty_struct *tty)
*/
clear_bit(TTY_LDISC, &tty->flags);
tty_unlock(tty);
- cancel_work_sync(&tty->buf.work);
+ cancel_work_sync(&tty->port->buf.work);
mutex_unlock(&tty->ldisc_mutex);
retry:
tty_lock(tty);
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index d7bdd8d..416b42f 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -21,6 +21,7 @@
void tty_port_init(struct tty_port *port)
{
memset(port, 0, sizeof(*port));
+ tty_buffer_init(port);
init_waitqueue_head(&port->open_wait);
init_waitqueue_head(&port->close_wait);
init_waitqueue_head(&port->delta_msr_wait);
@@ -126,6 +127,7 @@ static void tty_port_destructor(struct kref *kref)
struct tty_port *port = container_of(kref, struct tty_port, kref);
if (port->xmit_buf)
free_page((unsigned long)port->xmit_buf);
+ tty_buffer_free_all(port);
if (port->ops->destruct)
port->ops->destruct(port);
else
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 9be74d6..d7ff88f 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -188,6 +188,7 @@ struct tty_port_operations {
};

struct tty_port {
+ struct tty_bufhead buf; /* Locked internally */
struct tty_struct *tty; /* Back pointer */
struct tty_struct *itty; /* internal back ptr */
const struct tty_port_operations *ops; /* Port operations */
@@ -259,7 +260,6 @@ struct tty_struct {

struct tty_struct *link;
struct fasync_struct *fasync;
- struct tty_bufhead buf; /* Locked internally */
int alt_speed; /* For magic substitution of 38400 bps */
wait_queue_head_t write_wait;
wait_queue_head_t read_wait;
@@ -388,9 +388,9 @@ extern void disassociate_ctty(int priv);
extern void no_tty(void);
extern void tty_flip_buffer_push(struct tty_struct *tty);
extern void tty_flush_to_ldisc(struct tty_struct *tty);
-extern void tty_buffer_free_all(struct tty_struct *tty);
+extern void tty_buffer_free_all(struct tty_port *port);
extern void tty_buffer_flush(struct tty_struct *tty);
-extern void tty_buffer_init(struct tty_struct *tty);
+extern void tty_buffer_init(struct tty_port *port);
extern speed_t tty_get_baud_rate(struct tty_struct *tty);
extern speed_t tty_termios_baud_rate(struct ktermios *termios);
extern speed_t tty_termios_input_baud_rate(struct ktermios *termios);
diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h
index 9239d03..2002344 100644
--- a/include/linux/tty_flip.h
+++ b/include/linux/tty_flip.h
@@ -11,7 +11,7 @@ void tty_schedule_flip(struct tty_struct *tty);
static inline int tty_insert_flip_char(struct tty_struct *tty,
unsigned char ch, char flag)
{
- struct tty_buffer *tb = tty->buf.tail;
+ struct tty_buffer *tb = tty->port->buf.tail;
if (tb && tb->used < tb->size) {
tb->flag_buf_ptr[tb->used] = flag;
tb->char_buf_ptr[tb->used++] = ch;
--
1.7.12.3

2012-10-18 20:27:10

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 16/21] TTY: move ldisc data from tty_struct: locks

atomic_write_lock is not n_tty specific, so move it up in the
tty_struct.

And since these are the last ones to move, remove also the comment
saying there are some ldisc' members. There are none now.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/n_tty.c | 123 ++++++++++++++++++++++++++++++---------------------
drivers/tty/tty_io.c | 4 --
include/linux/tty.h | 11 +----
3 files changed, 73 insertions(+), 65 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 4794537..0a6fcda 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -96,6 +96,11 @@ struct n_tty_data {
int canon_data;
unsigned long canon_head;
unsigned int canon_column;
+
+ struct mutex atomic_read_lock;
+ struct mutex output_lock;
+ struct mutex echo_lock;
+ spinlock_t read_lock;
};

static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
@@ -171,14 +176,15 @@ static void put_tty_queue_nolock(unsigned char c, struct tty_struct *tty)

static void put_tty_queue(unsigned char c, struct tty_struct *tty)
{
+ struct n_tty_data *ldata = tty->disc_data;
unsigned long flags;
/*
* The problem of stomping on the buffers ends here.
* Why didn't anyone see this one coming? --AJK
*/
- spin_lock_irqsave(&tty->read_lock, flags);
+ spin_lock_irqsave(&ldata->read_lock, flags);
put_tty_queue_nolock(c, tty);
- spin_unlock_irqrestore(&tty->read_lock, flags);
+ spin_unlock_irqrestore(&ldata->read_lock, flags);
}

/**
@@ -212,13 +218,13 @@ static void reset_buffer_flags(struct tty_struct *tty)
struct n_tty_data *ldata = tty->disc_data;
unsigned long flags;

- spin_lock_irqsave(&tty->read_lock, flags);
+ spin_lock_irqsave(&ldata->read_lock, flags);
ldata->read_head = ldata->read_tail = ldata->read_cnt = 0;
- spin_unlock_irqrestore(&tty->read_lock, flags);
+ spin_unlock_irqrestore(&ldata->read_lock, flags);

- mutex_lock(&tty->echo_lock);
+ mutex_lock(&ldata->echo_lock);
ldata->echo_pos = ldata->echo_cnt = ldata->echo_overrun = 0;
- mutex_unlock(&tty->echo_lock);
+ mutex_unlock(&ldata->echo_lock);

ldata->canon_head = ldata->canon_data = ldata->erasing = 0;
bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);
@@ -270,7 +276,7 @@ static ssize_t n_tty_chars_in_buffer(struct tty_struct *tty)
unsigned long flags;
ssize_t n = 0;

- spin_lock_irqsave(&tty->read_lock, flags);
+ spin_lock_irqsave(&ldata->read_lock, flags);
if (!ldata->icanon) {
n = ldata->read_cnt;
} else if (ldata->canon_data) {
@@ -278,7 +284,7 @@ static ssize_t n_tty_chars_in_buffer(struct tty_struct *tty)
ldata->canon_head - ldata->read_tail :
ldata->canon_head + (N_TTY_BUF_SIZE - ldata->read_tail);
}
- spin_unlock_irqrestore(&tty->read_lock, flags);
+ spin_unlock_irqrestore(&ldata->read_lock, flags);
return n;
}

@@ -408,14 +414,15 @@ static int do_output_char(unsigned char c, struct tty_struct *tty, int space)

static int process_output(unsigned char c, struct tty_struct *tty)
{
+ struct n_tty_data *ldata = tty->disc_data;
int space, retval;

- mutex_lock(&tty->output_lock);
+ mutex_lock(&ldata->output_lock);

space = tty_write_room(tty);
retval = do_output_char(c, tty, space);

- mutex_unlock(&tty->output_lock);
+ mutex_unlock(&ldata->output_lock);
if (retval < 0)
return -1;
else
@@ -449,11 +456,11 @@ static ssize_t process_output_block(struct tty_struct *tty,
int i;
const unsigned char *cp;

- mutex_lock(&tty->output_lock);
+ mutex_lock(&ldata->output_lock);

space = tty_write_room(tty);
if (!space) {
- mutex_unlock(&tty->output_lock);
+ mutex_unlock(&ldata->output_lock);
return 0;
}
if (nr > space)
@@ -496,7 +503,7 @@ static ssize_t process_output_block(struct tty_struct *tty,
break_out:
i = tty->ops->write(tty, buf, i);

- mutex_unlock(&tty->output_lock);
+ mutex_unlock(&ldata->output_lock);
return i;
}

@@ -536,8 +543,8 @@ static void process_echoes(struct tty_struct *tty)
if (!ldata->echo_cnt)
return;

- mutex_lock(&tty->output_lock);
- mutex_lock(&tty->echo_lock);
+ mutex_lock(&ldata->output_lock);
+ mutex_lock(&ldata->echo_lock);

space = tty_write_room(tty);

@@ -682,8 +689,8 @@ static void process_echoes(struct tty_struct *tty)
ldata->echo_overrun = 0;
}

- mutex_unlock(&tty->echo_lock);
- mutex_unlock(&tty->output_lock);
+ mutex_unlock(&ldata->echo_lock);
+ mutex_unlock(&ldata->output_lock);

if (tty->ops->flush_chars)
tty->ops->flush_chars(tty);
@@ -748,12 +755,14 @@ static void add_echo_byte(unsigned char c, struct tty_struct *tty)

static void echo_move_back_col(struct tty_struct *tty)
{
- mutex_lock(&tty->echo_lock);
+ struct n_tty_data *ldata = tty->disc_data;
+
+ mutex_lock(&ldata->echo_lock);

add_echo_byte(ECHO_OP_START, tty);
add_echo_byte(ECHO_OP_MOVE_BACK_COL, tty);

- mutex_unlock(&tty->echo_lock);
+ mutex_unlock(&ldata->echo_lock);
}

/**
@@ -768,12 +777,14 @@ static void echo_move_back_col(struct tty_struct *tty)

static void echo_set_canon_col(struct tty_struct *tty)
{
- mutex_lock(&tty->echo_lock);
+ struct n_tty_data *ldata = tty->disc_data;
+
+ mutex_lock(&ldata->echo_lock);

add_echo_byte(ECHO_OP_START, tty);
add_echo_byte(ECHO_OP_SET_CANON_COL, tty);

- mutex_unlock(&tty->echo_lock);
+ mutex_unlock(&ldata->echo_lock);
}

/**
@@ -796,7 +807,9 @@ static void echo_set_canon_col(struct tty_struct *tty)
static void echo_erase_tab(unsigned int num_chars, int after_tab,
struct tty_struct *tty)
{
- mutex_lock(&tty->echo_lock);
+ struct n_tty_data *ldata = tty->disc_data;
+
+ mutex_lock(&ldata->echo_lock);

add_echo_byte(ECHO_OP_START, tty);
add_echo_byte(ECHO_OP_ERASE_TAB, tty);
@@ -810,7 +823,7 @@ static void echo_erase_tab(unsigned int num_chars, int after_tab,

add_echo_byte(num_chars, tty);

- mutex_unlock(&tty->echo_lock);
+ mutex_unlock(&ldata->echo_lock);
}

/**
@@ -828,7 +841,9 @@ static void echo_erase_tab(unsigned int num_chars, int after_tab,

static void echo_char_raw(unsigned char c, struct tty_struct *tty)
{
- mutex_lock(&tty->echo_lock);
+ struct n_tty_data *ldata = tty->disc_data;
+
+ mutex_lock(&ldata->echo_lock);

if (c == ECHO_OP_START) {
add_echo_byte(ECHO_OP_START, tty);
@@ -837,7 +852,7 @@ static void echo_char_raw(unsigned char c, struct tty_struct *tty)
add_echo_byte(c, tty);
}

- mutex_unlock(&tty->echo_lock);
+ mutex_unlock(&ldata->echo_lock);
}

/**
@@ -856,7 +871,9 @@ static void echo_char_raw(unsigned char c, struct tty_struct *tty)

static void echo_char(unsigned char c, struct tty_struct *tty)
{
- mutex_lock(&tty->echo_lock);
+ struct n_tty_data *ldata = tty->disc_data;
+
+ mutex_lock(&ldata->echo_lock);

if (c == ECHO_OP_START) {
add_echo_byte(ECHO_OP_START, tty);
@@ -867,7 +884,7 @@ static void echo_char(unsigned char c, struct tty_struct *tty)
add_echo_byte(c, tty);
}

- mutex_unlock(&tty->echo_lock);
+ mutex_unlock(&ldata->echo_lock);
}

/**
@@ -914,19 +931,19 @@ static void eraser(unsigned char c, struct tty_struct *tty)
kill_type = WERASE;
else {
if (!L_ECHO(tty)) {
- spin_lock_irqsave(&tty->read_lock, flags);
+ spin_lock_irqsave(&ldata->read_lock, flags);
ldata->read_cnt -= ((ldata->read_head - ldata->canon_head) &
(N_TTY_BUF_SIZE - 1));
ldata->read_head = ldata->canon_head;
- spin_unlock_irqrestore(&tty->read_lock, flags);
+ spin_unlock_irqrestore(&ldata->read_lock, flags);
return;
}
if (!L_ECHOK(tty) || !L_ECHOKE(tty) || !L_ECHOE(tty)) {
- spin_lock_irqsave(&tty->read_lock, flags);
+ spin_lock_irqsave(&ldata->read_lock, flags);
ldata->read_cnt -= ((ldata->read_head - ldata->canon_head) &
(N_TTY_BUF_SIZE - 1));
ldata->read_head = ldata->canon_head;
- spin_unlock_irqrestore(&tty->read_lock, flags);
+ spin_unlock_irqrestore(&ldata->read_lock, flags);
finish_erasing(tty);
echo_char(KILL_CHAR(tty), tty);
/* Add a newline if ECHOK is on and ECHOKE is off. */
@@ -960,10 +977,10 @@ static void eraser(unsigned char c, struct tty_struct *tty)
break;
}
cnt = (ldata->read_head - head) & (N_TTY_BUF_SIZE-1);
- spin_lock_irqsave(&tty->read_lock, flags);
+ spin_lock_irqsave(&ldata->read_lock, flags);
ldata->read_head = head;
ldata->read_cnt -= cnt;
- spin_unlock_irqrestore(&tty->read_lock, flags);
+ spin_unlock_irqrestore(&ldata->read_lock, flags);
if (L_ECHO(tty)) {
if (L_ECHOPRT(tty)) {
if (!ldata->erasing) {
@@ -1338,12 +1355,12 @@ send_signal:
put_tty_queue(c, tty);

handle_newline:
- spin_lock_irqsave(&tty->read_lock, flags);
+ spin_lock_irqsave(&ldata->read_lock, flags);
set_bit(ldata->read_head, ldata->read_flags);
put_tty_queue_nolock(c, tty);
ldata->canon_head = ldata->read_head;
ldata->canon_data++;
- spin_unlock_irqrestore(&tty->read_lock, flags);
+ spin_unlock_irqrestore(&ldata->read_lock, flags);
kill_fasync(&tty->fasync, SIGIO, POLL_IN);
if (waitqueue_active(&tty->read_wait))
wake_up_interruptible(&tty->read_wait);
@@ -1417,7 +1434,7 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
unsigned long cpuflags;

if (ldata->real_raw) {
- spin_lock_irqsave(&tty->read_lock, cpuflags);
+ spin_lock_irqsave(&ldata->read_lock, cpuflags);
i = min(N_TTY_BUF_SIZE - ldata->read_cnt,
N_TTY_BUF_SIZE - ldata->read_head);
i = min(count, i);
@@ -1433,7 +1450,7 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
memcpy(ldata->read_buf + ldata->read_head, cp, i);
ldata->read_head = (ldata->read_head + i) & (N_TTY_BUF_SIZE-1);
ldata->read_cnt += i;
- spin_unlock_irqrestore(&tty->read_lock, cpuflags);
+ spin_unlock_irqrestore(&ldata->read_lock, cpuflags);
} else {
for (i = count, p = cp, f = fp; i; i--, p++) {
if (f)
@@ -1626,6 +1643,10 @@ static int n_tty_open(struct tty_struct *tty)
goto err;

ldata->overrun_time = jiffies;
+ mutex_init(&ldata->atomic_read_lock);
+ mutex_init(&ldata->output_lock);
+ mutex_init(&ldata->echo_lock);
+ spin_lock_init(&ldata->read_lock);

/* These are ugly. Currently a malloc failure here can panic */
ldata->read_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
@@ -1677,7 +1698,7 @@ static inline int input_available_p(struct tty_struct *tty, int amt)
* buffer, and once to drain the space from the (physical) beginning of
* the buffer to head pointer.
*
- * Called under the tty->atomic_read_lock sem
+ * Called under the ldata->atomic_read_lock sem
*
*/

@@ -1693,10 +1714,10 @@ static int copy_from_read_buf(struct tty_struct *tty,
bool is_eof;

retval = 0;
- spin_lock_irqsave(&tty->read_lock, flags);
+ spin_lock_irqsave(&ldata->read_lock, flags);
n = min(ldata->read_cnt, N_TTY_BUF_SIZE - ldata->read_tail);
n = min(*nr, n);
- spin_unlock_irqrestore(&tty->read_lock, flags);
+ spin_unlock_irqrestore(&ldata->read_lock, flags);
if (n) {
retval = copy_to_user(*b, &ldata->read_buf[ldata->read_tail], n);
n -= retval;
@@ -1704,13 +1725,13 @@ static int copy_from_read_buf(struct tty_struct *tty,
ldata->read_buf[ldata->read_tail] == EOF_CHAR(tty);
tty_audit_add_data(tty, &ldata->read_buf[ldata->read_tail], n,
ldata->icanon);
- spin_lock_irqsave(&tty->read_lock, flags);
+ spin_lock_irqsave(&ldata->read_lock, flags);
ldata->read_tail = (ldata->read_tail + n) & (N_TTY_BUF_SIZE-1);
ldata->read_cnt -= n;
/* Turn single EOF into zero-length read */
if (L_EXTPROC(tty) && ldata->icanon && is_eof && !ldata->read_cnt)
n = 0;
- spin_unlock_irqrestore(&tty->read_lock, flags);
+ spin_unlock_irqrestore(&ldata->read_lock, flags);
*b += n;
*nr -= n;
}
@@ -1818,10 +1839,10 @@ do_it_again:
* Internal serialization of reads.
*/
if (file->f_flags & O_NONBLOCK) {
- if (!mutex_trylock(&tty->atomic_read_lock))
+ if (!mutex_trylock(&ldata->atomic_read_lock))
return -EAGAIN;
} else {
- if (mutex_lock_interruptible(&tty->atomic_read_lock))
+ if (mutex_lock_interruptible(&ldata->atomic_read_lock))
return -ERESTARTSYS;
}
packet = tty->packet;
@@ -1890,7 +1911,7 @@ do_it_again:

if (ldata->icanon && !L_EXTPROC(tty)) {
/* N.B. avoid overrun if nr == 0 */
- spin_lock_irqsave(&tty->read_lock, flags);
+ spin_lock_irqsave(&ldata->read_lock, flags);
while (nr && ldata->read_cnt) {
int eol;

@@ -1908,25 +1929,25 @@ do_it_again:
if (--ldata->canon_data < 0)
ldata->canon_data = 0;
}
- spin_unlock_irqrestore(&tty->read_lock, flags);
+ spin_unlock_irqrestore(&ldata->read_lock, flags);

if (!eol || (c != __DISABLED_CHAR)) {
if (tty_put_user(tty, c, b++)) {
retval = -EFAULT;
b--;
- spin_lock_irqsave(&tty->read_lock, flags);
+ spin_lock_irqsave(&ldata->read_lock, flags);
break;
}
nr--;
}
if (eol) {
tty_audit_push(tty);
- spin_lock_irqsave(&tty->read_lock, flags);
+ spin_lock_irqsave(&ldata->read_lock, flags);
break;
}
- spin_lock_irqsave(&tty->read_lock, flags);
+ spin_lock_irqsave(&ldata->read_lock, flags);
}
- spin_unlock_irqrestore(&tty->read_lock, flags);
+ spin_unlock_irqrestore(&ldata->read_lock, flags);
if (retval)
break;
} else {
@@ -1958,7 +1979,7 @@ do_it_again:
if (time)
timeout = time;
}
- mutex_unlock(&tty->atomic_read_lock);
+ mutex_unlock(&ldata->atomic_read_lock);
remove_wait_queue(&tty->read_wait, &wait);

if (!waitqueue_active(&tty->read_wait))
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 67b024c..f90b621 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2939,11 +2939,7 @@ void initialize_tty_struct(struct tty_struct *tty,
init_waitqueue_head(&tty->write_wait);
init_waitqueue_head(&tty->read_wait);
INIT_WORK(&tty->hangup_work, do_tty_hangup);
- mutex_init(&tty->atomic_read_lock);
mutex_init(&tty->atomic_write_lock);
- mutex_init(&tty->output_lock);
- mutex_init(&tty->echo_lock);
- spin_lock_init(&tty->read_lock);
spin_lock_init(&tty->ctrl_lock);
INIT_LIST_HEAD(&tty->tty_files);
INIT_WORK(&tty->SAK_work, do_SAK_work);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 226cf20..08787ec 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -235,6 +235,7 @@ struct tty_struct {
struct mutex ldisc_mutex;
struct tty_ldisc *ldisc;

+ struct mutex atomic_write_lock;
struct mutex legacy_mutex;
struct mutex termios_mutex;
spinlock_t ctrl_lock;
@@ -265,20 +266,10 @@ struct tty_struct {

#define N_TTY_BUF_SIZE 4096

- /*
- * The following is data for the N_TTY line discipline. For
- * historical reasons, this is included in the tty structure.
- * Mostly locked by the BKL.
- */
unsigned char closing:1;
unsigned short minimum_to_wake;
- struct mutex atomic_read_lock;
- struct mutex atomic_write_lock;
- struct mutex output_lock;
- struct mutex echo_lock;
unsigned char *write_buf;
int write_cnt;
- spinlock_t read_lock;
/* If the tty has a pending do_SAK, queue it here - akpm */
struct work_struct SAK_work;
struct tty_port *port;
--
1.7.12.3

2012-10-18 20:27:57

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 20/21] TTY: add port -> tty link

For that purpose we have to temporarily introduce a second tty back
pointer into tty_port. It is because serial layer, and maybe others,
still do not use tty_port_tty_set/get. So that we cannot set the
tty_port->tty to NULL at will now.

Yes, the fix would be to convert whole serial layer and all its users
to tty_port_tty_set/get. However we are in the process of removing the
need of tty in most of the call sites, so this would lead to a
duplicated work.

Instead we have now tty_port->itty (internal tty) which will be used
only in flush_to_ldisc. For that one it is ensured that itty is valid
wherever the work is run. IOW, the work is synchronously cancelled
before we set itty to NULL and also before hangup is processed.

After we need only tty_port and not tty_struct in most code, this
shall be changed to tty_port_tty_set/get and itty removed completely.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/pty.c | 2 ++
drivers/tty/tty_io.c | 3 +++
include/linux/tty.h | 1 +
3 files changed, 6 insertions(+)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 2728afe..c326908 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -345,6 +345,7 @@ static int pty_common_install(struct tty_driver *driver, struct tty_struct *tty,
tty_port_init(ports[1]);
o_tty->port = ports[0];
tty->port = ports[1];
+ o_tty->port->itty = o_tty;

tty_driver_kref_get(driver);
tty->count++;
@@ -371,6 +372,7 @@ static void pty_unix98_shutdown(struct tty_struct *tty)

static void pty_cleanup(struct tty_struct *tty)
{
+ tty->port->itty = NULL;
kfree(tty->port);
}

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index f90b621..202008f 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1417,6 +1417,8 @@ struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx)
"%s: %s driver does not set tty->port. This will crash the kernel later. Fix the driver!\n",
__func__, tty->driver->name);

+ tty->port->itty = tty;
+
/*
* Structures all installed ... call the ldisc open routines.
* If we fail here just call release_tty to clean up. No need
@@ -1552,6 +1554,7 @@ static void release_tty(struct tty_struct *tty, int idx)
tty->ops->shutdown(tty);
tty_free_termios(tty);
tty_driver_remove_tty(tty->driver, tty);
+ tty->port->itty = NULL;

if (tty->link)
tty_kref_put(tty->link);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index b4b3c56..9be74d6 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -189,6 +189,7 @@ struct tty_port_operations {

struct tty_port {
struct tty_struct *tty; /* Back pointer */
+ struct tty_struct *itty; /* internal back ptr */
const struct tty_port_operations *ops; /* Port operations */
spinlock_t lock; /* Lock protecting tty field */
int blocked_open; /* Waiting to open */
--
1.7.12.3

2012-10-18 20:27:56

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 08/21] TTY: hci_ldisc, remove invalid check in open

hci_ldisc's open checks if tty_struct->disc_data is set. And if so it
returns with an error. But nothing ensures disc_data to be NULL. And
since ld->ops->open shall be called only once, we do not need the
check at all. So remove it.

Note that this is not an issue now, but n_tty will start using the
disc_data pointer and this invalid 'if' would trigger then rendering
TTYs over BT unusable.

Signed-off-by: Jiri Slaby <[email protected]>
Cc: Marcel Holtmann <[email protected]>
Cc: Gustavo Padovan <[email protected]>
Cc: Johan Hedberg <[email protected]>
Cc: [email protected]
---
drivers/bluetooth/hci_ldisc.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index c8abce3..ed0fade 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -270,15 +270,10 @@ static int hci_uart_send_frame(struct sk_buff *skb)
*/
static int hci_uart_tty_open(struct tty_struct *tty)
{
- struct hci_uart *hu = (void *) tty->disc_data;
+ struct hci_uart *hu;

BT_DBG("tty %p", tty);

- /* FIXME: This btw is bogus, nothing requires the old ldisc to clear
- the pointer */
- if (hu)
- return -EEXIST;
-
/* Error if the tty has no write op instead of leaving an exploitable
hole */
if (tty->ops->write == NULL)
--
1.7.12.3

2012-10-18 20:27:00

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 09/21] TTY: n_tty, simplify read_buf+echo_buf allocation

ldisc->open and close are called only once and cannot cross. So the
tests in open and close are superfluous. Remove them. (But leave sets
to NULL to ensure there is not a bug somewhere.)

And when the tests are gone, handle properly failures in open. We
leaked read_buf if allocation of echo_buf failed before. Now this is
not the case anymore.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/n_tty.c | 31 +++++++++++++------------------
1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 8c0b7b4..f27289d 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1561,14 +1561,10 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
static void n_tty_close(struct tty_struct *tty)
{
n_tty_flush_buffer(tty);
- if (tty->read_buf) {
- kfree(tty->read_buf);
- tty->read_buf = NULL;
- }
- if (tty->echo_buf) {
- kfree(tty->echo_buf);
- tty->echo_buf = NULL;
- }
+ kfree(tty->read_buf);
+ kfree(tty->echo_buf);
+ tty->read_buf = NULL;
+ tty->echo_buf = NULL;
}

/**
@@ -1587,17 +1583,11 @@ static int n_tty_open(struct tty_struct *tty)
return -EINVAL;

/* These are ugly. Currently a malloc failure here can panic */
- if (!tty->read_buf) {
- tty->read_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
- if (!tty->read_buf)
- return -ENOMEM;
- }
- if (!tty->echo_buf) {
- tty->echo_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
+ tty->read_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
+ tty->echo_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
+ if (!tty->read_buf || !tty->echo_buf)
+ goto err_free_bufs;

- if (!tty->echo_buf)
- return -ENOMEM;
- }
reset_buffer_flags(tty);
tty_unthrottle(tty);
tty->column = 0;
@@ -1605,6 +1595,11 @@ static int n_tty_open(struct tty_struct *tty)
tty->minimum_to_wake = 1;
tty->closing = 0;
return 0;
+err_free_bufs:
+ kfree(tty->read_buf);
+ kfree(tty->echo_buf);
+
+ return -ENOMEM;
}

static inline int input_available_p(struct tty_struct *tty, int amt)
--
1.7.12.3

2012-10-18 20:28:42

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 19/21] TTY: tty_buffer, cache pointer to tty->buf

During the move of tty buffers from tty_struct to tty_port, we will
need to switch all users of buf to tty->port->buf. There are many
functions where this is accessed directly in their code many times.
Cache the tty->buf pointer in such functions now and change only
single lines in each function in the next patch.

Not that it is convenient for the next patch, but the code is now also
more readable.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/tty_buffer.c | 132 +++++++++++++++++++++++++++--------------------
1 file changed, 76 insertions(+), 56 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index ab3d5c3..ffaa441 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -29,17 +29,19 @@

void tty_buffer_free_all(struct tty_struct *tty)
{
+ struct tty_bufhead *buf = &tty->buf;
struct tty_buffer *thead;
- while ((thead = tty->buf.head) != NULL) {
- tty->buf.head = thead->next;
+
+ while ((thead = buf->head) != NULL) {
+ buf->head = thead->next;
kfree(thead);
}
- while ((thead = tty->buf.free) != NULL) {
- tty->buf.free = thead->next;
+ while ((thead = buf->free) != NULL) {
+ buf->free = thead->next;
kfree(thead);
}
- tty->buf.tail = NULL;
- tty->buf.memory_used = 0;
+ buf->tail = NULL;
+ buf->memory_used = 0;
}

/**
@@ -87,15 +89,17 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_struct *tty, size_t size)

static void tty_buffer_free(struct tty_struct *tty, struct tty_buffer *b)
{
+ struct tty_bufhead *buf = &tty->buf;
+
/* Dumb strategy for now - should keep some stats */
- tty->buf.memory_used -= b->size;
- WARN_ON(tty->buf.memory_used < 0);
+ buf->memory_used -= b->size;
+ WARN_ON(buf->memory_used < 0);

if (b->size >= 512)
kfree(b);
else {
- b->next = tty->buf.free;
- tty->buf.free = b;
+ b->next = buf->free;
+ buf->free = b;
}
}

@@ -112,13 +116,14 @@ static void tty_buffer_free(struct tty_struct *tty, struct tty_buffer *b)

static void __tty_buffer_flush(struct tty_struct *tty)
{
+ struct tty_bufhead *buf = &tty->buf;
struct tty_buffer *thead;

- while ((thead = tty->buf.head) != NULL) {
- tty->buf.head = thead->next;
+ while ((thead = buf->head) != NULL) {
+ buf->head = thead->next;
tty_buffer_free(tty, thead);
}
- tty->buf.tail = NULL;
+ buf->tail = NULL;
}

/**
@@ -135,21 +140,23 @@ static void __tty_buffer_flush(struct tty_struct *tty)
void tty_buffer_flush(struct tty_struct *tty)
{
struct tty_port *port = tty->port;
+ struct tty_bufhead *buf = &tty->buf;
unsigned long flags;
- spin_lock_irqsave(&tty->buf.lock, flags);
+
+ spin_lock_irqsave(&buf->lock, flags);

/* If the data is being pushed to the tty layer then we can't
process it here. Instead set a flag and the flush_to_ldisc
path will process the flush request before it exits */
if (test_bit(TTYP_FLUSHING, &port->iflags)) {
set_bit(TTYP_FLUSHPENDING, &port->iflags);
- spin_unlock_irqrestore(&tty->buf.lock, flags);
+ spin_unlock_irqrestore(&buf->lock, flags);
wait_event(tty->read_wait,
test_bit(TTYP_FLUSHPENDING, &port->iflags) == 0);
return;
} else
__tty_buffer_flush(tty);
- spin_unlock_irqrestore(&tty->buf.lock, flags);
+ spin_unlock_irqrestore(&buf->lock, flags);
}

/**
@@ -197,12 +204,14 @@ static struct tty_buffer *tty_buffer_find(struct tty_struct *tty, size_t size)
*/
static int __tty_buffer_request_room(struct tty_struct *tty, size_t size)
{
+ struct tty_bufhead *buf = &tty->buf;
struct tty_buffer *b, *n;
int left;
/* OPTIMISATION: We could keep a per tty "zero" sized buffer to
remove this conditional if its worth it. This would be invisible
to the callers */
- if ((b = tty->buf.tail) != NULL)
+ b = buf->tail;
+ if (b != NULL)
left = b->size - b->used;
else
left = 0;
@@ -214,8 +223,8 @@ static int __tty_buffer_request_room(struct tty_struct *tty, size_t size)
b->next = n;
b->commit = b->used;
} else
- tty->buf.head = n;
- tty->buf.tail = n;
+ buf->head = n;
+ buf->tail = n;
} else
size = left;
}
@@ -262,6 +271,7 @@ EXPORT_SYMBOL_GPL(tty_buffer_request_room);
int tty_insert_flip_string_fixed_flag(struct tty_struct *tty,
const unsigned char *chars, char flag, size_t size)
{
+ struct tty_bufhead *buf = &tty->buf;
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
@@ -269,18 +279,18 @@ int tty_insert_flip_string_fixed_flag(struct tty_struct *tty,
unsigned long flags;
struct tty_buffer *tb;

- spin_lock_irqsave(&tty->buf.lock, flags);
+ spin_lock_irqsave(&buf->lock, flags);
space = __tty_buffer_request_room(tty, goal);
- tb = tty->buf.tail;
+ tb = buf->tail;
/* If there is no space then tb may be NULL */
if (unlikely(space == 0)) {
- spin_unlock_irqrestore(&tty->buf.lock, flags);
+ spin_unlock_irqrestore(&buf->lock, flags);
break;
}
memcpy(tb->char_buf_ptr + tb->used, chars, space);
memset(tb->flag_buf_ptr + tb->used, flag, space);
tb->used += space;
- spin_unlock_irqrestore(&tty->buf.lock, flags);
+ spin_unlock_irqrestore(&buf->lock, flags);
copied += space;
chars += space;
/* There is a small chance that we need to split the data over
@@ -307,6 +317,7 @@ EXPORT_SYMBOL(tty_insert_flip_string_fixed_flag);
int tty_insert_flip_string_flags(struct tty_struct *tty,
const unsigned char *chars, const char *flags, size_t size)
{
+ struct tty_bufhead *buf = &tty->buf;
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
@@ -314,18 +325,18 @@ int tty_insert_flip_string_flags(struct tty_struct *tty,
unsigned long __flags;
struct tty_buffer *tb;

- spin_lock_irqsave(&tty->buf.lock, __flags);
+ spin_lock_irqsave(&buf->lock, __flags);
space = __tty_buffer_request_room(tty, goal);
- tb = tty->buf.tail;
+ tb = buf->tail;
/* If there is no space then tb may be NULL */
if (unlikely(space == 0)) {
- spin_unlock_irqrestore(&tty->buf.lock, __flags);
+ spin_unlock_irqrestore(&buf->lock, __flags);
break;
}
memcpy(tb->char_buf_ptr + tb->used, chars, space);
memcpy(tb->flag_buf_ptr + tb->used, flags, space);
tb->used += space;
- spin_unlock_irqrestore(&tty->buf.lock, __flags);
+ spin_unlock_irqrestore(&buf->lock, __flags);
copied += space;
chars += space;
flags += space;
@@ -349,12 +360,14 @@ EXPORT_SYMBOL(tty_insert_flip_string_flags);

void tty_schedule_flip(struct tty_struct *tty)
{
+ struct tty_bufhead *buf = &tty->buf;
unsigned long flags;
- spin_lock_irqsave(&tty->buf.lock, flags);
- if (tty->buf.tail != NULL)
- tty->buf.tail->commit = tty->buf.tail->used;
- spin_unlock_irqrestore(&tty->buf.lock, flags);
- schedule_work(&tty->buf.work);
+
+ spin_lock_irqsave(&buf->lock, flags);
+ if (buf->tail != NULL)
+ buf->tail->commit = buf->tail->used;
+ spin_unlock_irqrestore(&buf->lock, flags);
+ schedule_work(&buf->work);
}
EXPORT_SYMBOL(tty_schedule_flip);

@@ -376,20 +389,21 @@ EXPORT_SYMBOL(tty_schedule_flip);
int tty_prepare_flip_string(struct tty_struct *tty, unsigned char **chars,
size_t size)
{
+ struct tty_bufhead *buf = &tty->buf;
int space;
unsigned long flags;
struct tty_buffer *tb;

- spin_lock_irqsave(&tty->buf.lock, flags);
+ spin_lock_irqsave(&buf->lock, flags);
space = __tty_buffer_request_room(tty, size);

- tb = tty->buf.tail;
+ tb = buf->tail;
if (likely(space)) {
*chars = tb->char_buf_ptr + tb->used;
memset(tb->flag_buf_ptr + tb->used, TTY_NORMAL, space);
tb->used += space;
}
- spin_unlock_irqrestore(&tty->buf.lock, flags);
+ spin_unlock_irqrestore(&buf->lock, flags);
return space;
}
EXPORT_SYMBOL_GPL(tty_prepare_flip_string);
@@ -413,20 +427,21 @@ EXPORT_SYMBOL_GPL(tty_prepare_flip_string);
int tty_prepare_flip_string_flags(struct tty_struct *tty,
unsigned char **chars, char **flags, size_t size)
{
+ struct tty_bufhead *buf = &tty->buf;
int space;
unsigned long __flags;
struct tty_buffer *tb;

- spin_lock_irqsave(&tty->buf.lock, __flags);
+ spin_lock_irqsave(&buf->lock, __flags);
space = __tty_buffer_request_room(tty, size);

- tb = tty->buf.tail;
+ tb = buf->tail;
if (likely(space)) {
*chars = tb->char_buf_ptr + tb->used;
*flags = tb->flag_buf_ptr + tb->used;
tb->used += space;
}
- spin_unlock_irqrestore(&tty->buf.lock, __flags);
+ spin_unlock_irqrestore(&buf->lock, __flags);
return space;
}
EXPORT_SYMBOL_GPL(tty_prepare_flip_string_flags);
@@ -450,6 +465,7 @@ static void flush_to_ldisc(struct work_struct *work)
struct tty_struct *tty =
container_of(work, struct tty_struct, buf.work);
struct tty_port *port = tty->port;
+ struct tty_bufhead *buf = &tty->buf;
unsigned long flags;
struct tty_ldisc *disc;

@@ -457,11 +473,11 @@ static void flush_to_ldisc(struct work_struct *work)
if (disc == NULL) /* !TTY_LDISC */
return;

- spin_lock_irqsave(&tty->buf.lock, flags);
+ spin_lock_irqsave(&buf->lock, flags);

if (!test_and_set_bit(TTYP_FLUSHING, &port->iflags)) {
struct tty_buffer *head;
- while ((head = tty->buf.head) != NULL) {
+ while ((head = buf->head) != NULL) {
int count;
char *char_buf;
unsigned char *flag_buf;
@@ -470,7 +486,7 @@ static void flush_to_ldisc(struct work_struct *work)
if (!count) {
if (head->next == NULL)
break;
- tty->buf.head = head->next;
+ buf->head = head->next;
tty_buffer_free(tty, head);
continue;
}
@@ -486,10 +502,10 @@ static void flush_to_ldisc(struct work_struct *work)
char_buf = head->char_buf_ptr + head->read;
flag_buf = head->flag_buf_ptr + head->read;
head->read += count;
- spin_unlock_irqrestore(&tty->buf.lock, flags);
+ spin_unlock_irqrestore(&buf->lock, flags);
disc->ops->receive_buf(tty, char_buf,
flag_buf, count);
- spin_lock_irqsave(&tty->buf.lock, flags);
+ spin_lock_irqsave(&buf->lock, flags);
}
clear_bit(TTYP_FLUSHING, &port->iflags);
}
@@ -501,7 +517,7 @@ static void flush_to_ldisc(struct work_struct *work)
clear_bit(TTYP_FLUSHPENDING, &port->iflags);
wake_up(&tty->read_wait);
}
- spin_unlock_irqrestore(&tty->buf.lock, flags);
+ spin_unlock_irqrestore(&buf->lock, flags);

tty_ldisc_deref(disc);
}
@@ -534,16 +550,18 @@ void tty_flush_to_ldisc(struct tty_struct *tty)

void tty_flip_buffer_push(struct tty_struct *tty)
{
+ struct tty_bufhead *buf = &tty->buf;
unsigned long flags;
- spin_lock_irqsave(&tty->buf.lock, flags);
- if (tty->buf.tail != NULL)
- tty->buf.tail->commit = tty->buf.tail->used;
- spin_unlock_irqrestore(&tty->buf.lock, flags);
+
+ spin_lock_irqsave(&buf->lock, flags);
+ if (buf->tail != NULL)
+ buf->tail->commit = buf->tail->used;
+ spin_unlock_irqrestore(&buf->lock, flags);

if (tty->low_latency)
- flush_to_ldisc(&tty->buf.work);
+ flush_to_ldisc(&buf->work);
else
- schedule_work(&tty->buf.work);
+ schedule_work(&buf->work);
}
EXPORT_SYMBOL(tty_flip_buffer_push);

@@ -559,11 +577,13 @@ EXPORT_SYMBOL(tty_flip_buffer_push);

void tty_buffer_init(struct tty_struct *tty)
{
- spin_lock_init(&tty->buf.lock);
- tty->buf.head = NULL;
- tty->buf.tail = NULL;
- tty->buf.free = NULL;
- tty->buf.memory_used = 0;
- INIT_WORK(&tty->buf.work, flush_to_ldisc);
+ struct tty_bufhead *buf = &tty->buf;
+
+ spin_lock_init(&buf->lock);
+ buf->head = NULL;
+ buf->tail = NULL;
+ buf->free = NULL;
+ buf->memory_used = 0;
+ INIT_WORK(&buf->work, flush_to_ldisc);
}

--
1.7.12.3

2012-10-18 20:29:29

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 18/21] TTY: move TTY_FLUSH* flags to tty_port

They are only TTY buffers specific. And the buffers will go to
tty_port in the next patches. So to remove the need to have both
tty_port and tty_struct at some places, let us move the flags to
tty_port.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/tty_buffer.c | 18 ++++++++++--------
include/linux/tty.h | 5 +++--
2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 91e326f..ab3d5c3 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -134,17 +134,18 @@ static void __tty_buffer_flush(struct tty_struct *tty)

void tty_buffer_flush(struct tty_struct *tty)
{
+ struct tty_port *port = tty->port;
unsigned long flags;
spin_lock_irqsave(&tty->buf.lock, flags);

/* If the data is being pushed to the tty layer then we can't
process it here. Instead set a flag and the flush_to_ldisc
path will process the flush request before it exits */
- if (test_bit(TTY_FLUSHING, &tty->flags)) {
- set_bit(TTY_FLUSHPENDING, &tty->flags);
+ if (test_bit(TTYP_FLUSHING, &port->iflags)) {
+ set_bit(TTYP_FLUSHPENDING, &port->iflags);
spin_unlock_irqrestore(&tty->buf.lock, flags);
wait_event(tty->read_wait,
- test_bit(TTY_FLUSHPENDING, &tty->flags) == 0);
+ test_bit(TTYP_FLUSHPENDING, &port->iflags) == 0);
return;
} else
__tty_buffer_flush(tty);
@@ -448,6 +449,7 @@ static void flush_to_ldisc(struct work_struct *work)
{
struct tty_struct *tty =
container_of(work, struct tty_struct, buf.work);
+ struct tty_port *port = tty->port;
unsigned long flags;
struct tty_ldisc *disc;

@@ -457,7 +459,7 @@ static void flush_to_ldisc(struct work_struct *work)

spin_lock_irqsave(&tty->buf.lock, flags);

- if (!test_and_set_bit(TTY_FLUSHING, &tty->flags)) {
+ if (!test_and_set_bit(TTYP_FLUSHING, &port->iflags)) {
struct tty_buffer *head;
while ((head = tty->buf.head) != NULL) {
int count;
@@ -475,7 +477,7 @@ static void flush_to_ldisc(struct work_struct *work)
/* Ldisc or user is trying to flush the buffers
we are feeding to the ldisc, stop feeding the
line discipline as we want to empty the queue */
- if (test_bit(TTY_FLUSHPENDING, &tty->flags))
+ if (test_bit(TTYP_FLUSHPENDING, &port->iflags))
break;
if (!tty->receive_room)
break;
@@ -489,14 +491,14 @@ static void flush_to_ldisc(struct work_struct *work)
flag_buf, count);
spin_lock_irqsave(&tty->buf.lock, flags);
}
- clear_bit(TTY_FLUSHING, &tty->flags);
+ clear_bit(TTYP_FLUSHING, &port->iflags);
}

/* We may have a deferred request to flush the input buffer,
if so pull the chain under the lock and empty the queue */
- if (test_bit(TTY_FLUSHPENDING, &tty->flags)) {
+ if (test_bit(TTYP_FLUSHPENDING, &port->iflags)) {
__tty_buffer_flush(tty);
- clear_bit(TTY_FLUSHPENDING, &tty->flags);
+ clear_bit(TTYP_FLUSHPENDING, &port->iflags);
wake_up(&tty->read_wait);
}
spin_unlock_irqrestore(&tty->buf.lock, flags);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 08787ec..b4b3c56 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -197,6 +197,9 @@ struct tty_port {
wait_queue_head_t close_wait; /* Close waiters */
wait_queue_head_t delta_msr_wait; /* Modem status change */
unsigned long flags; /* TTY flags ASY_*/
+ unsigned long iflags; /* TTYP_ internal flags */
+#define TTYP_FLUSHING 1 /* Flushing to ldisc in progress */
+#define TTYP_FLUSHPENDING 2 /* Queued buffer flush pending */
unsigned char console:1; /* port is a console */
struct mutex mutex; /* Locking */
struct mutex buf_mutex; /* Buffer alloc lock */
@@ -309,8 +312,6 @@ struct tty_file_private {
#define TTY_PTY_LOCK 16 /* pty private */
#define TTY_NO_WRITE_SPLIT 17 /* Preserve write boundaries to driver */
#define TTY_HUPPED 18 /* Post driver->hangup() */
-#define TTY_FLUSHING 19 /* Flushing to ldisc in progress */
-#define TTY_FLUSHPENDING 20 /* Queued buffer flush pending */
#define TTY_HUPPING 21 /* ->hangup() in progress */

#define TTY_WRITE_FLUSH(tty) tty_write_flush((tty))
--
1.7.12.3

2012-10-18 20:26:59

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 07/21] TTY: ldisc, wait for idle ldisc in release

We reintroduced tty_ldisc_wait_idle in 100eeae2c5c (TTY: restore
tty_ldisc_wait_idle) and used in set_ldisc. Then we added it also to
the hangup path in 92f6fa09bd453 (TTY: ldisc, do not close until there
are readers). And we noted that there is one more path:
~ Before 65b770468e98 tty_ldisc_wait_idle was called also from
~ tty_ldisc_release. It is called from tty_release, so I don't think
~ we need to restore that one.

Well, I was wrong. There might still be holders of an ldisc
reference. Not from userspace, but drivers. If they take a reference
and a user closes the device immediately after that, we have a
problem. ldisc is halted and closed by TTY, but the driver still may
call some ldisc's operation and cause a crash.

So restore the tty_ldisc_wait_idle call also to the third location
where it was before 65b770468e98 (tty-ldisc: turn ldisc user count
into a proper refcount). Now we should be safe with respect to the
ldisc reference counting as all* tty_ldisc_close paths are safely
called with reference count of one.

* Not the one in tty_ldisc_setup's fail path. But that is called
before the first open finishes. So userspace does not see it yet.
Even thought the driver is given the TTY already via ->install, it
should not take a reference to the ldisc yet. If some driver is to
do this, we should put one tty_ldisc_wait_idle also in the setup.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/tty_ldisc.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 0f2a2c5..47e3968 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -897,6 +897,11 @@ int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty)

static void tty_ldisc_kill(struct tty_struct *tty)
{
+ /* There cannot be users from userspace now. But there still might be
+ * drivers holding a reference via tty_ldisc_ref. Do not steal them the
+ * ldisc until they are done. */
+ tty_ldisc_wait_idle(tty, MAX_SCHEDULE_TIMEOUT);
+
mutex_lock(&tty->ldisc_mutex);
/*
* Now kill off the ldisc
--
1.7.12.3

2012-10-18 20:30:05

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 15/21] TTY: move ldisc data from tty_struct: read_* and echo_* and canon_* stuff

All the ring-buffers...

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/n_tty.c | 260 +++++++++++++++++++++++++++-------------------------
include/linux/tty.h | 10 --
2 files changed, 137 insertions(+), 133 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 702dd4a..4794537 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -83,6 +83,19 @@ struct n_tty_data {

DECLARE_BITMAP(process_char_map, 256);
DECLARE_BITMAP(read_flags, N_TTY_BUF_SIZE);
+
+ char *read_buf;
+ int read_head;
+ int read_tail;
+ int read_cnt;
+
+ unsigned char *echo_buf;
+ unsigned int echo_pos;
+ unsigned int echo_cnt;
+
+ int canon_data;
+ unsigned long canon_head;
+ unsigned int canon_column;
};

static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
@@ -110,14 +123,14 @@ static void n_tty_set_room(struct tty_struct *tty)
int left;
int old_left;

- /* tty->read_cnt is not read locked ? */
+ /* ldata->read_cnt is not read locked ? */
if (I_PARMRK(tty)) {
/* Multiply read_cnt by 3, since each byte might take up to
* three times as many spaces when PARMRK is set (depending on
* its flags, e.g. parity error). */
- left = N_TTY_BUF_SIZE - tty->read_cnt * 3 - 1;
+ left = N_TTY_BUF_SIZE - ldata->read_cnt * 3 - 1;
} else
- left = N_TTY_BUF_SIZE - tty->read_cnt - 1;
+ left = N_TTY_BUF_SIZE - ldata->read_cnt - 1;

/*
* If we are doing input canonicalization, and there are no
@@ -126,7 +139,7 @@ static void n_tty_set_room(struct tty_struct *tty)
* characters will be beeped.
*/
if (left <= 0)
- left = ldata->icanon && !tty->canon_data;
+ left = ldata->icanon && !ldata->canon_data;
old_left = tty->receive_room;
tty->receive_room = left;

@@ -137,10 +150,12 @@ static void n_tty_set_room(struct tty_struct *tty)

static void put_tty_queue_nolock(unsigned char c, struct tty_struct *tty)
{
- if (tty->read_cnt < N_TTY_BUF_SIZE) {
- tty->read_buf[tty->read_head] = c;
- tty->read_head = (tty->read_head + 1) & (N_TTY_BUF_SIZE-1);
- tty->read_cnt++;
+ struct n_tty_data *ldata = tty->disc_data;
+
+ if (ldata->read_cnt < N_TTY_BUF_SIZE) {
+ ldata->read_buf[ldata->read_head] = c;
+ ldata->read_head = (ldata->read_head + 1) & (N_TTY_BUF_SIZE-1);
+ ldata->read_cnt++;
}
}

@@ -198,14 +213,14 @@ static void reset_buffer_flags(struct tty_struct *tty)
unsigned long flags;

spin_lock_irqsave(&tty->read_lock, flags);
- tty->read_head = tty->read_tail = tty->read_cnt = 0;
+ ldata->read_head = ldata->read_tail = ldata->read_cnt = 0;
spin_unlock_irqrestore(&tty->read_lock, flags);

mutex_lock(&tty->echo_lock);
- tty->echo_pos = tty->echo_cnt = ldata->echo_overrun = 0;
+ ldata->echo_pos = ldata->echo_cnt = ldata->echo_overrun = 0;
mutex_unlock(&tty->echo_lock);

- tty->canon_head = tty->canon_data = ldata->erasing = 0;
+ ldata->canon_head = ldata->canon_data = ldata->erasing = 0;
bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);
n_tty_set_room(tty);
}
@@ -257,11 +272,11 @@ static ssize_t n_tty_chars_in_buffer(struct tty_struct *tty)

spin_lock_irqsave(&tty->read_lock, flags);
if (!ldata->icanon) {
- n = tty->read_cnt;
- } else if (tty->canon_data) {
- n = (tty->canon_head > tty->read_tail) ?
- tty->canon_head - tty->read_tail :
- tty->canon_head + (N_TTY_BUF_SIZE - tty->read_tail);
+ n = ldata->read_cnt;
+ } else if (ldata->canon_data) {
+ n = (ldata->canon_head > ldata->read_tail) ?
+ ldata->canon_head - ldata->read_tail :
+ ldata->canon_head + (N_TTY_BUF_SIZE - ldata->read_tail);
}
spin_unlock_irqrestore(&tty->read_lock, flags);
return n;
@@ -331,11 +346,11 @@ static int do_output_char(unsigned char c, struct tty_struct *tty, int space)
if (O_ONLCR(tty)) {
if (space < 2)
return -1;
- tty->canon_column = ldata->column = 0;
+ ldata->canon_column = ldata->column = 0;
tty->ops->write(tty, "\r\n", 2);
return 2;
}
- tty->canon_column = ldata->column;
+ ldata->canon_column = ldata->column;
break;
case '\r':
if (O_ONOCR(tty) && ldata->column == 0)
@@ -343,10 +358,10 @@ static int do_output_char(unsigned char c, struct tty_struct *tty, int space)
if (O_OCRNL(tty)) {
c = '\n';
if (O_ONLRET(tty))
- tty->canon_column = ldata->column = 0;
+ ldata->canon_column = ldata->column = 0;
break;
}
- tty->canon_column = ldata->column = 0;
+ ldata->canon_column = ldata->column = 0;
break;
case '\t':
spaces = 8 - (ldata->column & 7);
@@ -453,14 +468,14 @@ static ssize_t process_output_block(struct tty_struct *tty,
ldata->column = 0;
if (O_ONLCR(tty))
goto break_out;
- tty->canon_column = ldata->column;
+ ldata->canon_column = ldata->column;
break;
case '\r':
if (O_ONOCR(tty) && ldata->column == 0)
goto break_out;
if (O_OCRNL(tty))
goto break_out;
- tty->canon_column = ldata->column = 0;
+ ldata->canon_column = ldata->column = 0;
break;
case '\t':
goto break_out;
@@ -518,7 +533,7 @@ static void process_echoes(struct tty_struct *tty)
unsigned char c;
unsigned char *cp, *buf_end;

- if (!tty->echo_cnt)
+ if (!ldata->echo_cnt)
return;

mutex_lock(&tty->output_lock);
@@ -526,9 +541,9 @@ static void process_echoes(struct tty_struct *tty)

space = tty_write_room(tty);

- buf_end = tty->echo_buf + N_TTY_BUF_SIZE;
- cp = tty->echo_buf + tty->echo_pos;
- nr = tty->echo_cnt;
+ buf_end = ldata->echo_buf + N_TTY_BUF_SIZE;
+ cp = ldata->echo_buf + ldata->echo_pos;
+ nr = ldata->echo_cnt;
while (nr > 0) {
c = *cp;
if (c == ECHO_OP_START) {
@@ -565,7 +580,7 @@ static void process_echoes(struct tty_struct *tty)
* Otherwise, tab spacing is normal.
*/
if (!(num_chars & 0x80))
- num_chars += tty->canon_column;
+ num_chars += ldata->canon_column;
num_bs = 8 - (num_chars & 7);

if (num_bs > space) {
@@ -583,7 +598,7 @@ static void process_echoes(struct tty_struct *tty)
break;

case ECHO_OP_SET_CANON_COL:
- tty->canon_column = ldata->column;
+ ldata->canon_column = ldata->column;
cp += 2;
nr -= 2;
break;
@@ -655,14 +670,14 @@ static void process_echoes(struct tty_struct *tty)
}

if (nr == 0) {
- tty->echo_pos = 0;
- tty->echo_cnt = 0;
+ ldata->echo_pos = 0;
+ ldata->echo_cnt = 0;
ldata->echo_overrun = 0;
} else {
- int num_processed = tty->echo_cnt - nr;
- tty->echo_pos += num_processed;
- tty->echo_pos &= N_TTY_BUF_SIZE - 1;
- tty->echo_cnt = nr;
+ int num_processed = ldata->echo_cnt - nr;
+ ldata->echo_pos += num_processed;
+ ldata->echo_pos &= N_TTY_BUF_SIZE - 1;
+ ldata->echo_cnt = nr;
if (num_processed > 0)
ldata->echo_overrun = 0;
}
@@ -689,37 +704,37 @@ static void add_echo_byte(unsigned char c, struct tty_struct *tty)
struct n_tty_data *ldata = tty->disc_data;
int new_byte_pos;

- if (tty->echo_cnt == N_TTY_BUF_SIZE) {
+ if (ldata->echo_cnt == N_TTY_BUF_SIZE) {
/* Circular buffer is already at capacity */
- new_byte_pos = tty->echo_pos;
+ new_byte_pos = ldata->echo_pos;

/*
* Since the buffer start position needs to be advanced,
* be sure to step by a whole operation byte group.
*/
- if (tty->echo_buf[tty->echo_pos] == ECHO_OP_START) {
- if (tty->echo_buf[(tty->echo_pos + 1) &
+ if (ldata->echo_buf[ldata->echo_pos] == ECHO_OP_START) {
+ if (ldata->echo_buf[(ldata->echo_pos + 1) &
(N_TTY_BUF_SIZE - 1)] ==
ECHO_OP_ERASE_TAB) {
- tty->echo_pos += 3;
- tty->echo_cnt -= 2;
+ ldata->echo_pos += 3;
+ ldata->echo_cnt -= 2;
} else {
- tty->echo_pos += 2;
- tty->echo_cnt -= 1;
+ ldata->echo_pos += 2;
+ ldata->echo_cnt -= 1;
}
} else {
- tty->echo_pos++;
+ ldata->echo_pos++;
}
- tty->echo_pos &= N_TTY_BUF_SIZE - 1;
+ ldata->echo_pos &= N_TTY_BUF_SIZE - 1;

ldata->echo_overrun = 1;
} else {
- new_byte_pos = tty->echo_pos + tty->echo_cnt;
+ new_byte_pos = ldata->echo_pos + ldata->echo_cnt;
new_byte_pos &= N_TTY_BUF_SIZE - 1;
- tty->echo_cnt++;
+ ldata->echo_cnt++;
}

- tty->echo_buf[new_byte_pos] = c;
+ ldata->echo_buf[new_byte_pos] = c;
}

/**
@@ -889,7 +904,7 @@ static void eraser(unsigned char c, struct tty_struct *tty)
unsigned long flags;

/* FIXME: locking needed ? */
- if (tty->read_head == tty->canon_head) {
+ if (ldata->read_head == ldata->canon_head) {
/* process_output('\a', tty); */ /* what do you think? */
return;
}
@@ -900,17 +915,17 @@ static void eraser(unsigned char c, struct tty_struct *tty)
else {
if (!L_ECHO(tty)) {
spin_lock_irqsave(&tty->read_lock, flags);
- tty->read_cnt -= ((tty->read_head - tty->canon_head) &
+ ldata->read_cnt -= ((ldata->read_head - ldata->canon_head) &
(N_TTY_BUF_SIZE - 1));
- tty->read_head = tty->canon_head;
+ ldata->read_head = ldata->canon_head;
spin_unlock_irqrestore(&tty->read_lock, flags);
return;
}
if (!L_ECHOK(tty) || !L_ECHOKE(tty) || !L_ECHOE(tty)) {
spin_lock_irqsave(&tty->read_lock, flags);
- tty->read_cnt -= ((tty->read_head - tty->canon_head) &
+ ldata->read_cnt -= ((ldata->read_head - ldata->canon_head) &
(N_TTY_BUF_SIZE - 1));
- tty->read_head = tty->canon_head;
+ ldata->read_head = ldata->canon_head;
spin_unlock_irqrestore(&tty->read_lock, flags);
finish_erasing(tty);
echo_char(KILL_CHAR(tty), tty);
@@ -924,14 +939,14 @@ static void eraser(unsigned char c, struct tty_struct *tty)

seen_alnums = 0;
/* FIXME: Locking ?? */
- while (tty->read_head != tty->canon_head) {
- head = tty->read_head;
+ while (ldata->read_head != ldata->canon_head) {
+ head = ldata->read_head;

/* erase a single possibly multibyte character */
do {
head = (head - 1) & (N_TTY_BUF_SIZE-1);
- c = tty->read_buf[head];
- } while (is_continuation(c, tty) && head != tty->canon_head);
+ c = ldata->read_buf[head];
+ } while (is_continuation(c, tty) && head != ldata->canon_head);

/* do not partially erase */
if (is_continuation(c, tty))
@@ -944,10 +959,10 @@ static void eraser(unsigned char c, struct tty_struct *tty)
else if (seen_alnums)
break;
}
- cnt = (tty->read_head - head) & (N_TTY_BUF_SIZE-1);
+ cnt = (ldata->read_head - head) & (N_TTY_BUF_SIZE-1);
spin_lock_irqsave(&tty->read_lock, flags);
- tty->read_head = head;
- tty->read_cnt -= cnt;
+ ldata->read_head = head;
+ ldata->read_cnt -= cnt;
spin_unlock_irqrestore(&tty->read_lock, flags);
if (L_ECHO(tty)) {
if (L_ECHOPRT(tty)) {
@@ -959,7 +974,7 @@ static void eraser(unsigned char c, struct tty_struct *tty)
echo_char(c, tty);
while (--cnt > 0) {
head = (head+1) & (N_TTY_BUF_SIZE-1);
- echo_char_raw(tty->read_buf[head], tty);
+ echo_char_raw(ldata->read_buf[head], tty);
echo_move_back_col(tty);
}
} else if (kill_type == ERASE && !L_ECHOE(tty)) {
@@ -967,7 +982,7 @@ static void eraser(unsigned char c, struct tty_struct *tty)
} else if (c == '\t') {
unsigned int num_chars = 0;
int after_tab = 0;
- unsigned long tail = tty->read_head;
+ unsigned long tail = ldata->read_head;

/*
* Count the columns used for characters
@@ -976,9 +991,9 @@ static void eraser(unsigned char c, struct tty_struct *tty)
* This info is used to go back the correct
* number of columns.
*/
- while (tail != tty->canon_head) {
+ while (tail != ldata->canon_head) {
tail = (tail-1) & (N_TTY_BUF_SIZE-1);
- c = tty->read_buf[tail];
+ c = ldata->read_buf[tail];
if (c == '\t') {
after_tab = 1;
break;
@@ -1006,7 +1021,7 @@ static void eraser(unsigned char c, struct tty_struct *tty)
if (kill_type == ERASE)
break;
}
- if (tty->read_head == tty->canon_head && L_ECHO(tty))
+ if (ldata->read_head == ldata->canon_head && L_ECHO(tty))
finish_erasing(tty);
}

@@ -1171,7 +1186,7 @@ static inline void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
if (!test_bit(c, ldata->process_char_map) || ldata->lnext) {
ldata->lnext = 0;
parmrk = (c == (unsigned char) '\377' && I_PARMRK(tty)) ? 1 : 0;
- if (tty->read_cnt >= (N_TTY_BUF_SIZE - parmrk - 1)) {
+ if (ldata->read_cnt >= (N_TTY_BUF_SIZE - parmrk - 1)) {
/* beep if no space */
if (L_ECHO(tty))
process_output('\a', tty);
@@ -1180,7 +1195,7 @@ static inline void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
if (L_ECHO(tty)) {
finish_erasing(tty);
/* Record the column of first canon char. */
- if (tty->canon_head == tty->read_head)
+ if (ldata->canon_head == ldata->read_head)
echo_set_canon_col(tty);
echo_char(c, tty);
process_echoes(tty);
@@ -1264,20 +1279,20 @@ send_signal:
}
if (c == REPRINT_CHAR(tty) && L_ECHO(tty) &&
L_IEXTEN(tty)) {
- unsigned long tail = tty->canon_head;
+ unsigned long tail = ldata->canon_head;

finish_erasing(tty);
echo_char(c, tty);
echo_char_raw('\n', tty);
- while (tail != tty->read_head) {
- echo_char(tty->read_buf[tail], tty);
+ while (tail != ldata->read_head) {
+ echo_char(ldata->read_buf[tail], tty);
tail = (tail+1) & (N_TTY_BUF_SIZE-1);
}
process_echoes(tty);
return;
}
if (c == '\n') {
- if (tty->read_cnt >= N_TTY_BUF_SIZE) {
+ if (ldata->read_cnt >= N_TTY_BUF_SIZE) {
if (L_ECHO(tty))
process_output('\a', tty);
return;
@@ -1289,9 +1304,9 @@ send_signal:
goto handle_newline;
}
if (c == EOF_CHAR(tty)) {
- if (tty->read_cnt >= N_TTY_BUF_SIZE)
+ if (ldata->read_cnt >= N_TTY_BUF_SIZE)
return;
- if (tty->canon_head != tty->read_head)
+ if (ldata->canon_head != ldata->read_head)
set_bit(TTY_PUSH, &tty->flags);
c = __DISABLED_CHAR;
goto handle_newline;
@@ -1300,7 +1315,7 @@ send_signal:
(c == EOL2_CHAR(tty) && L_IEXTEN(tty))) {
parmrk = (c == (unsigned char) '\377' && I_PARMRK(tty))
? 1 : 0;
- if (tty->read_cnt >= (N_TTY_BUF_SIZE - parmrk)) {
+ if (ldata->read_cnt >= (N_TTY_BUF_SIZE - parmrk)) {
if (L_ECHO(tty))
process_output('\a', tty);
return;
@@ -1310,7 +1325,7 @@ send_signal:
*/
if (L_ECHO(tty)) {
/* Record the column of first canon char. */
- if (tty->canon_head == tty->read_head)
+ if (ldata->canon_head == ldata->read_head)
echo_set_canon_col(tty);
echo_char(c, tty);
process_echoes(tty);
@@ -1324,10 +1339,10 @@ send_signal:

handle_newline:
spin_lock_irqsave(&tty->read_lock, flags);
- set_bit(tty->read_head, ldata->read_flags);
+ set_bit(ldata->read_head, ldata->read_flags);
put_tty_queue_nolock(c, tty);
- tty->canon_head = tty->read_head;
- tty->canon_data++;
+ ldata->canon_head = ldata->read_head;
+ ldata->canon_data++;
spin_unlock_irqrestore(&tty->read_lock, flags);
kill_fasync(&tty->fasync, SIGIO, POLL_IN);
if (waitqueue_active(&tty->read_wait))
@@ -1337,7 +1352,7 @@ handle_newline:
}

parmrk = (c == (unsigned char) '\377' && I_PARMRK(tty)) ? 1 : 0;
- if (tty->read_cnt >= (N_TTY_BUF_SIZE - parmrk - 1)) {
+ if (ldata->read_cnt >= (N_TTY_BUF_SIZE - parmrk - 1)) {
/* beep if no space */
if (L_ECHO(tty))
process_output('\a', tty);
@@ -1349,7 +1364,7 @@ handle_newline:
echo_char_raw('\n', tty);
else {
/* Record the column of first canon char. */
- if (tty->canon_head == tty->read_head)
+ if (ldata->canon_head == ldata->read_head)
echo_set_canon_col(tty);
echo_char(c, tty);
}
@@ -1403,21 +1418,21 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,

if (ldata->real_raw) {
spin_lock_irqsave(&tty->read_lock, cpuflags);
- i = min(N_TTY_BUF_SIZE - tty->read_cnt,
- N_TTY_BUF_SIZE - tty->read_head);
+ i = min(N_TTY_BUF_SIZE - ldata->read_cnt,
+ N_TTY_BUF_SIZE - ldata->read_head);
i = min(count, i);
- memcpy(tty->read_buf + tty->read_head, cp, i);
- tty->read_head = (tty->read_head + i) & (N_TTY_BUF_SIZE-1);
- tty->read_cnt += i;
+ memcpy(ldata->read_buf + ldata->read_head, cp, i);
+ ldata->read_head = (ldata->read_head + i) & (N_TTY_BUF_SIZE-1);
+ ldata->read_cnt += i;
cp += i;
count -= i;

- i = min(N_TTY_BUF_SIZE - tty->read_cnt,
- N_TTY_BUF_SIZE - tty->read_head);
+ i = min(N_TTY_BUF_SIZE - ldata->read_cnt,
+ N_TTY_BUF_SIZE - ldata->read_head);
i = min(count, i);
- memcpy(tty->read_buf + tty->read_head, cp, i);
- tty->read_head = (tty->read_head + i) & (N_TTY_BUF_SIZE-1);
- tty->read_cnt += i;
+ memcpy(ldata->read_buf + ldata->read_head, cp, i);
+ ldata->read_head = (ldata->read_head + i) & (N_TTY_BUF_SIZE-1);
+ ldata->read_cnt += i;
spin_unlock_irqrestore(&tty->read_lock, cpuflags);
} else {
for (i = count, p = cp, f = fp; i; i--, p++) {
@@ -1449,7 +1464,7 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,

n_tty_set_room(tty);

- if ((!ldata->icanon && (tty->read_cnt >= tty->minimum_to_wake)) ||
+ if ((!ldata->icanon && (ldata->read_cnt >= tty->minimum_to_wake)) ||
L_EXTPROC(tty)) {
kill_fasync(&tty->fasync, SIGIO, POLL_IN);
if (waitqueue_active(&tty->read_wait))
@@ -1500,12 +1515,12 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
canon_change = (old->c_lflag ^ tty->termios.c_lflag) & ICANON;
if (canon_change) {
bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);
- tty->canon_head = tty->read_tail;
- tty->canon_data = 0;
+ ldata->canon_head = ldata->read_tail;
+ ldata->canon_data = 0;
ldata->erasing = 0;
}

- if (canon_change && !L_ICANON(tty) && tty->read_cnt)
+ if (canon_change && !L_ICANON(tty) && ldata->read_cnt)
wake_up_interruptible(&tty->read_wait);

ldata->icanon = (L_ICANON(tty) != 0);
@@ -1586,11 +1601,9 @@ static void n_tty_close(struct tty_struct *tty)
struct n_tty_data *ldata = tty->disc_data;

n_tty_flush_buffer(tty);
- kfree(tty->read_buf);
- kfree(tty->echo_buf);
+ kfree(ldata->read_buf);
+ kfree(ldata->echo_buf);
kfree(ldata);
- tty->read_buf = NULL;
- tty->echo_buf = NULL;
tty->disc_data = NULL;
}

@@ -1615,9 +1628,9 @@ static int n_tty_open(struct tty_struct *tty)
ldata->overrun_time = jiffies;

/* These are ugly. Currently a malloc failure here can panic */
- tty->read_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
- tty->echo_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
- if (!tty->read_buf || !tty->echo_buf)
+ ldata->read_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
+ ldata->echo_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
+ if (!ldata->read_buf || !ldata->echo_buf)
goto err_free_bufs;

tty->disc_data = ldata;
@@ -1630,8 +1643,8 @@ static int n_tty_open(struct tty_struct *tty)

return 0;
err_free_bufs:
- kfree(tty->read_buf);
- kfree(tty->echo_buf);
+ kfree(ldata->read_buf);
+ kfree(ldata->echo_buf);
kfree(ldata);
err:
return -ENOMEM;
@@ -1643,9 +1656,9 @@ static inline int input_available_p(struct tty_struct *tty, int amt)

tty_flush_to_ldisc(tty);
if (ldata->icanon && !L_EXTPROC(tty)) {
- if (tty->canon_data)
+ if (ldata->canon_data)
return 1;
- } else if (tty->read_cnt >= (amt ? amt : 1))
+ } else if (ldata->read_cnt >= (amt ? amt : 1))
return 1;

return 0;
@@ -1681,21 +1694,21 @@ static int copy_from_read_buf(struct tty_struct *tty,

retval = 0;
spin_lock_irqsave(&tty->read_lock, flags);
- n = min(tty->read_cnt, N_TTY_BUF_SIZE - tty->read_tail);
+ n = min(ldata->read_cnt, N_TTY_BUF_SIZE - ldata->read_tail);
n = min(*nr, n);
spin_unlock_irqrestore(&tty->read_lock, flags);
if (n) {
- retval = copy_to_user(*b, &tty->read_buf[tty->read_tail], n);
+ retval = copy_to_user(*b, &ldata->read_buf[ldata->read_tail], n);
n -= retval;
is_eof = n == 1 &&
- tty->read_buf[tty->read_tail] == EOF_CHAR(tty);
- tty_audit_add_data(tty, &tty->read_buf[tty->read_tail], n,
+ ldata->read_buf[ldata->read_tail] == EOF_CHAR(tty);
+ tty_audit_add_data(tty, &ldata->read_buf[ldata->read_tail], n,
ldata->icanon);
spin_lock_irqsave(&tty->read_lock, flags);
- tty->read_tail = (tty->read_tail + n) & (N_TTY_BUF_SIZE-1);
- tty->read_cnt -= n;
+ ldata->read_tail = (ldata->read_tail + n) & (N_TTY_BUF_SIZE-1);
+ ldata->read_cnt -= n;
/* Turn single EOF into zero-length read */
- if (L_EXTPROC(tty) && ldata->icanon && is_eof && !tty->read_cnt)
+ if (L_EXTPROC(tty) && ldata->icanon && is_eof && !ldata->read_cnt)
n = 0;
spin_unlock_irqrestore(&tty->read_lock, flags);
*b += n;
@@ -1878,22 +1891,22 @@ do_it_again:
if (ldata->icanon && !L_EXTPROC(tty)) {
/* N.B. avoid overrun if nr == 0 */
spin_lock_irqsave(&tty->read_lock, flags);
- while (nr && tty->read_cnt) {
+ while (nr && ldata->read_cnt) {
int eol;

- eol = test_and_clear_bit(tty->read_tail,
+ eol = test_and_clear_bit(ldata->read_tail,
ldata->read_flags);
- c = tty->read_buf[tty->read_tail];
- tty->read_tail = ((tty->read_tail+1) &
+ c = ldata->read_buf[ldata->read_tail];
+ ldata->read_tail = ((ldata->read_tail+1) &
(N_TTY_BUF_SIZE-1));
- tty->read_cnt--;
+ ldata->read_cnt--;
if (eol) {
/* this test should be redundant:
* we shouldn't be reading data if
* canon_data is 0
*/
- if (--tty->canon_data < 0)
- tty->canon_data = 0;
+ if (--ldata->canon_data < 0)
+ ldata->canon_data = 0;
}
spin_unlock_irqrestore(&tty->read_lock, flags);

@@ -2111,15 +2124,15 @@ static unsigned long inq_canon(struct tty_struct *tty)
struct n_tty_data *ldata = tty->disc_data;
int nr, head, tail;

- if (!tty->canon_data)
+ if (!ldata->canon_data)
return 0;
- head = tty->canon_head;
- tail = tty->read_tail;
+ head = ldata->canon_head;
+ tail = ldata->read_tail;
nr = (head - tail) & (N_TTY_BUF_SIZE-1);
/* Skip EOF-chars.. */
while (head != tail) {
if (test_bit(tail, ldata->read_flags) &&
- tty->read_buf[tail] == __DISABLED_CHAR)
+ ldata->read_buf[tail] == __DISABLED_CHAR)
nr--;
tail = (tail+1) & (N_TTY_BUF_SIZE-1);
}
@@ -2129,6 +2142,7 @@ static unsigned long inq_canon(struct tty_struct *tty)
static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
unsigned int cmd, unsigned long arg)
{
+ struct n_tty_data *ldata = tty->disc_data;
int retval;

switch (cmd) {
@@ -2136,7 +2150,7 @@ static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
return put_user(tty_chars_in_buffer(tty), (int __user *) arg);
case TIOCINQ:
/* FIXME: Locking */
- retval = tty->read_cnt;
+ retval = ldata->read_cnt;
if (L_ICANON(tty))
retval = inq_canon(tty);
return put_user(retval, (unsigned int __user *) arg);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 2161e6b..226cf20 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -272,16 +272,6 @@ struct tty_struct {
*/
unsigned char closing:1;
unsigned short minimum_to_wake;
- char *read_buf;
- int read_head;
- int read_tail;
- int read_cnt;
- unsigned char *echo_buf;
- unsigned int echo_pos;
- unsigned int echo_cnt;
- int canon_data;
- unsigned long canon_head;
- unsigned int canon_column;
struct mutex atomic_read_lock;
struct mutex atomic_write_lock;
struct mutex output_lock;
--
1.7.12.3

2012-10-18 20:30:12

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 03/21] TTY: devpts, do not set driver_data

The goal is to stop setting and using tty->driver_data in devpts code.
It should be used solely by the driver's code, pty in this case.

Now driver_data are managed only in the pty driver. devpts_pty_new is
switched to accept what we used to dig out of tty_struct, i.e. device
node number and index.

This also removes a note about driver_data being set outside of the
driver.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/pty.c | 10 +++++-----
fs/devpts/inode.c | 21 ++++++---------------
include/linux/devpts_fs.h | 9 +++++----
3 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 9985b45..559e5b2 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -4,9 +4,6 @@
* Added support for a Unix98-style ptmx device.
* -- C. Scott Ananian <[email protected]>, 14-Jan-1998
*
- * When reading this code see also fs/devpts. In particular note that the
- * driver_data field is used by the devpts side as a binding to the devpts
- * inode.
*/

#include <linux/module.h>
@@ -59,7 +56,7 @@ static void pty_close(struct tty_struct *tty, struct file *filp)
#ifdef CONFIG_UNIX98_PTYS
if (tty->driver == ptm_driver) {
mutex_lock(&devpts_mutex);
- devpts_pty_kill(tty->link);
+ devpts_pty_kill(tty->link->driver_data);
mutex_unlock(&devpts_mutex);
}
#endif
@@ -651,7 +648,9 @@ static int ptmx_open(struct inode *inode, struct file *filp)

tty_add_file(tty, filp);

- slave_inode = devpts_pty_new(inode, tty->link);
+ slave_inode = devpts_pty_new(inode,
+ MKDEV(UNIX98_PTY_SLAVE_MAJOR, index), index,
+ tty->link);
if (IS_ERR(slave_inode)) {
retval = PTR_ERR(slave_inode);
goto err_release;
@@ -662,6 +661,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
goto err_release;

tty_unlock(tty);
+ tty->link->driver_data = slave_inode;
return 0;
err_release:
tty_unlock(tty);
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index ec3bab7..7a20d67 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -545,12 +545,9 @@ void devpts_kill_index(struct inode *ptmx_inode, int idx)
mutex_unlock(&allocated_ptys_lock);
}

-struct inode *devpts_pty_new(struct inode *ptmx_inode, struct tty_struct *tty)
+struct inode *devpts_pty_new(struct inode *ptmx_inode, dev_t device, int index,
+ void *priv)
{
- /* tty layer puts index from devpts_new_index() in here */
- int number = tty->index;
- struct tty_driver *driver = tty->driver;
- dev_t device = MKDEV(driver->major, driver->minor_start+number);
struct dentry *dentry;
struct super_block *sb = pts_sb_from_inode(ptmx_inode);
struct inode *inode;
@@ -559,23 +556,18 @@ struct inode *devpts_pty_new(struct inode *ptmx_inode, struct tty_struct *tty)
struct pts_mount_opts *opts = &fsi->mount_opts;
char s[12];

- /* We're supposed to be given the slave end of a pty */
- BUG_ON(driver->type != TTY_DRIVER_TYPE_PTY);
- BUG_ON(driver->subtype != PTY_TYPE_SLAVE);
-
inode = new_inode(sb);
if (!inode)
return ERR_PTR(-ENOMEM);

- inode->i_ino = number + 3;
+ inode->i_ino = index + 3;
inode->i_uid = opts->setuid ? opts->uid : current_fsuid();
inode->i_gid = opts->setgid ? opts->gid : current_fsgid();
inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
init_special_inode(inode, S_IFCHR|opts->mode, device);
- inode->i_private = tty;
- tty->driver_data = inode;
+ inode->i_private = priv;

- sprintf(s, "%d", number);
+ sprintf(s, "%d", index);

mutex_lock(&root->d_inode->i_mutex);

@@ -613,9 +605,8 @@ void *devpts_get_priv(struct inode *pts_inode)
return priv;
}

-void devpts_pty_kill(struct tty_struct *tty)
+void devpts_pty_kill(struct inode *inode)
{
- struct inode *inode = tty->driver_data;
struct super_block *sb = pts_sb_from_inode(inode);
struct dentry *root = sb->s_root;
struct dentry *dentry;
diff --git a/include/linux/devpts_fs.h b/include/linux/devpts_fs.h
index 4ca846f..251a209 100644
--- a/include/linux/devpts_fs.h
+++ b/include/linux/devpts_fs.h
@@ -20,11 +20,12 @@
int devpts_new_index(struct inode *ptmx_inode);
void devpts_kill_index(struct inode *ptmx_inode, int idx);
/* mknod in devpts */
-struct inode *devpts_pty_new(struct inode *ptmx_inode, struct tty_struct *tty);
+struct inode *devpts_pty_new(struct inode *ptmx_inode, dev_t device, int index,
+ void *priv);
/* get private structure */
void *devpts_get_priv(struct inode *pts_inode);
/* unlink */
-void devpts_pty_kill(struct tty_struct *tty);
+void devpts_pty_kill(struct inode *inode);

#else

@@ -32,7 +33,7 @@ void devpts_pty_kill(struct tty_struct *tty);
static inline int devpts_new_index(struct inode *ptmx_inode) { return -EINVAL; }
static inline void devpts_kill_index(struct inode *ptmx_inode, int idx) { }
static inline struct inode *devpts_pty_new(struct inode *ptmx_inode,
- struct tty_struct *tty)
+ dev_t device, int index, void *priv)
{
return ERR_PTR(-EINVAL);
}
@@ -40,7 +41,7 @@ static inline void *devpts_get_priv(struct inode *pts_inode)
{
return NULL;
}
-static inline void devpts_pty_kill(struct tty_struct *tty) { }
+static inline void devpts_pty_kill(struct inode *inode) { }

#endif

--
1.7.12.3

2012-10-18 20:30:10

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 02/21] TTY: devpts, return created inode from devpts_pty_new

The goal is to stop setting and using tty->driver_data in devpts code.
It should be used solely by the driver's code, pty in this case.

For the cleanup of layering, we will need the inode created in
devpts_pty_new to be stored into slave's driver_data. So we convert
devpts_pty_new to return the inode or an ERR_PTR-encoded error in case
of failure.

The move of 'inode = new_inode(sb);' from declarators to the code is
only cosmetical, but it makes the code easier to read.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/pty.c | 7 +++++--
fs/devpts/inode.c | 12 ++++++------
include/linux/devpts_fs.h | 8 ++++----
3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 65f7671..9985b45 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -614,6 +614,7 @@ static const struct tty_operations pty_unix98_ops = {
static int ptmx_open(struct inode *inode, struct file *filp)
{
struct tty_struct *tty;
+ struct inode *slave_inode;
int retval;
int index;

@@ -650,9 +651,11 @@ static int ptmx_open(struct inode *inode, struct file *filp)

tty_add_file(tty, filp);

- retval = devpts_pty_new(inode, tty->link);
- if (retval)
+ slave_inode = devpts_pty_new(inode, tty->link);
+ if (IS_ERR(slave_inode)) {
+ retval = PTR_ERR(slave_inode);
goto err_release;
+ }

retval = ptm_driver->ops->open(tty, filp);
if (retval)
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index 4796580..ec3bab7 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -545,7 +545,7 @@ void devpts_kill_index(struct inode *ptmx_inode, int idx)
mutex_unlock(&allocated_ptys_lock);
}

-int devpts_pty_new(struct inode *ptmx_inode, struct tty_struct *tty)
+struct inode *devpts_pty_new(struct inode *ptmx_inode, struct tty_struct *tty)
{
/* tty layer puts index from devpts_new_index() in here */
int number = tty->index;
@@ -553,19 +553,19 @@ int devpts_pty_new(struct inode *ptmx_inode, struct tty_struct *tty)
dev_t device = MKDEV(driver->major, driver->minor_start+number);
struct dentry *dentry;
struct super_block *sb = pts_sb_from_inode(ptmx_inode);
- struct inode *inode = new_inode(sb);
+ struct inode *inode;
struct dentry *root = sb->s_root;
struct pts_fs_info *fsi = DEVPTS_SB(sb);
struct pts_mount_opts *opts = &fsi->mount_opts;
- int ret = 0;
char s[12];

/* We're supposed to be given the slave end of a pty */
BUG_ON(driver->type != TTY_DRIVER_TYPE_PTY);
BUG_ON(driver->subtype != PTY_TYPE_SLAVE);

+ inode = new_inode(sb);
if (!inode)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);

inode->i_ino = number + 3;
inode->i_uid = opts->setuid ? opts->uid : current_fsuid();
@@ -585,12 +585,12 @@ int devpts_pty_new(struct inode *ptmx_inode, struct tty_struct *tty)
fsnotify_create(root->d_inode, dentry);
} else {
iput(inode);
- ret = -ENOMEM;
+ inode = ERR_PTR(-ENOMEM);
}

mutex_unlock(&root->d_inode->i_mutex);

- return ret;
+ return inode;
}

void *devpts_get_priv(struct inode *pts_inode)
diff --git a/include/linux/devpts_fs.h b/include/linux/devpts_fs.h
index de635a5..4ca846f 100644
--- a/include/linux/devpts_fs.h
+++ b/include/linux/devpts_fs.h
@@ -20,7 +20,7 @@
int devpts_new_index(struct inode *ptmx_inode);
void devpts_kill_index(struct inode *ptmx_inode, int idx);
/* mknod in devpts */
-int devpts_pty_new(struct inode *ptmx_inode, struct tty_struct *tty);
+struct inode *devpts_pty_new(struct inode *ptmx_inode, struct tty_struct *tty);
/* get private structure */
void *devpts_get_priv(struct inode *pts_inode);
/* unlink */
@@ -31,10 +31,10 @@ void devpts_pty_kill(struct tty_struct *tty);
/* Dummy stubs in the no-pty case */
static inline int devpts_new_index(struct inode *ptmx_inode) { return -EINVAL; }
static inline void devpts_kill_index(struct inode *ptmx_inode, int idx) { }
-static inline int devpts_pty_new(struct inode *ptmx_inode,
- struct tty_struct *tty)
+static inline struct inode *devpts_pty_new(struct inode *ptmx_inode,
+ struct tty_struct *tty)
{
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
}
static inline void *devpts_get_priv(struct inode *pts_inode)
{
--
1.7.12.3

2012-10-18 20:30:08

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 04/21] TTY: devpts, document devpts inode operations

Add kernel-doc texts for some devpts functions, i.e. document them.

Signed-off-by: Jiri Slaby <[email protected]>
---
fs/devpts/inode.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index 7a20d67..472e6be 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -545,6 +545,15 @@ void devpts_kill_index(struct inode *ptmx_inode, int idx)
mutex_unlock(&allocated_ptys_lock);
}

+/**
+ * devpts_pty_new -- create a new inode in /dev/pts/
+ * @ptmx_inode: inode of the master
+ * @device: major+minor of the node to be created
+ * @index: used as a name of the node
+ * @priv: what's given back by devpts_get_priv
+ *
+ * The created inode is returned. Remove it from /dev/pts/ by devpts_pty_kill.
+ */
struct inode *devpts_pty_new(struct inode *ptmx_inode, dev_t device, int index,
void *priv)
{
@@ -585,6 +594,12 @@ struct inode *devpts_pty_new(struct inode *ptmx_inode, dev_t device, int index,
return inode;
}

+/**
+ * devpts_get_priv -- get private data for a slave
+ * @pts_inode: inode of the slave
+ *
+ * Returns whatever was passed as priv in devpts_pty_new for a given inode.
+ */
void *devpts_get_priv(struct inode *pts_inode)
{
struct dentry *dentry;
@@ -605,6 +620,12 @@ void *devpts_get_priv(struct inode *pts_inode)
return priv;
}

+/**
+ * devpts_pty_kill -- remove inode form /dev/pts/
+ * @inode: inode of the slave to be removed
+ *
+ * This is an inverse operation of devpts_pty_new.
+ */
void devpts_pty_kill(struct inode *inode)
{
struct super_block *sb = pts_sb_from_inode(inode);
--
1.7.12.3

2012-10-18 20:30:07

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 13/21] TTY: move ldisc data from tty_struct: simple members

Here we start moving all the n_tty related bits from tty_struct to
the newly defined n_tty_data struct in n_tty proper.

In this patch primitive members and bits are moved. The rest will be
done per-partes in the next patches.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/n_tty.c | 160 ++++++++++++++++++++++++++++++---------------------
drivers/tty/tty_io.c | 1 -
include/linux/tty.h | 5 --
3 files changed, 93 insertions(+), 73 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 3d1594e..bd775a7 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -74,13 +74,20 @@
#define ECHO_OP_ERASE_TAB 0x82

struct n_tty_data {
- char dummy;
+ unsigned int column;
+ unsigned long overrun_time;
+ int num_overrun;
+
+ unsigned char lnext:1, erasing:1, raw:1, real_raw:1, icanon:1;
+ unsigned char echo_overrun:1;
};

static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
unsigned char __user *ptr)
{
- tty_audit_add_data(tty, &x, 1, tty->icanon);
+ struct n_tty_data *ldata = tty->disc_data;
+
+ tty_audit_add_data(tty, &x, 1, ldata->icanon);
return put_user(x, ptr);
}

@@ -96,6 +103,7 @@ static inline int tty_put_user(struct tty_struct *tty, unsigned char x,

static void n_tty_set_room(struct tty_struct *tty)
{
+ struct n_tty_data *ldata = tty->disc_data;
int left;
int old_left;

@@ -115,7 +123,7 @@ static void n_tty_set_room(struct tty_struct *tty)
* characters will be beeped.
*/
if (left <= 0)
- left = tty->icanon && !tty->canon_data;
+ left = ldata->icanon && !tty->canon_data;
old_left = tty->receive_room;
tty->receive_room = left;

@@ -183,6 +191,7 @@ static void check_unthrottle(struct tty_struct *tty)

static void reset_buffer_flags(struct tty_struct *tty)
{
+ struct n_tty_data *ldata = tty->disc_data;
unsigned long flags;

spin_lock_irqsave(&tty->read_lock, flags);
@@ -190,10 +199,10 @@ static void reset_buffer_flags(struct tty_struct *tty)
spin_unlock_irqrestore(&tty->read_lock, flags);

mutex_lock(&tty->echo_lock);
- tty->echo_pos = tty->echo_cnt = tty->echo_overrun = 0;
+ tty->echo_pos = tty->echo_cnt = ldata->echo_overrun = 0;
mutex_unlock(&tty->echo_lock);

- tty->canon_head = tty->canon_data = tty->erasing = 0;
+ tty->canon_head = tty->canon_data = ldata->erasing = 0;
memset(&tty->read_flags, 0, sizeof tty->read_flags);
n_tty_set_room(tty);
}
@@ -239,11 +248,12 @@ static void n_tty_flush_buffer(struct tty_struct *tty)

static ssize_t n_tty_chars_in_buffer(struct tty_struct *tty)
{
+ struct n_tty_data *ldata = tty->disc_data;
unsigned long flags;
ssize_t n = 0;

spin_lock_irqsave(&tty->read_lock, flags);
- if (!tty->icanon) {
+ if (!ldata->icanon) {
n = tty->read_cnt;
} else if (tty->canon_data) {
n = (tty->canon_head > tty->read_tail) ?
@@ -305,6 +315,7 @@ static inline int is_continuation(unsigned char c, struct tty_struct *tty)

static int do_output_char(unsigned char c, struct tty_struct *tty, int space)
{
+ struct n_tty_data *ldata = tty->disc_data;
int spaces;

if (!space)
@@ -313,48 +324,48 @@ static int do_output_char(unsigned char c, struct tty_struct *tty, int space)
switch (c) {
case '\n':
if (O_ONLRET(tty))
- tty->column = 0;
+ ldata->column = 0;
if (O_ONLCR(tty)) {
if (space < 2)
return -1;
- tty->canon_column = tty->column = 0;
+ tty->canon_column = ldata->column = 0;
tty->ops->write(tty, "\r\n", 2);
return 2;
}
- tty->canon_column = tty->column;
+ tty->canon_column = ldata->column;
break;
case '\r':
- if (O_ONOCR(tty) && tty->column == 0)
+ if (O_ONOCR(tty) && ldata->column == 0)
return 0;
if (O_OCRNL(tty)) {
c = '\n';
if (O_ONLRET(tty))
- tty->canon_column = tty->column = 0;
+ tty->canon_column = ldata->column = 0;
break;
}
- tty->canon_column = tty->column = 0;
+ tty->canon_column = ldata->column = 0;
break;
case '\t':
- spaces = 8 - (tty->column & 7);
+ spaces = 8 - (ldata->column & 7);
if (O_TABDLY(tty) == XTABS) {
if (space < spaces)
return -1;
- tty->column += spaces;
+ ldata->column += spaces;
tty->ops->write(tty, " ", spaces);
return spaces;
}
- tty->column += spaces;
+ ldata->column += spaces;
break;
case '\b':
- if (tty->column > 0)
- tty->column--;
+ if (ldata->column > 0)
+ ldata->column--;
break;
default:
if (!iscntrl(c)) {
if (O_OLCUC(tty))
c = toupper(c);
if (!is_continuation(c, tty))
- tty->column++;
+ ldata->column++;
}
break;
}
@@ -415,6 +426,7 @@ static int process_output(unsigned char c, struct tty_struct *tty)
static ssize_t process_output_block(struct tty_struct *tty,
const unsigned char *buf, unsigned int nr)
{
+ struct n_tty_data *ldata = tty->disc_data;
int space;
int i;
const unsigned char *cp;
@@ -435,30 +447,30 @@ static ssize_t process_output_block(struct tty_struct *tty,
switch (c) {
case '\n':
if (O_ONLRET(tty))
- tty->column = 0;
+ ldata->column = 0;
if (O_ONLCR(tty))
goto break_out;
- tty->canon_column = tty->column;
+ tty->canon_column = ldata->column;
break;
case '\r':
- if (O_ONOCR(tty) && tty->column == 0)
+ if (O_ONOCR(tty) && ldata->column == 0)
goto break_out;
if (O_OCRNL(tty))
goto break_out;
- tty->canon_column = tty->column = 0;
+ tty->canon_column = ldata->column = 0;
break;
case '\t':
goto break_out;
case '\b':
- if (tty->column > 0)
- tty->column--;
+ if (ldata->column > 0)
+ ldata->column--;
break;
default:
if (!iscntrl(c)) {
if (O_OLCUC(tty))
goto break_out;
if (!is_continuation(c, tty))
- tty->column++;
+ ldata->column++;
}
break;
}
@@ -498,6 +510,7 @@ break_out:

static void process_echoes(struct tty_struct *tty)
{
+ struct n_tty_data *ldata = tty->disc_data;
int space, nr;
unsigned char c;
unsigned char *cp, *buf_end;
@@ -559,22 +572,22 @@ static void process_echoes(struct tty_struct *tty)
space -= num_bs;
while (num_bs--) {
tty_put_char(tty, '\b');
- if (tty->column > 0)
- tty->column--;
+ if (ldata->column > 0)
+ ldata->column--;
}
cp += 3;
nr -= 3;
break;

case ECHO_OP_SET_CANON_COL:
- tty->canon_column = tty->column;
+ tty->canon_column = ldata->column;
cp += 2;
nr -= 2;
break;

case ECHO_OP_MOVE_BACK_COL:
- if (tty->column > 0)
- tty->column--;
+ if (ldata->column > 0)
+ ldata->column--;
cp += 2;
nr -= 2;
break;
@@ -586,7 +599,7 @@ static void process_echoes(struct tty_struct *tty)
break;
}
tty_put_char(tty, ECHO_OP_START);
- tty->column++;
+ ldata->column++;
space--;
cp += 2;
nr -= 2;
@@ -608,7 +621,7 @@ static void process_echoes(struct tty_struct *tty)
}
tty_put_char(tty, '^');
tty_put_char(tty, op ^ 0100);
- tty->column += 2;
+ ldata->column += 2;
space -= 2;
cp += 2;
nr -= 2;
@@ -641,14 +654,14 @@ static void process_echoes(struct tty_struct *tty)
if (nr == 0) {
tty->echo_pos = 0;
tty->echo_cnt = 0;
- tty->echo_overrun = 0;
+ ldata->echo_overrun = 0;
} else {
int num_processed = tty->echo_cnt - nr;
tty->echo_pos += num_processed;
tty->echo_pos &= N_TTY_BUF_SIZE - 1;
tty->echo_cnt = nr;
if (num_processed > 0)
- tty->echo_overrun = 0;
+ ldata->echo_overrun = 0;
}

mutex_unlock(&tty->echo_lock);
@@ -670,6 +683,7 @@ static void process_echoes(struct tty_struct *tty)

static void add_echo_byte(unsigned char c, struct tty_struct *tty)
{
+ struct n_tty_data *ldata = tty->disc_data;
int new_byte_pos;

if (tty->echo_cnt == N_TTY_BUF_SIZE) {
@@ -695,7 +709,7 @@ static void add_echo_byte(unsigned char c, struct tty_struct *tty)
}
tty->echo_pos &= N_TTY_BUF_SIZE - 1;

- tty->echo_overrun = 1;
+ ldata->echo_overrun = 1;
} else {
new_byte_pos = tty->echo_pos + tty->echo_cnt;
new_byte_pos &= N_TTY_BUF_SIZE - 1;
@@ -845,9 +859,10 @@ static void echo_char(unsigned char c, struct tty_struct *tty)

static inline void finish_erasing(struct tty_struct *tty)
{
- if (tty->erasing) {
+ struct n_tty_data *ldata = tty->disc_data;
+ if (ldata->erasing) {
echo_char_raw('/', tty);
- tty->erasing = 0;
+ ldata->erasing = 0;
}
}

@@ -865,6 +880,7 @@ static inline void finish_erasing(struct tty_struct *tty)

static void eraser(unsigned char c, struct tty_struct *tty)
{
+ struct n_tty_data *ldata = tty->disc_data;
enum { ERASE, WERASE, KILL } kill_type;
int head, seen_alnums, cnt;
unsigned long flags;
@@ -932,9 +948,9 @@ static void eraser(unsigned char c, struct tty_struct *tty)
spin_unlock_irqrestore(&tty->read_lock, flags);
if (L_ECHO(tty)) {
if (L_ECHOPRT(tty)) {
- if (!tty->erasing) {
+ if (!ldata->erasing) {
echo_char_raw('\\', tty);
- tty->erasing = 1;
+ ldata->erasing = 1;
}
/* if cnt > 1, output a multi-byte character */
echo_char(c, tty);
@@ -1056,16 +1072,17 @@ static inline void n_tty_receive_break(struct tty_struct *tty)

static inline void n_tty_receive_overrun(struct tty_struct *tty)
{
+ struct n_tty_data *ldata = tty->disc_data;
char buf[64];

- tty->num_overrun++;
- if (time_before(tty->overrun_time, jiffies - HZ) ||
- time_after(tty->overrun_time, jiffies)) {
+ ldata->num_overrun++;
+ if (time_after(jiffies, ldata->overrun_time + HZ) ||
+ time_after(ldata->overrun_time, jiffies)) {
printk(KERN_WARNING "%s: %d input overrun(s)\n",
tty_name(tty, buf),
- tty->num_overrun);
- tty->overrun_time = jiffies;
- tty->num_overrun = 0;
+ ldata->num_overrun);
+ ldata->overrun_time = jiffies;
+ ldata->num_overrun = 0;
}
}

@@ -1105,10 +1122,11 @@ static inline void n_tty_receive_parity_error(struct tty_struct *tty,

static inline void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
{
+ struct n_tty_data *ldata = tty->disc_data;
unsigned long flags;
int parmrk;

- if (tty->raw) {
+ if (ldata->raw) {
put_tty_queue(c, tty);
return;
}
@@ -1147,8 +1165,8 @@ static inline void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
* handle specially, do shortcut processing to speed things
* up.
*/
- if (!test_bit(c, tty->process_char_map) || tty->lnext) {
- tty->lnext = 0;
+ if (!test_bit(c, tty->process_char_map) || ldata->lnext) {
+ ldata->lnext = 0;
parmrk = (c == (unsigned char) '\377' && I_PARMRK(tty)) ? 1 : 0;
if (tty->read_cnt >= (N_TTY_BUF_SIZE - parmrk - 1)) {
/* beep if no space */
@@ -1222,7 +1240,7 @@ send_signal:
} else if (c == '\n' && I_INLCR(tty))
c = '\r';

- if (tty->icanon) {
+ if (ldata->icanon) {
if (c == ERASE_CHAR(tty) || c == KILL_CHAR(tty) ||
(c == WERASE_CHAR(tty) && L_IEXTEN(tty))) {
eraser(c, tty);
@@ -1230,7 +1248,7 @@ send_signal:
return;
}
if (c == LNEXT_CHAR(tty) && L_IEXTEN(tty)) {
- tty->lnext = 1;
+ ldata->lnext = 1;
if (L_ECHO(tty)) {
finish_erasing(tty);
if (L_ECHOCTL(tty)) {
@@ -1373,13 +1391,14 @@ static void n_tty_write_wakeup(struct tty_struct *tty)
static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
char *fp, int count)
{
+ struct n_tty_data *ldata = tty->disc_data;
const unsigned char *p;
char *f, flags = TTY_NORMAL;
int i;
char buf[64];
unsigned long cpuflags;

- if (tty->real_raw) {
+ if (ldata->real_raw) {
spin_lock_irqsave(&tty->read_lock, cpuflags);
i = min(N_TTY_BUF_SIZE - tty->read_cnt,
N_TTY_BUF_SIZE - tty->read_head);
@@ -1427,7 +1446,7 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,

n_tty_set_room(tty);

- if ((!tty->icanon && (tty->read_cnt >= tty->minimum_to_wake)) ||
+ if ((!ldata->icanon && (tty->read_cnt >= tty->minimum_to_wake)) ||
L_EXTPROC(tty)) {
kill_fasync(&tty->fasync, SIGIO, POLL_IN);
if (waitqueue_active(&tty->read_wait))
@@ -1471,6 +1490,7 @@ int is_ignored(int sig)

static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
{
+ struct n_tty_data *ldata = tty->disc_data;
int canon_change = 1;

if (old)
@@ -1479,16 +1499,16 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
memset(&tty->read_flags, 0, sizeof tty->read_flags);
tty->canon_head = tty->read_tail;
tty->canon_data = 0;
- tty->erasing = 0;
+ ldata->erasing = 0;
}

if (canon_change && !L_ICANON(tty) && tty->read_cnt)
wake_up_interruptible(&tty->read_wait);

- tty->icanon = (L_ICANON(tty) != 0);
+ ldata->icanon = (L_ICANON(tty) != 0);
if (test_bit(TTY_HW_COOK_IN, &tty->flags)) {
- tty->raw = 1;
- tty->real_raw = 1;
+ ldata->raw = 1;
+ ldata->real_raw = 1;
n_tty_set_room(tty);
return;
}
@@ -1531,16 +1551,16 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
set_bit(SUSP_CHAR(tty), tty->process_char_map);
}
clear_bit(__DISABLED_CHAR, tty->process_char_map);
- tty->raw = 0;
- tty->real_raw = 0;
+ ldata->raw = 0;
+ ldata->real_raw = 0;
} else {
- tty->raw = 1;
+ ldata->raw = 1;
if ((I_IGNBRK(tty) || (!I_BRKINT(tty) && !I_PARMRK(tty))) &&
(I_IGNPAR(tty) || !I_INPCK(tty)) &&
(tty->driver->flags & TTY_DRIVER_REAL_RAW))
- tty->real_raw = 1;
+ ldata->real_raw = 1;
else
- tty->real_raw = 0;
+ ldata->real_raw = 0;
}
n_tty_set_room(tty);
/* The termios change make the tty ready for I/O */
@@ -1589,6 +1609,8 @@ static int n_tty_open(struct tty_struct *tty)
if (!ldata)
goto err;

+ ldata->overrun_time = jiffies;
+
/* These are ugly. Currently a malloc failure here can panic */
tty->read_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
tty->echo_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
@@ -1598,7 +1620,7 @@ static int n_tty_open(struct tty_struct *tty)
tty->disc_data = ldata;
reset_buffer_flags(tty);
tty_unthrottle(tty);
- tty->column = 0;
+ ldata->column = 0;
n_tty_set_termios(tty, NULL);
tty->minimum_to_wake = 1;
tty->closing = 0;
@@ -1614,8 +1636,10 @@ err:

static inline int input_available_p(struct tty_struct *tty, int amt)
{
+ struct n_tty_data *ldata = tty->disc_data;
+
tty_flush_to_ldisc(tty);
- if (tty->icanon && !L_EXTPROC(tty)) {
+ if (ldata->icanon && !L_EXTPROC(tty)) {
if (tty->canon_data)
return 1;
} else if (tty->read_cnt >= (amt ? amt : 1))
@@ -1646,6 +1670,7 @@ static int copy_from_read_buf(struct tty_struct *tty,
size_t *nr)

{
+ struct n_tty_data *ldata = tty->disc_data;
int retval;
size_t n;
unsigned long flags;
@@ -1662,12 +1687,12 @@ static int copy_from_read_buf(struct tty_struct *tty,
is_eof = n == 1 &&
tty->read_buf[tty->read_tail] == EOF_CHAR(tty);
tty_audit_add_data(tty, &tty->read_buf[tty->read_tail], n,
- tty->icanon);
+ ldata->icanon);
spin_lock_irqsave(&tty->read_lock, flags);
tty->read_tail = (tty->read_tail + n) & (N_TTY_BUF_SIZE-1);
tty->read_cnt -= n;
/* Turn single EOF into zero-length read */
- if (L_EXTPROC(tty) && tty->icanon && is_eof && !tty->read_cnt)
+ if (L_EXTPROC(tty) && ldata->icanon && is_eof && !tty->read_cnt)
n = 0;
spin_unlock_irqrestore(&tty->read_lock, flags);
*b += n;
@@ -1736,6 +1761,7 @@ static int job_control(struct tty_struct *tty, struct file *file)
static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
unsigned char __user *buf, size_t nr)
{
+ struct n_tty_data *ldata = tty->disc_data;
unsigned char __user *b = buf;
DECLARE_WAITQUEUE(wait, current);
int c;
@@ -1753,7 +1779,7 @@ do_it_again:

minimum = time = 0;
timeout = MAX_SCHEDULE_TIMEOUT;
- if (!tty->icanon) {
+ if (!ldata->icanon) {
time = (HZ / 10) * TIME_CHAR(tty);
minimum = MIN_CHAR(tty);
if (minimum) {
@@ -1846,7 +1872,7 @@ do_it_again:
nr--;
}

- if (tty->icanon && !L_EXTPROC(tty)) {
+ if (ldata->icanon && !L_EXTPROC(tty)) {
/* N.B. avoid overrun if nr == 0 */
spin_lock_irqsave(&tty->read_lock, flags);
while (nr && tty->read_cnt) {
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e835a5b8..67b024c 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2932,7 +2932,6 @@ void initialize_tty_struct(struct tty_struct *tty,
tty_ldisc_init(tty);
tty->session = NULL;
tty->pgrp = NULL;
- tty->overrun_time = jiffies;
tty_buffer_init(tty);
mutex_init(&tty->legacy_mutex);
mutex_init(&tty->termios_mutex);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index f02712d..de590ce 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -270,13 +270,8 @@ struct tty_struct {
* historical reasons, this is included in the tty structure.
* Mostly locked by the BKL.
*/
- unsigned int column;
- unsigned char lnext:1, erasing:1, raw:1, real_raw:1, icanon:1;
unsigned char closing:1;
- unsigned char echo_overrun:1;
unsigned short minimum_to_wake;
- unsigned long overrun_time;
- int num_overrun;
unsigned long process_char_map[256/(8*sizeof(unsigned long))];
char *read_buf;
int read_head;
--
1.7.12.3

2012-10-18 20:31:33

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 11/21] TTY: audit, stop accessing tty->icount

This is a private member of n_tty. Stop accessing it. Instead, take is
as an argument.

This is needed to allow clean switch of the private members to a
separate private structure of n_tty.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/n_tty.c | 5 +++--
drivers/tty/tty_audit.c | 15 ++++++++-------
include/linux/tty.h | 4 ++--
3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index ceae074..3ebab0c 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -76,7 +76,7 @@
static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
unsigned char __user *ptr)
{
- tty_audit_add_data(tty, &x, 1);
+ tty_audit_add_data(tty, &x, 1, tty->icanon);
return put_user(x, ptr);
}

@@ -1644,7 +1644,8 @@ static int copy_from_read_buf(struct tty_struct *tty,
n -= retval;
is_eof = n == 1 &&
tty->read_buf[tty->read_tail] == EOF_CHAR(tty);
- tty_audit_add_data(tty, &tty->read_buf[tty->read_tail], n);
+ tty_audit_add_data(tty, &tty->read_buf[tty->read_tail], n,
+ tty->icanon);
spin_lock_irqsave(&tty->read_lock, flags);
tty->read_tail = (tty->read_tail + n) & (N_TTY_BUF_SIZE-1);
tty->read_cnt -= n;
diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index b0b39b8..6953dc8 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -23,7 +23,7 @@ struct tty_audit_buf {
};

static struct tty_audit_buf *tty_audit_buf_alloc(int major, int minor,
- int icanon)
+ unsigned icanon)
{
struct tty_audit_buf *buf;

@@ -239,7 +239,8 @@ int tty_audit_push_task(struct task_struct *tsk, kuid_t loginuid, u32 sessionid)
* if TTY auditing is disabled or out of memory. Otherwise, return a new
* reference to the buffer.
*/
-static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty)
+static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty,
+ unsigned icanon)
{
struct tty_audit_buf *buf, *buf2;

@@ -257,7 +258,7 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty)

buf2 = tty_audit_buf_alloc(tty->driver->major,
tty->driver->minor_start + tty->index,
- tty->icanon);
+ icanon);
if (buf2 == NULL) {
audit_log_lost("out of memory in TTY auditing");
return NULL;
@@ -287,7 +288,7 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty)
* Audit @data of @size from @tty, if necessary.
*/
void tty_audit_add_data(struct tty_struct *tty, unsigned char *data,
- size_t size)
+ size_t size, unsigned icanon)
{
struct tty_audit_buf *buf;
int major, minor;
@@ -299,7 +300,7 @@ void tty_audit_add_data(struct tty_struct *tty, unsigned char *data,
&& tty->driver->subtype == PTY_TYPE_MASTER)
return;

- buf = tty_audit_buf_get(tty);
+ buf = tty_audit_buf_get(tty, icanon);
if (!buf)
return;

@@ -307,11 +308,11 @@ void tty_audit_add_data(struct tty_struct *tty, unsigned char *data,
major = tty->driver->major;
minor = tty->driver->minor_start + tty->index;
if (buf->major != major || buf->minor != minor
- || buf->icanon != tty->icanon) {
+ || buf->icanon != icanon) {
tty_audit_buf_push_current(buf);
buf->major = major;
buf->minor = minor;
- buf->icanon = tty->icanon;
+ buf->icanon = icanon;
}
do {
size_t run;
diff --git a/include/linux/tty.h b/include/linux/tty.h
index f0b4eb4..f02712d 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -535,7 +535,7 @@ extern void n_tty_inherit_ops(struct tty_ldisc_ops *ops);
/* tty_audit.c */
#ifdef CONFIG_AUDIT
extern void tty_audit_add_data(struct tty_struct *tty, unsigned char *data,
- size_t size);
+ size_t size, unsigned icanon);
extern void tty_audit_exit(void);
extern void tty_audit_fork(struct signal_struct *sig);
extern void tty_audit_tiocsti(struct tty_struct *tty, char ch);
@@ -544,7 +544,7 @@ extern int tty_audit_push_task(struct task_struct *tsk,
kuid_t loginuid, u32 sessionid);
#else
static inline void tty_audit_add_data(struct tty_struct *tty,
- unsigned char *data, size_t size)
+ unsigned char *data, size_t size, unsigned icanon)
{
}
static inline void tty_audit_tiocsti(struct tty_struct *tty, char ch)
--
1.7.12.3

2012-10-18 20:31:32

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 14/21] TTY: move ldisc data from tty_struct: bitmaps

Here we move bitmaps and use DECLARE_BITMAP to declare them in the new
structure. And instead of memset, we use bitmap_zero as it is more
appropriate.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/n_tty.c | 52 ++++++++++++++++++++++++++++------------------------
include/linux/tty.h | 2 --
2 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index bd775a7..702dd4a 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -80,6 +80,9 @@ struct n_tty_data {

unsigned char lnext:1, erasing:1, raw:1, real_raw:1, icanon:1;
unsigned char echo_overrun:1;
+
+ DECLARE_BITMAP(process_char_map, 256);
+ DECLARE_BITMAP(read_flags, N_TTY_BUF_SIZE);
};

static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
@@ -203,7 +206,7 @@ static void reset_buffer_flags(struct tty_struct *tty)
mutex_unlock(&tty->echo_lock);

tty->canon_head = tty->canon_data = ldata->erasing = 0;
- memset(&tty->read_flags, 0, sizeof tty->read_flags);
+ bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);
n_tty_set_room(tty);
}

@@ -1165,7 +1168,7 @@ static inline void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
* handle specially, do shortcut processing to speed things
* up.
*/
- if (!test_bit(c, tty->process_char_map) || ldata->lnext) {
+ if (!test_bit(c, ldata->process_char_map) || ldata->lnext) {
ldata->lnext = 0;
parmrk = (c == (unsigned char) '\377' && I_PARMRK(tty)) ? 1 : 0;
if (tty->read_cnt >= (N_TTY_BUF_SIZE - parmrk - 1)) {
@@ -1321,7 +1324,7 @@ send_signal:

handle_newline:
spin_lock_irqsave(&tty->read_lock, flags);
- set_bit(tty->read_head, tty->read_flags);
+ set_bit(tty->read_head, ldata->read_flags);
put_tty_queue_nolock(c, tty);
tty->canon_head = tty->read_head;
tty->canon_data++;
@@ -1496,7 +1499,7 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
if (old)
canon_change = (old->c_lflag ^ tty->termios.c_lflag) & ICANON;
if (canon_change) {
- memset(&tty->read_flags, 0, sizeof tty->read_flags);
+ bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);
tty->canon_head = tty->read_tail;
tty->canon_data = 0;
ldata->erasing = 0;
@@ -1516,41 +1519,41 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
I_ICRNL(tty) || I_INLCR(tty) || L_ICANON(tty) ||
I_IXON(tty) || L_ISIG(tty) || L_ECHO(tty) ||
I_PARMRK(tty)) {
- memset(tty->process_char_map, 0, 256/8);
+ bitmap_zero(ldata->process_char_map, 256);

if (I_IGNCR(tty) || I_ICRNL(tty))
- set_bit('\r', tty->process_char_map);
+ set_bit('\r', ldata->process_char_map);
if (I_INLCR(tty))
- set_bit('\n', tty->process_char_map);
+ set_bit('\n', ldata->process_char_map);

if (L_ICANON(tty)) {
- set_bit(ERASE_CHAR(tty), tty->process_char_map);
- set_bit(KILL_CHAR(tty), tty->process_char_map);
- set_bit(EOF_CHAR(tty), tty->process_char_map);
- set_bit('\n', tty->process_char_map);
- set_bit(EOL_CHAR(tty), tty->process_char_map);
+ set_bit(ERASE_CHAR(tty), ldata->process_char_map);
+ set_bit(KILL_CHAR(tty), ldata->process_char_map);
+ set_bit(EOF_CHAR(tty), ldata->process_char_map);
+ set_bit('\n', ldata->process_char_map);
+ set_bit(EOL_CHAR(tty), ldata->process_char_map);
if (L_IEXTEN(tty)) {
set_bit(WERASE_CHAR(tty),
- tty->process_char_map);
+ ldata->process_char_map);
set_bit(LNEXT_CHAR(tty),
- tty->process_char_map);
+ ldata->process_char_map);
set_bit(EOL2_CHAR(tty),
- tty->process_char_map);
+ ldata->process_char_map);
if (L_ECHO(tty))
set_bit(REPRINT_CHAR(tty),
- tty->process_char_map);
+ ldata->process_char_map);
}
}
if (I_IXON(tty)) {
- set_bit(START_CHAR(tty), tty->process_char_map);
- set_bit(STOP_CHAR(tty), tty->process_char_map);
+ set_bit(START_CHAR(tty), ldata->process_char_map);
+ set_bit(STOP_CHAR(tty), ldata->process_char_map);
}
if (L_ISIG(tty)) {
- set_bit(INTR_CHAR(tty), tty->process_char_map);
- set_bit(QUIT_CHAR(tty), tty->process_char_map);
- set_bit(SUSP_CHAR(tty), tty->process_char_map);
+ set_bit(INTR_CHAR(tty), ldata->process_char_map);
+ set_bit(QUIT_CHAR(tty), ldata->process_char_map);
+ set_bit(SUSP_CHAR(tty), ldata->process_char_map);
}
- clear_bit(__DISABLED_CHAR, tty->process_char_map);
+ clear_bit(__DISABLED_CHAR, ldata->process_char_map);
ldata->raw = 0;
ldata->real_raw = 0;
} else {
@@ -1879,7 +1882,7 @@ do_it_again:
int eol;

eol = test_and_clear_bit(tty->read_tail,
- tty->read_flags);
+ ldata->read_flags);
c = tty->read_buf[tty->read_tail];
tty->read_tail = ((tty->read_tail+1) &
(N_TTY_BUF_SIZE-1));
@@ -2105,6 +2108,7 @@ static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,

static unsigned long inq_canon(struct tty_struct *tty)
{
+ struct n_tty_data *ldata = tty->disc_data;
int nr, head, tail;

if (!tty->canon_data)
@@ -2114,7 +2118,7 @@ static unsigned long inq_canon(struct tty_struct *tty)
nr = (head - tail) & (N_TTY_BUF_SIZE-1);
/* Skip EOF-chars.. */
while (head != tail) {
- if (test_bit(tail, tty->read_flags) &&
+ if (test_bit(tail, ldata->read_flags) &&
tty->read_buf[tail] == __DISABLED_CHAR)
nr--;
tail = (tail+1) & (N_TTY_BUF_SIZE-1);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index de590ce..2161e6b 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -272,12 +272,10 @@ struct tty_struct {
*/
unsigned char closing:1;
unsigned short minimum_to_wake;
- unsigned long process_char_map[256/(8*sizeof(unsigned long))];
char *read_buf;
int read_head;
int read_tail;
int read_cnt;
- unsigned long read_flags[N_TTY_BUF_SIZE/(8*sizeof(unsigned long))];
unsigned char *echo_buf;
unsigned int echo_pos;
unsigned int echo_cnt;
--
1.7.12.3

2012-10-18 20:31:57

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 06/21] TTY: vt, fix paste_selection ldisc handling

There used to be a single tty_ldisc_ref_wait. But then, when a
big-tty-mutex (BTM) was introduced, it has to be tty_ldisc_ref +
tty_unlock + tty_ldisc_ref_wait + tty_lock. Later, BTM was removed
from that path and tty_ldisc_ref + tty_ldisc_ref_wait remained there.
But it makes no sense now. So leave there only tty_ldisc_ref_wait.

And when we have a reference to an ldisc, actually use it in the loop.
Otherwise it may be racy.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/vt/selection.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 8e9b4be..60b7b69 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -341,15 +341,11 @@ int paste_selection(struct tty_struct *tty)
struct tty_ldisc *ld;
DECLARE_WAITQUEUE(wait, current);

-
console_lock();
poke_blanked_console();
console_unlock();

- /* FIXME: wtf is this supposed to achieve ? */
- ld = tty_ldisc_ref(tty);
- if (!ld)
- ld = tty_ldisc_ref_wait(tty);
+ ld = tty_ldisc_ref_wait(tty);

/* FIXME: this is completely unsafe */
add_wait_queue(&vc->paste_wait, &wait);
@@ -361,8 +357,7 @@ int paste_selection(struct tty_struct *tty)
}
count = sel_buffer_lth - pasted;
count = min(count, tty->receive_room);
- tty->ldisc->ops->receive_buf(tty, sel_buffer + pasted,
- NULL, count);
+ ld->ops->receive_buf(tty, sel_buffer + pasted, NULL, count);
pasted += count;
}
remove_wait_queue(&vc->paste_wait, &wait);
--
1.7.12.3

2012-10-18 20:32:00

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 01/21] TTY: devpts, don't care about TTY in devpts_get_tty

The goal is to stop setting and using tty->driver_data in devpts code.
It should be used solely by the driver's code, pty in this case.

First, here we remove TTY from devpts_get_tty and rename it to
devpts_get_priv. Note we do not remove type safety, we just shift the
[implicit] (void *) cast one layer up.

index was unused in devpts_get_tty, so remove that from the prototype
too.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/pty.c | 2 +-
fs/devpts/inode.c | 9 ++++-----
include/linux/devpts_fs.h | 7 +++----
3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index a82b399..65f7671 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -547,7 +547,7 @@ static struct tty_struct *pts_unix98_lookup(struct tty_driver *driver,
struct tty_struct *tty;

mutex_lock(&devpts_mutex);
- tty = devpts_get_tty(pts_inode, idx);
+ tty = devpts_get_priv(pts_inode);
mutex_unlock(&devpts_mutex);
/* Master must be open before slave */
if (!tty)
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index 14afbab..4796580 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -593,10 +593,10 @@ int devpts_pty_new(struct inode *ptmx_inode, struct tty_struct *tty)
return ret;
}

-struct tty_struct *devpts_get_tty(struct inode *pts_inode, int number)
+void *devpts_get_priv(struct inode *pts_inode)
{
struct dentry *dentry;
- struct tty_struct *tty;
+ void *priv = NULL;

BUG_ON(pts_inode->i_rdev == MKDEV(TTYAUX_MAJOR, PTMX_MINOR));

@@ -605,13 +605,12 @@ struct tty_struct *devpts_get_tty(struct inode *pts_inode, int number)
if (!dentry)
return NULL;

- tty = NULL;
if (pts_inode->i_sb->s_magic == DEVPTS_SUPER_MAGIC)
- tty = (struct tty_struct *)pts_inode->i_private;
+ priv = pts_inode->i_private;

dput(dentry);

- return tty;
+ return priv;
}

void devpts_pty_kill(struct tty_struct *tty)
diff --git a/include/linux/devpts_fs.h b/include/linux/devpts_fs.h
index 5ce0e5f..de635a5 100644
--- a/include/linux/devpts_fs.h
+++ b/include/linux/devpts_fs.h
@@ -21,8 +21,8 @@ int devpts_new_index(struct inode *ptmx_inode);
void devpts_kill_index(struct inode *ptmx_inode, int idx);
/* mknod in devpts */
int devpts_pty_new(struct inode *ptmx_inode, struct tty_struct *tty);
-/* get tty structure */
-struct tty_struct *devpts_get_tty(struct inode *pts_inode, int number);
+/* get private structure */
+void *devpts_get_priv(struct inode *pts_inode);
/* unlink */
void devpts_pty_kill(struct tty_struct *tty);

@@ -36,8 +36,7 @@ static inline int devpts_pty_new(struct inode *ptmx_inode,
{
return -EINVAL;
}
-static inline struct tty_struct *devpts_get_tty(struct inode *pts_inode,
- int number)
+static inline void *devpts_get_priv(struct inode *pts_inode)
{
return NULL;
}
--
1.7.12.3

2012-10-18 20:31:58

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 10/21] TTY: n_tty, remove bogus checks

* BUG_ON(!tty) in n_tty_set_termios -- it cannot be called with tty ==
NULL. It is called from two call sites. First, from n_tty_open where
we have a valid tty. Second, as ld->ops->set_termios from
tty_set_termios. But there we have a valid tty too.
* if (!tty) in n_tty_open -- why would the TTY layer call ldisc's
open with an invalid TTY? No it indeed does not. All call sites have
a tty and dereference that.
* BUG_ON(!tty->read_buf) in n_tty_read -- this used to be a valid
check. The ldisc handling was broken some time ago when I added the
check to ensure everything is OK. It still can catch the case, but
no later than we move the buffer to ldisc data. Then there will be
no read_buf in tty_struct, i.e. nothing to check for.
* if (!tty->read_buf) in n_tty_receive_buf -- this should never
happen. All callers of ldisc->ops->receive_ops should hold a
reference to an ldisc and close (which frees read_buf) cannot be
called until the reference is dropped.
* if (WARN_ON(!tty->read_buf)) in n_tty_read -- the same as in the
previous case.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/n_tty.c | 12 ------------
1 file changed, 12 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index f27289d..ceae074 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1375,9 +1375,6 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
char buf[64];
unsigned long cpuflags;

- if (!tty->read_buf)
- return;
-
if (tty->real_raw) {
spin_lock_irqsave(&tty->read_lock, cpuflags);
i = min(N_TTY_BUF_SIZE - tty->read_cnt,
@@ -1471,7 +1468,6 @@ int is_ignored(int sig)
static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
{
int canon_change = 1;
- BUG_ON(!tty);

if (old)
canon_change = (old->c_lflag ^ tty->termios.c_lflag) & ICANON;
@@ -1579,9 +1575,6 @@ static void n_tty_close(struct tty_struct *tty)

static int n_tty_open(struct tty_struct *tty)
{
- if (!tty)
- return -EINVAL;
-
/* These are ugly. Currently a malloc failure here can panic */
tty->read_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
tty->echo_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
@@ -1736,10 +1729,6 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
int packet;

do_it_again:
-
- if (WARN_ON(!tty->read_buf))
- return -EAGAIN;
-
c = job_control(tty, file);
if (c < 0)
return c;
@@ -1825,7 +1814,6 @@ do_it_again:
/* FIXME: does n_tty_set_room need locking ? */
n_tty_set_room(tty);
timeout = schedule_timeout(timeout);
- BUG_ON(!tty->read_buf);
continue;
}
__set_current_state(TASK_RUNNING);
--
1.7.12.3

2012-10-18 20:51:58

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 08/21] TTY: hci_ldisc, remove invalid check in open

Hi Jiri,

> hci_ldisc's open checks if tty_struct->disc_data is set. And if so it
> returns with an error. But nothing ensures disc_data to be NULL. And
> since ld->ops->open shall be called only once, we do not need the
> check at all. So remove it.
>
> Note that this is not an issue now, but n_tty will start using the
> disc_data pointer and this invalid 'if' would trigger then rendering
> TTYs over BT unusable.
>
> Signed-off-by: Jiri Slaby <[email protected]>
> Cc: Marcel Holtmann <[email protected]>
> Cc: Gustavo Padovan <[email protected]>
> Cc: Johan Hedberg <[email protected]>
> Cc: [email protected]
> ---
> drivers/bluetooth/hci_ldisc.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel

2012-10-18 21:12:36

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 00/21] TTY buffer in tty_port and other stuff

On Thu, Oct 18, 2012 at 10:26:26PM +0200, Jiri Slaby wrote:
> Hi,
>
> this is the fifth series of patches which finally move tty buffers
> from tty_struct (present from open to close/hangup) to tty_port
> (present as long as the device). This allows us to get rid of the tty
> refcounting in the interrupt service routines and other hot paths
> after we are done. This is because we do not need to handle races
> among ISRs, timers, hangups and others, because tty_port lives as long
> as an interrupt/timer tick may occur. Unlike tty_struct.
>
> This set also cleans up devpts handling a bit. Devpts used to play
> with tty->driver_data which was a bit ugly. Now devpts returns a node
> which we store to driver_data and pass it back when we need devpts to
> kill that. As a result, we can do that in the pty code instead of an
> ugly hook in tty_release.
>
> Finally, the set moves all the n_tty private stuff from tty_struct to
> its own (internal) structure. This was an intention last time ago (at
> least here), but the races and undefined ldisc->open/close behavior
> did not allow us to do that. Now that we have ldisc kills and waits
> and bells and whistles we could finally go ahead.
>
> As usual, standard x86 stuff was runtime-tested. The rest is only
> checked to be compilation-errors free.

Nice work, I'll queue it up for 3.8 and let's see what breaks in
linux-next :)

greg k-h

2012-10-22 14:52:06

by Alan

[permalink] [raw]
Subject: Re: [PATCH 00/21] TTY buffer in tty_port and other stuff

On Thu, 18 Oct 2012 22:26:26 +0200
Jiri Slaby <[email protected]> wrote:

> Hi,
>
> this is the fifth series of patches which finally move tty buffers
> from tty_struct (present from open to close/hangup) to tty_port
> (present as long as the device). This allows us to get rid of the tty
> refcounting in the interrupt service routines and other hot paths
> after we are done. This is because we do not need to handle races
> among ISRs, timers, hangups and others, because tty_port lives as long
> as an interrupt/timer tick may occur. Unlike tty_struct.

Looks good to me working through them

Acked-by: Alan Cox <[email protected]>

2012-10-23 00:00:05

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 00/21] TTY buffer in tty_port and other stuff

On Thu, Oct 18, 2012 at 10:26:26PM +0200, Jiri Slaby wrote:
> Hi,
>
> this is the fifth series of patches which finally move tty buffers
> from tty_struct (present from open to close/hangup) to tty_port
> (present as long as the device). This allows us to get rid of the tty
> refcounting in the interrupt service routines and other hot paths
> after we are done. This is because we do not need to handle races
> among ISRs, timers, hangups and others, because tty_port lives as long
> as an interrupt/timer tick may occur. Unlike tty_struct.
>
> This set also cleans up devpts handling a bit. Devpts used to play
> with tty->driver_data which was a bit ugly. Now devpts returns a node
> which we store to driver_data and pass it back when we need devpts to
> kill that. As a result, we can do that in the pty code instead of an
> ugly hook in tty_release.
>
> Finally, the set moves all the n_tty private stuff from tty_struct to
> its own (internal) structure. This was an intention last time ago (at
> least here), but the races and undefined ldisc->open/close behavior
> did not allow us to do that. Now that we have ldisc kills and waits
> and bells and whistles we could finally go ahead.
>
> As usual, standard x86 stuff was runtime-tested. The rest is only
> checked to be compilation-errors free.

I've applied all of these, very nice.

But you broke one of the staging drivers (drivers/staging/dgrp/) Care
to fix that up now?

thanks,

greg k-h

2012-10-25 18:02:17

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

Hi guys,

On 10/18/2012 04:26 PM, Jiri Slaby wrote:
> So this is it. The big step why we did all the work over the past
> kernel releases. Now everything is prepared, so nothing protects us
> from doing that big step.
>
> | | \ \ nnnn/^l | |
> | | \ / / | |
> | '-,.__ => \/ ,-` => | '-,.__
> | O __.´´) ( .` | O __.´´)
> ~~~ ~~ `` ~~~ ~~
> The buffers are now in the tty_port structure and we can start
> teaching the buffer helpers (insert char/string, flip etc.) to use
> tty_port instead of tty_struct all around.
>
> Signed-off-by: Jiri Slaby <[email protected]>
> ---

Fuzzing with trinity inside a KVM tools (lkvm) guest with -next kernel
uncovered the following warning:

[ 1339.448706] ------------[ cut here ]------------
[ 1339.451224] WARNING: at drivers/tty/tty_buffer.c:476 flush_to_ldisc+0x60/0x200()
[ 1339.454272] tty is NULLPid: 7147, comm: kworker/4:0 Tainted: G W 3.7.0-rc2-next-20121025-sasha-00001-g673f98e-dirty #75
[ 1339.458693] Call Trace:
[ 1339.459410] [<ffffffff81bb1ea0>] ? flush_to_ldisc+0x60/0x200
[ 1339.461289] [<ffffffff81109b86>] warn_slowpath_common+0x86/0xb0
[ 1339.462992] [<ffffffff81109c11>] warn_slowpath_fmt+0x41/0x50
[ 1339.464772] [<ffffffff81bb1ea0>] flush_to_ldisc+0x60/0x200
[ 1339.467076] [<ffffffff8112d5a9>] process_one_work+0x3b9/0x770
[ 1339.469501] [<ffffffff8112d458>] ? process_one_work+0x268/0x770
[ 1339.472053] [<ffffffff8112dcc1>] ? worker_thread+0x51/0x3f0
[ 1339.473831] [<ffffffff81bb1e40>] ? __tty_buffer_request_room+0x180/0x180
[ 1339.475834] [<ffffffff8112df2a>] worker_thread+0x2ba/0x3f0
[ 1339.478027] [<ffffffff8112dc70>] ? rescuer_thread+0x2d0/0x2d0
[ 1339.480431] [<ffffffff81138c33>] kthread+0xe3/0xf0
[ 1339.482383] [<ffffffff8117d7be>] ? put_lock_stats.isra.16+0xe/0x40
[ 1339.484171] [<ffffffff81138b50>] ? insert_kthread_work+0x90/0x90
[ 1339.485886] [<ffffffff83aedebc>] ret_from_fork+0x7c/0xb0
[ 1339.487943] [<ffffffff81138b50>] ? insert_kthread_work+0x90/0x90
[ 1339.490435] ---[ end trace e01a8b0af77894c4 ]---

I'm guessing it happens because we never cancel the scheduled work when we
free the buffer, so the scheduled work may run even after we freed the buffer.

Besides the warning itself, I think that 'tty is NULL' would need a newline
after it. Greg, should I send a patch for that?


Thanks,
Sasha

2012-10-25 18:08:07

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On Thu, Oct 25, 2012 at 02:02:00PM -0400, Sasha Levin wrote:
> Hi guys,
>
> On 10/18/2012 04:26 PM, Jiri Slaby wrote:
> > So this is it. The big step why we did all the work over the past
> > kernel releases. Now everything is prepared, so nothing protects us
> > from doing that big step.
> >
> > | | \ \ nnnn/^l | |
> > | | \ / / | |
> > | '-,.__ => \/ ,-` => | '-,.__
> > | O __.??) ( .` | O __.??)
> > ~~~ ~~ `` ~~~ ~~
> > The buffers are now in the tty_port structure and we can start
> > teaching the buffer helpers (insert char/string, flip etc.) to use
> > tty_port instead of tty_struct all around.
> >
> > Signed-off-by: Jiri Slaby <[email protected]>
> > ---
>
> Fuzzing with trinity inside a KVM tools (lkvm) guest with -next kernel
> uncovered the following warning:
>
> [ 1339.448706] ------------[ cut here ]------------
> [ 1339.451224] WARNING: at drivers/tty/tty_buffer.c:476 flush_to_ldisc+0x60/0x200()
> [ 1339.454272] tty is NULLPid: 7147, comm: kworker/4:0 Tainted: G W 3.7.0-rc2-next-20121025-sasha-00001-g673f98e-dirty #75
> [ 1339.458693] Call Trace:
> [ 1339.459410] [<ffffffff81bb1ea0>] ? flush_to_ldisc+0x60/0x200
> [ 1339.461289] [<ffffffff81109b86>] warn_slowpath_common+0x86/0xb0
> [ 1339.462992] [<ffffffff81109c11>] warn_slowpath_fmt+0x41/0x50
> [ 1339.464772] [<ffffffff81bb1ea0>] flush_to_ldisc+0x60/0x200
> [ 1339.467076] [<ffffffff8112d5a9>] process_one_work+0x3b9/0x770
> [ 1339.469501] [<ffffffff8112d458>] ? process_one_work+0x268/0x770
> [ 1339.472053] [<ffffffff8112dcc1>] ? worker_thread+0x51/0x3f0
> [ 1339.473831] [<ffffffff81bb1e40>] ? __tty_buffer_request_room+0x180/0x180
> [ 1339.475834] [<ffffffff8112df2a>] worker_thread+0x2ba/0x3f0
> [ 1339.478027] [<ffffffff8112dc70>] ? rescuer_thread+0x2d0/0x2d0
> [ 1339.480431] [<ffffffff81138c33>] kthread+0xe3/0xf0
> [ 1339.482383] [<ffffffff8117d7be>] ? put_lock_stats.isra.16+0xe/0x40
> [ 1339.484171] [<ffffffff81138b50>] ? insert_kthread_work+0x90/0x90
> [ 1339.485886] [<ffffffff83aedebc>] ret_from_fork+0x7c/0xb0
> [ 1339.487943] [<ffffffff81138b50>] ? insert_kthread_work+0x90/0x90
> [ 1339.490435] ---[ end trace e01a8b0af77894c4 ]---
>
> I'm guessing it happens because we never cancel the scheduled work when we
> free the buffer, so the scheduled work may run even after we freed the buffer.
>
> Besides the warning itself, I think that 'tty is NULL' would need a newline
> after it. Greg, should I send a patch for that?

Yes, please do.

thanks,

greg k-h

2012-10-31 12:54:07

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On 10/25/2012 08:02 PM, Sasha Levin wrote:
> Fuzzing with trinity inside a KVM tools (lkvm) guest with -next kernel
> uncovered the following warning:

I cannot reproduce that :(. Do you still see it?

> [ 1339.448706] ------------[ cut here ]------------
> [ 1339.451224] WARNING: at drivers/tty/tty_buffer.c:476 flush_to_ldisc+0x60/0x200()
> [ 1339.454272] tty is NULLPid: 7147, comm: kworker/4:0 Tainted: G W 3.7.0-rc2-next-20121025-sasha-00001-g673f98e-dirty #75
> [ 1339.458693] Call Trace:
> [ 1339.459410] [<ffffffff81bb1ea0>] ? flush_to_ldisc+0x60/0x200
> [ 1339.461289] [<ffffffff81109b86>] warn_slowpath_common+0x86/0xb0
> [ 1339.462992] [<ffffffff81109c11>] warn_slowpath_fmt+0x41/0x50
> [ 1339.464772] [<ffffffff81bb1ea0>] flush_to_ldisc+0x60/0x200

thanks,
--
js
suse labs

2012-10-31 15:30:28

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On Wed, Oct 31, 2012 at 8:53 AM, Jiri Slaby <[email protected]> wrote:
> On 10/25/2012 08:02 PM, Sasha Levin wrote:
>> Fuzzing with trinity inside a KVM tools (lkvm) guest with -next kernel
>> uncovered the following warning:
>
> I cannot reproduce that :(. Do you still see it?

Yes, it reproduces pretty easily while fuzzing.


Thanks,
Sasha

2012-10-31 15:32:35

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On 10/31/2012 04:30 PM, Sasha Levin wrote:
> On Wed, Oct 31, 2012 at 8:53 AM, Jiri Slaby <[email protected]> wrote:
>> On 10/25/2012 08:02 PM, Sasha Levin wrote:
>>> Fuzzing with trinity inside a KVM tools (lkvm) guest with -next kernel
>>> uncovered the following warning:
>>
>> I cannot reproduce that :(. Do you still see it?
>
> Yes, it reproduces pretty easily while fuzzing.

What is your exact setup? I tried trinity with 100 000 syscalls inside
KVM with an LDEP-enabled kernel. How many serial ports do you have in
the guest? Any USB serials in there?

--
js
suse labs

2012-10-31 15:59:56

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On Wed, Oct 31, 2012 at 11:32 AM, Jiri Slaby <[email protected]> wrote:
> On 10/31/2012 04:30 PM, Sasha Levin wrote:
>> On Wed, Oct 31, 2012 at 8:53 AM, Jiri Slaby <[email protected]> wrote:
>>> On 10/25/2012 08:02 PM, Sasha Levin wrote:
>>>> Fuzzing with trinity inside a KVM tools (lkvm) guest with -next kernel
>>>> uncovered the following warning:
>>>
>>> I cannot reproduce that :(. Do you still see it?
>>
>> Yes, it reproduces pretty easily while fuzzing.
>
> What is your exact setup? I tried trinity with 100 000 syscalls inside
> KVM with an LDEP-enabled kernel. How many serial ports do you have in
> the guest? Any USB serials in there?

So you probably want a lot more than 100k syscalls, why limit it at
all actually?

lkvm is emulating 4 serial ports, no USB ports.

I've attached my .config for the guest kernel as reference.


Thanks,
Sasha


Attachments:
config-sasha (132.36 kB)

2012-10-31 20:10:36

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On 10/31/2012 11:32 AM, Jiri Slaby wrote:
> On 10/31/2012 04:30 PM, Sasha Levin wrote:
>> On Wed, Oct 31, 2012 at 8:53 AM, Jiri Slaby <[email protected]> wrote:
>>> On 10/25/2012 08:02 PM, Sasha Levin wrote:
>>>> Fuzzing with trinity inside a KVM tools (lkvm) guest with -next kernel
>>>> uncovered the following warning:
>>>
>>> I cannot reproduce that :(. Do you still see it?
>>
>> Yes, it reproduces pretty easily while fuzzing.
>
> What is your exact setup? I tried trinity with 100 000 syscalls inside
> KVM with an LDEP-enabled kernel. How many serial ports do you have in
> the guest? Any USB serials in there?

btw, I'm also seeing the following lockups, don't know if it's related:


[ 2283.070569] INFO: task trinity-child20:9161 blocked for more than 120 seconds.
[ 2283.071775] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 2283.074673] trinity-child20 D ffff8800276cb000 5424 9161 6364 0x00000000
[ 2283.076018] ffff880059d9da58 0000000000000002 0000000000000002 0000000000000000
[ 2283.077393] ffff880059d7b000 ffff880059d9dfd8 ffff880059d9dfd8 ffff880059d9dfd8
[ 2283.078763] ffff8800276cb000 ffff880059d7b000 ffff880059d9da78 ffff88001a095180
[ 2283.084144] Call Trace:
[ 2283.085039] [<ffffffff83a98bd5>] schedule+0x55/0x60
[ 2283.086748] [<ffffffff83a98bf3>] schedule_preempt_disabled+0x13/0x20
[ 2283.089000] [<ffffffff83a9735d>] __mutex_lock_common+0x36d/0x5a0
[ 2283.090658] [<ffffffff83a9afb3>] ? tty_lock_nested+0x73/0x80
[ 2283.091691] [<ffffffff83a9afb3>] ? tty_lock_nested+0x73/0x80
[ 2283.092779] [<ffffffff83a975cf>] mutex_lock_nested+0x3f/0x50
[ 2283.093875] [<ffffffff83a9afb3>] tty_lock_nested+0x73/0x80
[ 2283.094872] [<ffffffff83a9afcb>] tty_lock+0xb/0x10
[ 2283.095443] [<ffffffff81bae880>] tty_open+0x270/0x5f0
[ 2283.096181] [<ffffffff8127cda8>] chrdev_open+0xf8/0x1d0
[ 2283.097054] [<ffffffff8127693c>] do_dentry_open+0x1fc/0x310
[ 2283.098015] [<ffffffff8127ccb0>] ? cdev_put+0x20/0x20
[ 2283.098943] [<ffffffff8127777a>] finish_open+0x4a/0x60
[ 2283.099935] [<ffffffff81286947>] do_last+0xb87/0xe70
[ 2283.100910] [<ffffffff812844b0>] ? link_path_walk+0x70/0x900
[ 2283.101553] [<ffffffff81286cf2>] path_openat+0xc2/0x500
[ 2283.102282] [<ffffffff83a9a314>] ? _raw_spin_unlock_irqrestore+0x84/0xb0
[ 2283.103506] [<ffffffff8128716c>] do_filp_open+0x3c/0xa0
[ 2283.104282] [<ffffffff81296c11>] ? __alloc_fd+0x1e1/0x200
[ 2283.105278] [<ffffffff81277c0c>] do_sys_open+0x11c/0x1c0
[ 2283.106519] [<ffffffff81277ccc>] sys_open+0x1c/0x20
[ 2283.107241] [<ffffffff81277d01>] sys_creat+0x11/0x20
[ 2283.107975] [<ffffffff83a9be18>] tracesys+0xe1/0xe6

2012-11-02 15:52:01

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On 10/31/2012 04:59 PM, Sasha Levin wrote:
> So you probably want a lot more than 100k syscalls, why limit it at
> all actually?

I unset the limit but I still can't reproduce...

> I've attached my .config for the guest kernel as reference.

Even using this config does not help to reproduce that.

Do you use some special trinity params?

thanks,
--
js
suse labs

2012-11-02 16:07:32

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On Fri, Nov 2, 2012 at 11:51 AM, Jiri Slaby <[email protected]> wrote:
> On 10/31/2012 04:59 PM, Sasha Levin wrote:
>> So you probably want a lot more than 100k syscalls, why limit it at
>> all actually?
>
> I unset the limit but I still can't reproduce...
>
>> I've attached my .config for the guest kernel as reference.
>
> Even using this config does not help to reproduce that.
>
> Do you use some special trinity params?

Not really:

./trinity -m --quiet --dangerous -l off

Can I add something to my kernel to provide more info when it happens?


Thanks,
Sasha

2012-11-02 16:18:47

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On 11/02/2012 05:07 PM, Sasha Levin wrote:
> On Fri, Nov 2, 2012 at 11:51 AM, Jiri Slaby <[email protected]> wrote:
>> On 10/31/2012 04:59 PM, Sasha Levin wrote:
>>> So you probably want a lot more than 100k syscalls, why limit it at
>>> all actually?
>>
>> I unset the limit but I still can't reproduce...
>>
>>> I've attached my .config for the guest kernel as reference.
>>
>> Even using this config does not help to reproduce that.
>>
>> Do you use some special trinity params?
>
> Not really:
>
> ./trinity -m --quiet --dangerous -l off

Oh, you run that as root??

> Can I add something to my kernel to provide more info when it happens?

Maybe the attached patch can tell us more...

--
js
suse labs


Attachments:
0001-tty-BUF-DEBUG.patch (1.00 kB)

2012-11-02 16:24:09

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On 11/02/2012 12:18 PM, Jiri Slaby wrote:
> On 11/02/2012 05:07 PM, Sasha Levin wrote:
>> On Fri, Nov 2, 2012 at 11:51 AM, Jiri Slaby <[email protected]> wrote:
>>> On 10/31/2012 04:59 PM, Sasha Levin wrote:
>>>> So you probably want a lot more than 100k syscalls, why limit it at
>>>> all actually?
>>>
>>> I unset the limit but I still can't reproduce...
>>>
>>>> I've attached my .config for the guest kernel as reference.
>>>
>>> Even using this config does not help to reproduce that.
>>>
>>> Do you use some special trinity params?
>>
>> Not really:
>>
>> ./trinity -m --quiet --dangerous -l off
>
> Oh, you run that as root??

Yup, it runs inside a disposable VM.


Thanks,
Sasha

2012-11-03 02:04:06

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On 11/02/2012 12:18 PM, Jiri Slaby wrote:
> On 11/02/2012 05:07 PM, Sasha Levin wrote:
>> On Fri, Nov 2, 2012 at 11:51 AM, Jiri Slaby <[email protected]> wrote:
>>> On 10/31/2012 04:59 PM, Sasha Levin wrote:
>>>> So you probably want a lot more than 100k syscalls, why limit it at
>>>> all actually?
>>>
>>> I unset the limit but I still can't reproduce...
>>>
>>>> I've attached my .config for the guest kernel as reference.
>>>
>>> Even using this config does not help to reproduce that.
>>>
>>> Do you use some special trinity params?
>>
>> Not really:
>>
>> ./trinity -m --quiet --dangerous -l off
>
> Oh, you run that as root??
>
>> Can I add something to my kernel to provide more info when it happens?
>
> Maybe the attached patch can tell us more...
>

Nope, I see the warnings mentioned before, without the new 'HUH' warnings.


Thanks,
Sasha

2012-11-03 17:54:34

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On 11/03/2012 03:03 AM, Sasha Levin wrote:
> On 11/02/2012 12:18 PM, Jiri Slaby wrote:
>> On 11/02/2012 05:07 PM, Sasha Levin wrote:
>>> On Fri, Nov 2, 2012 at 11:51 AM, Jiri Slaby <[email protected]> wrote:
>>>> On 10/31/2012 04:59 PM, Sasha Levin wrote:
>>>>> So you probably want a lot more than 100k syscalls, why limit it at
>>>>> all actually?
>>>>
>>>> I unset the limit but I still can't reproduce...
>>>>
>>>>> I've attached my .config for the guest kernel as reference.
>>>>
>>>> Even using this config does not help to reproduce that.
>>>>
>>>> Do you use some special trinity params?
>>>
>>> Not really:
>>>
>>> ./trinity -m --quiet --dangerous -l off
>>
>> Oh, you run that as root??
>>
>>> Can I add something to my kernel to provide more info when it happens?
>>
>> Maybe the attached patch can tell us more...
>>
>
> Nope, I see the warnings mentioned before, without the new 'HUH' warnings.

Actually it does. It is exactly as you wrote some time earlier. The work
is scheduled after is was cancelled and should not trigger anymore. Or,
it is scheduled before it is supposed to do. Could you try the attached
patch and report what happens with that patch?

PS I can't reproduce by whatever I tried.

thanks,
--
js
suse labs


Attachments:
0001-tty-debug-part-2.patch (1.58 kB)

2012-11-03 23:07:12

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On 11/03/2012 11:55 AM, Jiri Slaby wrote:
> On 11/03/2012 03:03 AM, Sasha Levin wrote:
>> On 11/02/2012 12:18 PM, Jiri Slaby wrote:
>>> On 11/02/2012 05:07 PM, Sasha Levin wrote:
>>>> On Fri, Nov 2, 2012 at 11:51 AM, Jiri Slaby <[email protected]> wrote:
>>>>> On 10/31/2012 04:59 PM, Sasha Levin wrote:
>>>>>> So you probably want a lot more than 100k syscalls, why limit it at
>>>>>> all actually?
>>>>>
>>>>> I unset the limit but I still can't reproduce...
>>>>>
>>>>>> I've attached my .config for the guest kernel as reference.
>>>>>
>>>>> Even using this config does not help to reproduce that.
>>>>>
>>>>> Do you use some special trinity params?
>>>>
>>>> Not really:
>>>>
>>>> ./trinity -m --quiet --dangerous -l off
>>>
>>> Oh, you run that as root??
>>>
>>>> Can I add something to my kernel to provide more info when it happens?
>>>
>>> Maybe the attached patch can tell us more...
>>>
>>
>> Nope, I see the warnings mentioned before, without the new 'HUH' warnings.
>
> Actually it does. It is exactly as you wrote some time earlier. The work
> is scheduled after is was cancelled and should not trigger anymore. Or,
> it is scheduled before it is supposed to do. Could you try the attached
> patch and report what happens with that patch?
>
> PS I can't reproduce by whatever I tried.
>
> thanks,
>

Interesting...

[ 388.783955] tty is bad=0 ops= (null)Pid: 6480, comm: kworker/1:2 Tainted: G W
3.7.0-rc3-next-20121102-sasha-00002-gbb570e0-dirty #111


Thanks,
Sasha

2012-11-04 00:53:27

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On 11/03/2012 07:06 PM, Sasha Levin wrote:
> On 11/03/2012 11:55 AM, Jiri Slaby wrote:
>> On 11/03/2012 03:03 AM, Sasha Levin wrote:
>>> On 11/02/2012 12:18 PM, Jiri Slaby wrote:
>>>> On 11/02/2012 05:07 PM, Sasha Levin wrote:
>>>>> On Fri, Nov 2, 2012 at 11:51 AM, Jiri Slaby <[email protected]> wrote:
>>>>>> On 10/31/2012 04:59 PM, Sasha Levin wrote:
>>>>>>> So you probably want a lot more than 100k syscalls, why limit it at
>>>>>>> all actually?
>>>>>>
>>>>>> I unset the limit but I still can't reproduce...
>>>>>>
>>>>>>> I've attached my .config for the guest kernel as reference.
>>>>>>
>>>>>> Even using this config does not help to reproduce that.
>>>>>>
>>>>>> Do you use some special trinity params?
>>>>>
>>>>> Not really:
>>>>>
>>>>> ./trinity -m --quiet --dangerous -l off
>>>>
>>>> Oh, you run that as root??
>>>>
>>>>> Can I add something to my kernel to provide more info when it happens?
>>>>
>>>> Maybe the attached patch can tell us more...
>>>>
>>>
>>> Nope, I see the warnings mentioned before, without the new 'HUH' warnings.
>>
>> Actually it does. It is exactly as you wrote some time earlier. The work
>> is scheduled after is was cancelled and should not trigger anymore. Or,
>> it is scheduled before it is supposed to do. Could you try the attached
>> patch and report what happens with that patch?
>>
>> PS I can't reproduce by whatever I tried.
>>
>> thanks,
>>
>
> Interesting...
>
> [ 388.783955] tty is bad=0 ops= (null)Pid: 6480, comm: kworker/1:2 Tainted: G W
> 3.7.0-rc3-next-20121102-sasha-00002-gbb570e0-dirty #111

So after fuzzing for a while I'm also seeing these:

[ 603.533932] tty is bad=-2 ops= (null)Pid: 37, comm: kworker/4:0 Tainted: G W 3.7.0-rc3-next-20121102-sasha-000
02-gbb570e0-dirty #112


Thanks,
Sasha

2012-11-27 19:57:47

by Peter Hurley

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On Sat, 2012-11-03 at 20:53 -0400, Sasha Levin wrote:
> On 11/03/2012 07:06 PM, Sasha Levin wrote:
> > On 11/03/2012 11:55 AM, Jiri Slaby wrote:
> >> On 11/03/2012 03:03 AM, Sasha Levin wrote:
> >>> On 11/02/2012 12:18 PM, Jiri Slaby wrote:
> >>>> On 11/02/2012 05:07 PM, Sasha Levin wrote:
> >>>>> On Fri, Nov 2, 2012 at 11:51 AM, Jiri Slaby <[email protected]> wrote:
> >>>>>> On 10/31/2012 04:59 PM, Sasha Levin wrote:
> >>>>>>> So you probably want a lot more than 100k syscalls, why limit it at
> >>>>>>> all actually?
> >>>>>>
> >>>>>> I unset the limit but I still can't reproduce...
> >>>>>>
> >>>>>>> I've attached my .config for the guest kernel as reference.
> >>>>>>
> >>>>>> Even using this config does not help to reproduce that.
> >>>>>>
> >>>>>> Do you use some special trinity params?
> >>>>>
> >>>>> Not really:
> >>>>>
> >>>>> ./trinity -m --quiet --dangerous -l off
> >>>>
> >>>> Oh, you run that as root??
> >>>>
> >>>>> Can I add something to my kernel to provide more info when it happens?
> >>>>
> >>>> Maybe the attached patch can tell us more...
> >>>>
> >>>
> >>> Nope, I see the warnings mentioned before, without the new 'HUH' warnings.
> >>
> >> Actually it does. It is exactly as you wrote some time earlier. The work
> >> is scheduled after is was cancelled and should not trigger anymore. Or,
> >> it is scheduled before it is supposed to do. Could you try the attached
> >> patch and report what happens with that patch?
> >>
> >> PS I can't reproduce by whatever I tried.
> >>
> >> thanks,
> >>
> >
> > Interesting...
> >
> > [ 388.783955] tty is bad=0 ops= (null)Pid: 6480, comm: kworker/1:2 Tainted: G W
> > 3.7.0-rc3-next-20121102-sasha-00002-gbb570e0-dirty #111
>
> So after fuzzing for a while I'm also seeing these:
>
> [ 603.533932] tty is bad=-2 ops= (null)Pid: 37, comm: kworker/4:0 Tainted: G W 3.7.0-rc3-next-20121102-sasha-000
> 02-gbb570e0-dirty #112

Hi Sasha,

Assuming this access-after-free is still reproducible for you, would you
be willing to try the patch below? I tried to reproduce this and
couldn't (with multiple cores and with just single core).

It would distinguish between case A (that the buf work is not being
cancelled) and case B (that the buf work is being scheduled after the
port has already been freed). It should BUG in case B, which would also
expose the call chain. It won't help at all in case A though :\

Regards,
Peter Hurley

-- >% --
Subject: [PATCH -next] tty: debug: Narrow possible causes of access-after-free


Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/pty.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index be6a373..893fe69 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -409,6 +409,7 @@ static void pty_cleanup(struct tty_struct *tty)
{
tty->port->itty = NULL;
tty_port_put(tty->port);
+ tty->port = NULL;
}

/* Traditional BSD devices */
--
1.8.0


2012-11-30 23:59:44

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH 21/21] TTY: move tty buffers to tty_port

On 11/27/2012 02:57 PM, Peter Hurley wrote:
> On Sat, 2012-11-03 at 20:53 -0400, Sasha Levin wrote:
>> On 11/03/2012 07:06 PM, Sasha Levin wrote:
>>> On 11/03/2012 11:55 AM, Jiri Slaby wrote:
>>>> On 11/03/2012 03:03 AM, Sasha Levin wrote:
>>>>> On 11/02/2012 12:18 PM, Jiri Slaby wrote:
>>>>>> On 11/02/2012 05:07 PM, Sasha Levin wrote:
>>>>>>> On Fri, Nov 2, 2012 at 11:51 AM, Jiri Slaby <[email protected]> wrote:
>>>>>>>> On 10/31/2012 04:59 PM, Sasha Levin wrote:
>>>>>>>>> So you probably want a lot more than 100k syscalls, why limit it at
>>>>>>>>> all actually?
>>>>>>>>
>>>>>>>> I unset the limit but I still can't reproduce...
>>>>>>>>
>>>>>>>>> I've attached my .config for the guest kernel as reference.
>>>>>>>>
>>>>>>>> Even using this config does not help to reproduce that.
>>>>>>>>
>>>>>>>> Do you use some special trinity params?
>>>>>>>
>>>>>>> Not really:
>>>>>>>
>>>>>>> ./trinity -m --quiet --dangerous -l off
>>>>>>
>>>>>> Oh, you run that as root??
>>>>>>
>>>>>>> Can I add something to my kernel to provide more info when it happens?
>>>>>>
>>>>>> Maybe the attached patch can tell us more...
>>>>>>
>>>>>
>>>>> Nope, I see the warnings mentioned before, without the new 'HUH' warnings.
>>>>
>>>> Actually it does. It is exactly as you wrote some time earlier. The work
>>>> is scheduled after is was cancelled and should not trigger anymore. Or,
>>>> it is scheduled before it is supposed to do. Could you try the attached
>>>> patch and report what happens with that patch?
>>>>
>>>> PS I can't reproduce by whatever I tried.
>>>>
>>>> thanks,
>>>>
>>>
>>> Interesting...
>>>
>>> [ 388.783955] tty is bad=0 ops= (null)Pid: 6480, comm: kworker/1:2 Tainted: G W
>>> 3.7.0-rc3-next-20121102-sasha-00002-gbb570e0-dirty #111
>>
>> So after fuzzing for a while I'm also seeing these:
>>
>> [ 603.533932] tty is bad=-2 ops= (null)Pid: 37, comm: kworker/4:0 Tainted: G W 3.7.0-rc3-next-20121102-sasha-000
>> 02-gbb570e0-dirty #112
>
> Hi Sasha,
>
> Assuming this access-after-free is still reproducible for you, would you
> be willing to try the patch below? I tried to reproduce this and
> couldn't (with multiple cores and with just single core).
>
> It would distinguish between case A (that the buf work is not being
> cancelled) and case B (that the buf work is being scheduled after the
> port has already been freed). It should BUG in case B, which would also
> expose the call chain. It won't help at all in case A though :\
>
> Regards,
> Peter Hurley
>
> -- >% --
> Subject: [PATCH -next] tty: debug: Narrow possible causes of access-after-free
>
>
> Signed-off-by: Peter Hurley <[email protected]>
> ---
> drivers/tty/pty.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
> index be6a373..893fe69 100644
> --- a/drivers/tty/pty.c
> +++ b/drivers/tty/pty.c
> @@ -409,6 +409,7 @@ static void pty_cleanup(struct tty_struct *tty)
> {
> tty->port->itty = NULL;
> tty_port_put(tty->port);
> + tty->port = NULL;
> }
>
> /* Traditional BSD devices */
>

Still reproducible, I'm still seeing this with the patch above applied:

[ 1315.419759] ------------[ cut here ]------------
[ 1315.420611] WARNING: at drivers/tty/tty_buffer.c:476 flush_to_ldisc+0x60/0x200()
[ 1315.423098] tty is NULL
[ 1315.423885] Pid: 727, comm: kworker/0:1 Tainted: G W 3.7.0-rc7-next-20121130-sasha-00015-g06fcc7a-dirty #2
[ 1315.427278] Call Trace:
[ 1315.428064] [<ffffffff81c32ae0>] ? flush_to_ldisc+0x60/0x200
[ 1315.429898] [<ffffffff8110dc56>] warn_slowpath_common+0x86/0xb0
[ 1315.431155] [<ffffffff8110dce1>] warn_slowpath_fmt+0x41/0x50
[ 1315.433087] [<ffffffff81c32ae0>] flush_to_ldisc+0x60/0x200
[ 1315.434972] [<ffffffff811319e9>] process_one_work+0x3b9/0x780
[ 1315.436797] [<ffffffff81131898>] ? process_one_work+0x268/0x780
[ 1315.438660] [<ffffffff81c32a80>] ? __tty_buffer_request_room+0x180/0x180
[ 1315.440772] [<ffffffff8113239a>] worker_thread+0x2ca/0x400
[ 1315.442012] [<ffffffff811320d0>] ? rescuer_thread+0x2e0/0x2e0
[ 1315.443821] [<ffffffff8113d0a3>] kthread+0xe3/0xf0
[ 1315.445362] [<ffffffff811824be>] ? put_lock_stats.isra.16+0xe/0x40
[ 1315.447331] [<ffffffff8113cfc0>] ? insert_kthread_work+0x90/0x90
[ 1315.449254] [<ffffffff83cb107c>] ret_from_fork+0x7c/0xb0
[ 1315.450246] [<ffffffff8113cfc0>] ? insert_kthread_work+0x90/0x90
[ 1315.455389] ---[ end trace 63e808312c27e968 ]---


Thanks,
Sasha