| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c62985530810130154q65a162adg2bc018cfddf4dbc9@mail.gmail.com>
Date: Mon, 13 Oct 2008 10:54:07 +0200
From: "Frédéric Weisbecker" <fweisbec@...il.com>
To: "Ingo Molnar" <mingo@...e.hu>
Cc: Linux-Kernel <linux-kernel@...r.kernel.org>,
"Alan Cox" <alan@...rguk.ukuu.org.uk>
Subject: Re: [RFC][PATCH] tty: remove the BKL in n_tty.c
2008/10/13 Frederic Weisbecker <fweisbec@...il.com>:
>
> By using a bkl tracer, I saw that the opost function in n_tty.c, used when we
> write to the console, requests a lock_kernel(). And since I'm displaying the
> trace during the tracing, I see this lock very often.
>
> The opost and opost_block functions in drivers/char/n_tty.c use
> the bkl to protect the field tty->column.
> Actually this attribute seems to be only touched on these two functions and
> also we are in user context.
>
> A mutex seems to be sufficient to prevent from race conditions here.
> I made some tests by writing on a tty console whith several threads and it
> seems to be correct.
>
> I don't know where to post this patch. I'm not sure if the kill-the-bkl is
> still on the air on the -tip tree.
>
> Any comments?
>
> Signed-off-by: Frederic Weisbecker <fweisbec@...il.com>
> ---
> diff --git a/drivers/char/n_tty.c b/drivers/char/n_tty.c
> index 708c2b1..55c963f 100644
> --- a/drivers/char/n_tty.c
> +++ b/drivers/char/n_tty.c
> @@ -62,6 +62,13 @@
> #define TTY_THRESHOLD_THROTTLE 128 /* now based on remaining room */
> #define TTY_THRESHOLD_UNTHROTTLE 128
>
> +
> +/*
> + * This Mutex is used to replace a big kernel lock that protected
> + * the access to tty->column.
> + */
> +static DEFINE_MUTEX(tty_column_mutex);
> +
> static inline unsigned char *alloc_buf(void)
> {
> gfp_t prio = in_interrupt() ? GFP_ATOMIC : GFP_KERNEL;
> @@ -264,18 +271,19 @@ static inline int is_continuation(unsigned char c, struct tty_struct *tty)
> * relevant in the world today. If you ever need them, add them here.
> *
> * Called from both the receive and transmit sides and can be called
> - * re-entrantly. Relies on lock_kernel() for tty->column state.
> + * re-entrantly.
> */
>
> static int opost(unsigned char c, struct tty_struct *tty)
> {
> int space, spaces;
> + int ret = 0;
>
> space = tty_write_room(tty);
> if (!space)
> return -1;
>
> - lock_kernel();
> + mutex_lock(&tty_column_mutex);
> if (O_OPOST(tty)) {
> switch (c) {
> case '\n':
> @@ -283,8 +291,8 @@ static int opost(unsigned char c, struct tty_struct *tty)
> tty->column = 0;
> if (O_ONLCR(tty)) {
> if (space < 2) {
> - unlock_kernel();
> - return -1;
> + ret = -1;
> + goto out;
> }
> tty_put_char(tty, '\r');
> tty->column = 0;
> @@ -293,8 +301,7 @@ static int opost(unsigned char c, struct tty_struct *tty)
> break;
> case '\r':
> if (O_ONOCR(tty) && tty->column == 0) {
> - unlock_kernel();
> - return 0;
> + goto out;
> }
> if (O_OCRNL(tty)) {
> c = '\n';
> @@ -308,13 +315,12 @@ static int opost(unsigned char c, struct tty_struct *tty)
> spaces = 8 - (tty->column & 7);
> if (O_TABDLY(tty) == XTABS) {
> if (space < spaces) {
> - unlock_kernel();
> - return -1;
> + ret = -1;
> + goto out;
> }
> tty->column += spaces;
> tty->ops->write(tty, " ", spaces);
> - unlock_kernel();
> - return 0;
> + goto out;
> }
> tty->column += spaces;
> break;
> @@ -331,8 +337,9 @@ static int opost(unsigned char c, struct tty_struct *tty)
> }
> }
> tty_put_char(tty, c);
> - unlock_kernel();
> - return 0;
> +out:
> + mutex_unlock(&tty_column_mutex);
> + return ret;
> }
>
> /**
> @@ -346,8 +353,7 @@ static int opost(unsigned char c, struct tty_struct *tty)
> * the simple cases normally found and helps to generate blocks of
> * symbols for the console driver and thus improve performance.
> *
> - * Called from write_chan under the tty layer write lock. Relies
> - * on lock_kernel for the tty->column state.
> + * Called from write_chan under the tty layer write lock.
> */
>
> static ssize_t opost_block(struct tty_struct *tty,
> @@ -363,7 +369,7 @@ static ssize_t opost_block(struct tty_struct *tty,
> if (nr > space)
> nr = space;
>
> - lock_kernel();
> + mutex_lock(&tty_column_mutex);
> for (i = 0, cp = buf; i < nr; i++, cp++) {
> switch (*cp) {
> case '\n':
> @@ -398,7 +404,8 @@ break_out:
> if (tty->ops->flush_chars)
> tty->ops->flush_chars(tty);
> i = tty->ops->write(tty, buf, i);
> - unlock_kernel();
> +
> + mutex_unlock(&tty_column_mutex);
> return i;
> }
>
>
>
(Ading Alan in Cc)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists