Messages in this thread | | | Date | Sun, 26 Apr 2009 08:15:47 +0200 | From | Nick Piggin <> | Subject | Re: [patch 01/27] fs: cleanup files_lock |
| |
On Sat, Apr 25, 2009 at 10:42:34AM +0100, Alan Cox wrote: > On Sat, 25 Apr 2009 11:20:21 +1000 > npiggin@suse.de 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.
| |