All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alan Cox <alan@linux.intel.com>
To: Ilya Zykov <ilya@ilyx.ru>
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, 7 Nov 2011 13:06:43 +0000	[thread overview]
Message-ID: <20111107130643.07e84fca@bob.linux.org.uk> (raw)
In-Reply-To: <4EB7CCFF.5090304@ilyx.ru>

> if (!test_and_set_bit(TTY_FLUSHING, &tty->flags)) {
> 	.....
> } else
> 	prink...
> In my syslog appear this prink on pty devices.

Then we need to understand the call paths that did this. I suspect it
may be a bug in the hacks Linus made to n_tty. They've always bothered
me as they never looked correct.

Can you get traces to see if it is actually calling flush_to_ldisc
multithreaded on a given tty

> Why we need: "if (!test_and_set_bit(TTY_FLUSHING, &tty->flags)) {"
> if flush_to_ldisc is single threaded?
> we can: set_bit(TTY_FLUSHING, &tty->flags)
> without if() at all.

It is single threaded with respect to itself (you can't have two
flush_to_ldisc on the same tty at once) but you can have a parallel
call to tty_buffer_flush. The tty_buffer_flush path needs to pick the
right approach reliably.

So the theory is

If TTY_FLUSHING is set then tty_buffer_flush lets the flush_to_ldisc do
the work. During this time we won't fluish a buffer under the ldisc.

If TTY_FLUSHING is not set then we will do the flush directly. As we
hold tty->buf.lock at that point the two cannot race as far as I can
see.

We are actually now probably at the point we could take a per tty mutex
on the flush_to_ldisc and use it to tidy up ldisc change, flush and
other things. So this code could welll be something worth simplifying
once the rest of the tty_lock() is cleaned out.

Alan

  reply	other threads:[~2011-11-07 12:55 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 [this message]
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
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=20111107130643.07e84fca@bob.linux.org.uk \
    --to=alan@linux.intel.com \
    --cc=gregkh@suse.de \
    --cc=ilya@ilyx.ru \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.