On Sat, Apr 25, 2009 at 10:42:34AM +0100, Alan Cox wrote: > On Sat, 25 Apr 2009 11:20:21 +1000 > npiggin@xxxxxxx wrote: > > > Lock tty_files with tty_mutex, provide helpers to manipulate the per-sb > > files list, and unexport the files_lock spinlock. > > This looks half like a backward step to me: It swaps clean method calls > for open coded stuff and it adds more random undocumented uses to > tty_mutex, which has far too much already. > > I don't think > > - file_move(filp, &tty->tty_files); > + > + mutex_lock(&tty_mutex); > + file_list_del(filp); > + list_add(&filp->f_u.fu_list, &tty->tty_files); > + mutex_unlock(&tty_mutex); > > is exactly an improvement, nor is > > - file_move(filp, &tty->tty_files); > - check_tty_count(tty, "tty_open"); > + mutex_lock(&tty_mutex); > + BUG_ON(list_empty(&filp->f_u.fu_list)); > + file_list_del(filp); /* __dentry_open has put it on the sb list > */ > + list_add(&filp->f_u.fu_list, &tty->tty_files); > + __check_tty_count(tty, "tty_open"); > + mutex_unlock(&tty_mutex); > > The basic idea looks totally sound but it can use its own lock and there > should be helpers so this stuff doesn't have to get open coded. Yes, I agree it was silly to try reusing tty_mutex for this, as you and Al point out. I've just added a new spinlock for the tty layer for the moment, which makes it much more like a mechanical search/ replace. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
