Re: [patch 01/27] fs: cleanup files_lock

[Alan Cox]

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/