linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Benie <pjb1008@myrddin.greenend.org.uk>
To: linux-kernel@vger.kernel.org
Subject: [PATCH] 2.2/2.4 write can block in non-blocking write
Date: Thu, 6 Dec 2001 00:33:26 +0000	[thread overview]
Message-ID: <15374.48342.184932.392175@myrddin.sinister.greenend.org.uk> (raw)

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 778 bytes --]

This mail contains four parts.
Part 1 - (This part) Description of the bug
Part 2 - Demonstrates the bug
Part 3 - Patch for 2.2 series kernels
Part 4 - Patch for 2.4 series kernels

When writing to a tty device, a process can block in write(2) even if
the file descriptor is non-blocking. Not many processes rely on
non-blocking writes to terminal devices, however, one process that
does is 'wall'. The bug typically manifests itself as delayed
messages.  (When the terminal that caused the blockage is closed, the
wall process continues and notifies users about events that have long
since past.)

The cause is that do_tty_write takes out the atomic_write lock using
down_interruptible. What it should do is to use down_trylock when the 
non-blocking flag is set.

Peter Benie

[-- Attachment #2: Demonstration of blocking write bug --]
[-- Type: text/plain, Size: 1966 bytes --]

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <string.h>
#include <errno.h>
#include <sys/wait.h>

void die(const char *msg) 
{fprintf(stderr, "%s: %s\n",msg,strerror(errno)); exit(1);}

void timer_expired(int sig) {}

int main(void)
{
	int pty_master_fd, pty_slave_fd1, pty_slave_fd2, i, err;
	const char *slave_name;
	char data[2048];
	pid_t pid;

	/* Open the master and two slaves. Make one slave non-blocking. */
	if ((pty_master_fd=open("/dev/ptmx", O_RDWR))==-1) die("open ptmx");
	if (grantpt(pty_master_fd)) die("grantpt");
	if (unlockpt(pty_master_fd)) die("unlockpt");
	if (!(slave_name=ptsname(pty_master_fd))) die("ptsname");
	if ((pty_slave_fd1=open(slave_name, O_RDWR))==-1) die(slave_name);
	if ((pty_slave_fd2=open(slave_name, O_RDWR))==-1) die(slave_name);
	if (fcntl(pty_slave_fd1, F_SETFL, O_NONBLOCK)==-1) die("fcntl");

	/* Fill the tty buffer until full */
	memset(data, 0, 2048);
	for (i=0; i<10; i++)
	{
		if (write(pty_slave_fd1, data, 2048)==-1) {
			if (errno==EINTR) continue;
			if (errno==EAGAIN) break;
			die("write");
		}
	}

	/* Fork, and do a blocking write in the child */
	if ((pid=fork())==-1) die("fork");
	if (!pid)
	{
		if (write(pty_slave_fd2, data, 2048)==-1)
			die("write");
		_exit(0);
	}
	sleep(1);
	alarm(0);
	signal(SIGALRM, timer_expired);
	alarm(1);
	errno=0;

	/* In the parent, do a non-blocking write while the atomic_write
	   semaphore is held. */
	if (write(pty_slave_fd1, data, 2048)==-1) {
		if (errno!=EINTR && errno!=EAGAIN) die("write");
	}
	err=errno;
	if (kill(pid, SIGTERM)) die("kill");
		
	switch(errno)
	{
	case EAGAIN:
		printf("write failed with EAGAIN. Good.\n");
		return 0;
	case 0:
		printf("write completed sucessfully. "
			"Bug test failed - run program again.\n");
		return 1;
	case EINTR:
		printf("write failed with EINTR.\n"
			"write blocked despite file descriptor being "
			"non-blocking!\n");
		return 2;
	}
	return 1; /* NOTREACHED */
}

[-- Attachment #3: tty-nonblock-2.2.patch --]
[-- Type: text/plain, Size: 651 bytes --]

--- 2.2.19/drivers/char/tty_io.c~	Sun Mar 25 17:31:24 2001
+++ 2.2.19/drivers/char/tty_io.c	Mon Dec  3 23:31:43 2001
@@ -667,9 +667,17 @@
 	struct inode *inode = file->f_dentry->d_inode;
 	
 	up(&inode->i_sem);
-	if (down_interruptible(&tty->atomic_write)) {
-		down(&inode->i_sem);
-		return -ERESTARTSYS;
+	if (file->f_flags & O_NONBLOCK) {
+		if (down_trylock(&tty->atomic_write)) {
+			down(&inode->i_sem);
+			return -EAGAIN;
+		}
+	}
+	else {
+		if (down_interruptible(&tty->atomic_write)) {
+			down(&inode->i_sem);
+			return -ERESTARTSYS;
+		}
 	}
 	if ( test_bit(TTY_NO_WRITE_SPLIT, &tty->flags) )
 		written = write(tty, file, buf, count);

[-- Attachment #4: tty-nonblock-2.4.patch --]
[-- Type: text/plain, Size: 509 bytes --]

--- 2.4.14/drivers/char/tty_io.c.orig	Thu Dec  6 00:07:23 2001
+++ 2.4.14/drivers/char/tty_io.c	Thu Dec  6 00:09:41 2001
@@ -697,8 +697,13 @@
 {
 	ssize_t ret = 0, written = 0;
 	
-	if (down_interruptible(&tty->atomic_write)) {
-		return -ERESTARTSYS;
+	if (file->f_flags & O_NONBLOCK) {
+		if (down_trylock(&tty->atomic_write))
+			return -EAGAIN;
+	}
+	else {
+		if (down_interruptible(&tty->atomic_write))
+			return -ERESTARTSYS;
 	}
 	if ( test_bit(TTY_NO_WRITE_SPLIT, &tty->flags) ) {
 		lock_kernel();

                 reply	other threads:[~2001-12-06  0:33 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=15374.48342.184932.392175@myrddin.sinister.greenend.org.uk \
    --to=pjb1008@myrddin.greenend.org.uk \
    --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;
as well as URLs for NNTP newsgroup(s).