mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@suse.de>
To: linux-kernel@vger.kernel.org
Cc: Lawrence Rust <lvr@softsystem.co.uk>,
	Greg Kroah-Hartman <gregkh@suse.de>
Subject: [PATCH 11/19] 8250: Fix tcsetattr to avoid ioctl(TIOCMIWAIT) hang
Date: Fri, 12 Nov 2010 13:40:58 -0800	[thread overview]
Message-ID: <1289598066-10112-11-git-send-email-gregkh@suse.de> (raw)
In-Reply-To: <20101112213215.GB9294@kroah.com>

From: Lawrence Rust <lvr@softsystem.co.uk>

Calling tcsetattr prevents any thread(s) currently suspended in ioctl
TIOCMIWAIT for the same device from ever resuming.

If a thread is suspended inside a call to ioctl TIOCMIWAIT, waiting for
a modem status change, then the 8250 driver enables modem status
interrupts (MSI).  The device interrupt service routine resumes the
suspended thread(s) on the next MSI.

If while the thread(s) are suspended, another thread calls tcsetattr
then the 8250 driver disables MSI (unless CTS/RTS handshaking is
enabled) thus preventing the suspended thread(s) from ever being
resumed.

This patch only disables MSI in tcsetattr handling if there are no
suspended threads.

Program to demonstrate bug & fix:

/* gcc miwait.c -o miwait -l pthread */
#include <stdio.h>
#include <errno.h>
#include <unistd.h>
#include <fcntl.h>
#include <pthread.h>
#include <termios.h>
#include <sys/ioctl.h>
#include <linux/serial.h>

static void* monitor( void* pv);
static int s_fd;

int main( void)
  {
  const char kszDev[] = "/dev/ttyS0";
  pthread_t t;
  struct termios tio;

  s_fd = open( kszDev, O_RDWR | O_NONBLOCK);
  if ( s_fd < 0)
    return fprintf( stderr, "Error(%d) opening %s: %s\n", errno, kszDev, strerror( errno)), 1;

  pthread_create( &t, NULL, &monitor, NULL);

  /* Modem status changes seen here */
  puts( "Main: awaiting status changes");
  sleep( 5);

  tcgetattr( s_fd, &tio);
  tio.c_cflag ^= CSTOPB;

  /* But not after here */
  puts( "Main: tcsetattr called");
  tcsetattr( s_fd, TCSANOW, &tio);

  for (;;)
    sleep( 1);
  }

static void* monitor( void* pv)
  {
  (void)pv;
  for(;;)
    {
    unsigned uModem;
    struct serial_icounter_struct cnt;

    if ( ioctl( s_fd, TIOCMGET, &uModem) < 0)
      fprintf( stderr, "Error(%d) in TIOCMGET: %s\n", errno, strerror( errno));
    printf( "Modem status:%s%s%s%s%s%s\n",
      (uModem & TIOCM_RTS) ? " RTS" : "",
      (uModem & TIOCM_DTR) ? " DTR" : "",
      (uModem & TIOCM_CTS) ? " CTS" : "",
      (uModem & TIOCM_DSR) ? " DSR" : "",
      (uModem & TIOCM_CD) ? " CD" : "",
      (uModem & TIOCM_RI) ? " RI" : ""
    );

    if ( ioctl( s_fd, TIOCGICOUNT, &cnt) < 0)
      fprintf( stderr, "Error(%d) in TIOCGICOUNT: %s\n", errno, strerror( errno));
    printf( "Irqs: CTS:%d DSR:%d RNG:%d DCD:%d Rx:%d Tx:%d Frame:%d Orun:%d Par:%d Brk:%d Oflow:%d\n",
      cnt.cts, cnt.dsr, cnt.rng, cnt.dcd,
      cnt.rx, cnt.tx, cnt.frame, cnt.overrun, cnt.parity,
      cnt.brk, cnt.buf_overrun
    );

    fputs( "Waiting...", stdout), fflush( stdout);
    if ( 0 > ioctl( s_fd, TIOCMIWAIT, (unsigned long)(TIOCM_CAR | TIOCM_RNG | TIOCM_DSR | TIOCM_CTS)))
      fprintf( stderr, "\nError(%d) in TIOCMIWAIT: %s\n", errno, strerror( errno));
    fputs( "\n", stdout);
    }
  return NULL;
  }

