RFC/CFT 0/1: Alternative tty fasync fix

RFC/CFT 0/1: Alternative tty fasync fix

[Russell King]

Hi,

tty stuff came up in some conversations this morning, which caused a
re-think the fix that went into 2.5.60 for the tty_fasync whinge.
I think the following is an all-round better fix for the problem.
(I'm not saying that the 2.5.60 fix is wrong though.)

Instead of detecting when filp->private_data becomes NULL in
do_tty_hangup and check_tty_count, we remove the file descriptor
from the list of descriptors associated with the tty.  We use the
same method that dentry_open() uses.

In addition, we change the for() loops into real list_for_each()
loops.

If the general concensus is that this is a better method, please
speak up and I'll push Linus-ward.

--- orig/drivers/char/tty_io.c	Tue Feb 11 16:10:08 2003
+++ linux/drivers/char/tty_io.c	Fri Feb 14 20:03:44 2003
@@ -229,9 +229,8 @@
 	int count = 0;
 	
 	file_list_lock();
-	for(p = tty->tty_files.next; p != &tty->tty_files; p = p->next) {
-		if(list_entry(p, struct file, f_list)->private_data == tty)
-			count++;
+	list_for_each(p, &tty->tty_files) {
+		count++;
 	}
 	file_list_unlock();
 	if (tty->driver.type == TTY_DRIVER_TYPE_PTY &&
@@ -440,15 +439,8 @@
 	
 	check_tty_count(tty, "do_tty_hangup");
 	file_list_lock();
-	for (l = tty->tty_files.next; l != &tty->tty_files; l = l->next) {
+	list_for_each(l, &tty->tty_files) {
 		struct file * filp = list_entry(l, struct file, f_list);
-		/*
-		 * If this file descriptor has been closed, ignore it; it
-		 * will be going away shortly. (We don't test filp->f_count
-		 * for zero since that could open another race.) --rmk
-		 */
-		if (filp->private_data == NULL)
-			continue;
 		if (IS_CONSOLE_DEV(filp->f_dentry->d_inode->i_rdev) ||
 		    IS_SYSCONS_DEV(filp->f_dentry->d_inode->i_rdev)) {
 			cons_filp = filp;
@@ -1060,6 +1052,7 @@
  */
 static void release_dev(struct file * filp)
 {
+	static LIST_HEAD(kill_list);
 	struct tty_struct *tty, *o_tty;
 	int	pty_master, tty_closing, o_tty_closing, do_sleep;
 	int	idx;
@@ -1210,14 +1203,16 @@
 	}
 
 	/*
-	 * We've decremented tty->count, so we should zero out
-	 * filp->private_data, to break the link between the tty and
-	 * the file descriptor.  Otherwise if filp_close() blocks before
-	 * the file descriptor is removed from the inuse_filp
-	 * list, check_tty_count() could observe a discrepancy and
-	 * printk a warning message to the user.
+	 * We've decremented tty->count, so we need to remove this file
+	 * descriptor off the tty->tty_files list; this serves two
+	 * purposes:
+	 *  - check_tty_count sees the correct number of file descriptors
+	 *    associated with this tty.
+	 *  - do_tty_hangup no longer sees this file descriptor as
+	 *    something that needs to be handled for hangups.
 	 */
-	filp->private_data = 0;
+	file_move(filp, &kill_list);
+	filp->private_data = NULL;
 
 	/*
 	 * Perform some housekeeping before deciding whether to return.


-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html