Messages in this thread Patch in this message | | | Date | Fri, 14 Feb 2003 20:07:37 +0000 | From | Russell King <> | Subject | RFC/CFT 0/1: Alternative tty fasync fix |
| |
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
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |