Theodore Y. Ts'o wrote:
> At least some (if not most) of the recent CHECK_TTY_COUNTS appear to be
> caused by the syscons patch which went in. I'm not yet 100% convinced
> that this solves all of the problems though, although it probably
> accounts for at least 90% of the cases reported in the last few weeks.
Hi Ted,
I did some further review of the tty release_dev and file closing paths, and
have found a small problem in the handling of filp->private_data that could
(rarely) trigger one of the tty count warning messages. It involves a subtle
race that may result in the filp->private_data for a closed file being
counted as an active use of the tty.
The filp->private_data field defines the association between files and ttys,
with one tty possibly having many associated files. Since the call to
release_dev() indicates the last use of a file, the association with the tty
is logically complete at this time, and private_data should be cleared.
However, the current code only clears private_data when the _tty_ is closed,
which happens only when the last of the multiple files is closed.
Normally this isn't a problem, as the released file is soon removed from the
inuse_filp list. But an examination of the closing path shows calls in the
following sequence:
close_fp
fput
__fput
release (resolves to tty release_dev)
dput <-- could block
remove_filp <-- filp removed from inuse_filp
insert_file_free
After the call to release_dev the file will still be on the inuse_filp list
with private_data pointing to the tty, even though the tty->count for the
file has been decremented. The race occurs because the call to dput() could
block, thereby allowing a call to check_tty_count to observe the inconsistent
state.
The problem is easy to fix, as we simply need to clear filp->private_data
after the last call requiring the file. The attached patch should take care
of the problem, and hopefully will solve the remaining 10% of the tty count
discrepancies.
Regards,
Bill
--------------3CCB674AF48403B5A678AEC3
Content-Type: text/plain; charset=us-ascii; name="tty_release115-patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="tty_release115-patch"
--- linux-2.1.115/drivers/char/tty_io.c.old Thu Aug 6 16:54:08 1998
+++ linux-2.1.115/drivers/char/tty_io.c Sun Aug 9 09:09:54 1998
@@ -1051,6 +1051,15 @@
if (tty->driver.close)
tty->driver.close(tty, filp);
+ /*
+ * Since release_dev() is called to indicate the last use of a file,
+ * we disassociate the tty regardless of tty->count.
+ */
+ filp->private_data = 0;
+ if (filp->f_count != 1)
+ printk(KERN_WARNING "release_dev: file count %d not 1\n",
+ filp->f_count);
+
/*
* Sanity check: if tty->count is going to zero, there shouldn't be
* any waiters on tty->read_wait or tty->write_wait. We test the
@@ -1155,7 +1164,6 @@
/* check whether both sides are closing ... */
if (!tty_closing || (o_tty && !o_tty_closing))
return;
- filp->private_data = 0;
#ifdef TTY_DEBUG_HANGUP
printk("freeing tty structure...");
--------------3CCB674AF48403B5A678AEC3--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.altern.org/andrebalsa/doc/lkml-faq.html