Signed-off by Lawrence Rust <lawrence@softsystem.co.uk>

Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/serial/8250.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 4d8e14b..dd5e1ac 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -2343,8 +2343,11 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	/*
 	 * CTS flow control flag and modem status interrupts
+	 * Only disable MSI if no threads are waiting in
+	 * serial_core::uart_wait_modem_status
 	 */
-	up->ier &= ~UART_IER_MSI;
+	if (!waitqueue_active(&up->port.state->port.delta_msr_wait))
+		up->ier &= ~UART_IER_MSI;
 	if (!(up->bugs & UART_BUG_NOMSR) &&
 			UART_ENABLE_MS(&up->port, termios->c_cflag))
 		up->ier |= UART_IER_MSI;
-- 
1.7.1


  parent reply	other threads:[~2010-11-12 21:43 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-12 21:32 [GIT PATCH] TTY/serial fixes for .37-rc1 Greg KH
2010-11-12 21:40 ` [PATCH 01/19] tty: the development tree is now done in git Greg Kroah-Hartman
2010-11-12 21:40 ` [PATCH 02/19] tty: Fix formatting in tty.h Greg Kroah-Hartman
2010-11-12 21:40 ` [PATCH 03/19] tty: fix warning in synclink driver Greg Kroah-Hartman
2010-11-12 21:40 ` [PATCH 04/19] nozomi: Fix warning from the previous TIOCGCOUNT changes Greg Kroah-Hartman
2010-11-12 21:40 ` [PATCH 05/19] 8250: add support for Kouwell KW-L221N-2 Greg Kroah-Hartman
2010-11-12 21:40 ` [PATCH 06/19] tty: prevent DOS in the flush_to_ldisc Greg Kroah-Hartman
2010-11-12 21:40 ` [PATCH 07/19] drivers/serial/bfin_5xx.c: Fix line continuation defects Greg Kroah-Hartman
2010-11-12 21:40 ` [PATCH 08/19] SERIAL: blacklist si3052 chip Greg Kroah-Hartman
2010-11-12 21:40 ` [PATCH 10/19] tty_ldisc: Fix BUG() on hangup Greg Kroah-Hartman
2010-12-07 21:00   ` Russ Dill
2010-11-12 21:40 ` Greg Kroah-Hartman [this message]
2010-11-13  4:18   ` [PATCH 11/19] 8250: Fix tcsetattr to avoid ioctl(TIOCMIWAIT) hang Alexey Zaytsev
2010-11-13 14:49     ` Greg KH
2010-11-13 17:42       ` Alan Cox
2010-11-13 19:22       ` Lawrence Rust
2010-11-13 23:49         ` Greg KH
2010-11-12 21:40 ` [PATCH 12/19] amiserial: Remove unused variable icount Greg Kroah-Hartman
2010-11-12 21:41 ` [PATCH 13/19] vcs: make proper usage of the poll flags Greg Kroah-Hartman
2010-11-12 21:41 ` [PATCH 14/19] serial: bfin_5xx: always include DMA headers Greg Kroah-Hartman
2010-11-12 21:41 ` [PATCH 15/19] serial: bfin_5xx: remove redundant SSYNC to improve TX speed Greg Kroah-Hartman
2010-11-12 21:41 ` [PATCH 16/19] serial: bfin_5xx: disable CON_PRINTBUFFER for consoles Greg Kroah-Hartman
2010-11-12 21:41 ` [PATCH 17/19] serial: bfin_5xx: grab port lock before making port termios changes Greg Kroah-Hartman
2010-11-12 21:41 ` [PATCH 18/19] n_gsm: Copy n2 over when configuring via ioctl interface Greg Kroah-Hartman
2010-11-12 21:41 ` [PATCH 19/19] n_gsm: Fix length handling Greg Kroah-Hartman
2010-11-13 15:10 ` [boot crash] Re: [GIT PATCH] TTY/serial fixes for .37-rc1 Ingo Molnar
2010-11-13 15:21   ` Ingo Molnar
2010-11-13 15:25     ` Ingo Molnar
2010-11-13 23:49       ` Greg KH

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=1289598066-10112-11-git-send-email-gregkh@suse.de \
    --to=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lvr@softsystem.co.uk \
    /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

Powered by JetHome