public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ilya Zykov <ilya@ilyx.ru>
To: Alan Cox <alan@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>, linux-kernel@vger.kernel.org
Subject: Re: PROBLEM: Race condition in tty buffer's function flush_to_ldisc().
Date: Mon, 07 Nov 2011 22:26:15 +0400	[thread overview]
Message-ID: <4EB822C7.6070200@ilyx.ru> (raw)
In-Reply-To: <20111107145005.68ca1050@bob.linux.org.uk>

Alan Cox wrote:

>> Of course I know about tty_buffer_flush(), it only read TTY_FLUSHING,
>> it can't change TTY_FLUSHING, if flush_to_ldisc() single threaded,
>> we can change TTY_FLUSHING only in one place in one time(in
>> flush_to_ldisc()), therefor we can use only "set_bit(TTY_FLUSHING,
>> &tty->flags)" without test.
> 
> Yes.. if you can pin down why in your testing you see the other case
> sometimes being true.
> 
> Alan
>  

Nested call flush_to_ldisc() happened on different CPU only. It happens because
one side pty call "schedule_work(&tty->buf.work)" on one CPU from "tty_flip_buffer_push()",
the other side call "schedule_work(&tty->buf.work)" from n_tty's layer from "n_tty_set_room()"
on different CPU. It also can happen in interrupt if interrupt handle on 
different CPU. Schedule_work() schedule work on CPU which was called(IMHO).

For testing I use in xterminal: cat big_file
and flush_to_ldisc() with this patch:

diff -uprN -X ../../../dontdiff a/drivers/tty/tty_buffer.c c/drivers/tty/tty_buffer.c
--- a/drivers/tty/tty_buffer.c	2011-11-07 14:45:27.000000000 +0400
+++ c/drivers/tty/tty_buffer.c	2011-11-07 21:48:49.000000000 +0400
@@ -405,13 +405,14 @@ static void flush_to_ldisc(struct work_s
 		container_of(work, struct tty_struct, buf.work);
 	unsigned long 	flags;
 	struct tty_ldisc *disc;
+	static int mthread;
 
 	disc = tty_ldisc_ref(tty);
 	if (disc == NULL)	/*  !TTY_LDISC */
 		return;
 
 	spin_lock_irqsave(&tty->buf.lock, flags);
-
+	mthread = 0;
 	if (!test_and_set_bit(TTY_FLUSHING, &tty->flags)) {
 		struct tty_buffer *head;
 		while ((head = tty->buf.head) != NULL) {
@@ -445,6 +446,12 @@ static void flush_to_ldisc(struct work_s
 			spin_lock_irqsave(&tty->buf.lock, flags);
 		}
 		clear_bit(TTY_FLUSHING, &tty->flags);
+	} else {
+		mthread = 1;
+		printk(KERN_WARNING "Tty %s FLUSHING CPU %d.\n", tty->name, percpu_read(cpu_number));
+	}
+	if (mthread) {
+		printk(KERN_WARNING "Tty %s FLUSHING multithreaded CPU %d.\n", tty->name, percpu_read(cpu_number));
 	}
 
 	/* We may have a deferred request to flush the input buffer,

And get in my syslog:

Nov  7 21:34:13 serh kernel: [   76.323848] Tty ptm0 FLUSHING CPU 0.
Nov  7 21:34:13 serh kernel: [   76.323850] Tty ptm0 FLUSHING multithreaded CPU 0.
Nov  7 21:34:13 serh kernel: [   76.323856] Tty ptm0 FLUSHING CPU 0.
Nov  7 21:34:13 serh kernel: [   76.323857] Tty ptm0 FLUSHING multithreaded CPU 0.
Nov  7 21:34:13 serh kernel: [   76.323861] Tty ptm0 FLUSHING multithreaded CPU 1.
Nov  7 21:34:13 serh kernel: [   76.336022] Tty ptm0 FLUSHING CPU 0.
Nov  7 21:34:13 serh kernel: [   76.336024] Tty ptm0 FLUSHING multithreaded CPU 0.
Nov  7 21:34:13 serh kernel: [   76.336030] Tty ptm0 FLUSHING multithreaded CPU 1.
Nov  7 21:34:13 serh kernel: [   76.353134] Tty ptm0 FLUSHING CPU 0.
Nov  7 21:34:13 serh kernel: [   76.353136] Tty ptm0 FLUSHING multithreaded CPU 0.
Nov  7 21:34:13 serh kernel: [   76.353143] Tty ptm0 FLUSHING CPU 0.
Nov  7 21:34:13 serh kernel: [   76.353145] Tty ptm0 FLUSHING multithreaded CPU 0.
Nov  7 21:34:13 serh kernel: [   76.353148] Tty ptm0 FLUSHING multithreaded CPU 1.
......
......

  parent reply	other threads:[~2011-11-07 18:26 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-07 11:14 PROBLEM: Race condition in tty buffer's function flush_to_ldisc() Ilya Zykov
2011-11-07 11:58 ` Alan Cox
2011-11-07 12:20   ` Ilya Zykov
2011-11-07 13:06     ` Alan Cox
2011-11-07 13:44       ` Ilya Zykov
2011-11-07 14:50         ` Alan Cox
2011-11-07 14:52           ` Ilya Zykov
2011-11-07 18:26           ` Ilya Zykov [this message]
2011-11-07 18:39             ` Alan Cox
2011-11-07 18:41               ` Alan Cox
2011-11-07 12:35   ` Ilya Zykov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EB822C7.6070200@ilyx.ru \
    --to=ilya@ilyx.ru \
    --cc=alan@linux.intel.com \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox