fs: cleanup files_lock locking

fs: cleanup files_lock locking Lock tty_files with a new spinlock, tty_files_lock; provide helpers to manipulate the per-sb files list; unexport the files_lock spinlock. Cc: linux-kernel@vger.kernel.org Cc: Christoph Hellwig <hch@infradead.org> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> Acked-by: Andi Kleen <ak@linux.intel.com> Acked-by: Greg Kroah-Hartman <gregkh@suse.de> Signed-off-by: Nick Piggin <npiggin@kernel.dk> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

fs: cleanup files_lock locking
fs: cleanup files_lock locking Lock tty_files with a new spinlock, tty_files_lock; provide helpers to manipulate the per-sb files list; unexport the files_lock spinlock. Cc: linux-kernel@vger.kernel.org Cc: Christoph Hellwig <hch@infradead.org> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> Acked-by: Andi Kleen <ak@linux.intel.com> Acked-by: Greg Kroah-Hartman <gregkh@suse.de> Signed-off-by: Nick Piggin <npiggin@kernel.dk> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Nick Piggin · Al Viro
1 parent b04f784e5d
Showing 7 changed files with 48 additions and 42 deletions Side-by-side Diff
drivers/char/pty.c
drivers/char/tty_io.c
fs/file_table.c
fs/open.c
include/linux/fs.h
include/linux/tty.h
security/selinux/hooks.c
@@ -676,7 +676,11 @@
  
 	set_bit(TTY_PTY_LOCK, &tty->flags); /* LOCK THE SLAVE */
 	filp->private_data = tty;
-	file_move(filp, &tty->tty_files);
+
+	file_sb_list_del(filp); /* __dentry_open has put it on the sb list */
+	spin_lock(&tty_files_lock);
+	list_add(&filp->f_u.fu_list, &tty->tty_files);
+	spin_unlock(&tty_files_lock);
  
 	retval = devpts_pty_new(inode, tty->link);
 	if (retval)
@@ -136,6 +136,9 @@
 DEFINE_MUTEX(tty_mutex);
 EXPORT_SYMBOL(tty_mutex);
  
+/* Spinlock to protect the tty->tty_files list */
+DEFINE_SPINLOCK(tty_files_lock);
+
 static ssize_t tty_read(struct file *, char __user *, size_t, loff_t *);
 static ssize_t tty_write(struct file *, const char __user *, size_t, loff_t *);
 ssize_t redirected_tty_write(struct file *, const char __user *,
  
@@ -235,11 +238,11 @@
 	struct list_head *p;
 	int count = 0;
  
-	file_list_lock();
+	spin_lock(&tty_files_lock);
 	list_for_each(p, &tty->tty_files) {
 		count++;
 	}
-	file_list_unlock();
+	spin_unlock(&tty_files_lock);
 	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
 	    tty->driver->subtype == PTY_TYPE_SLAVE &&
 	    tty->link && tty->link->count)
@@ -519,7 +522,7 @@
 	   workqueue with the lock held */
 	check_tty_count(tty, "tty_hangup");
  
-	file_list_lock();
+	spin_lock(&tty_files_lock);
 	/* This breaks for file handles being sent over AF_UNIX sockets ? */
 	list_for_each_entry(filp, &tty->tty_files, f_u.fu_list) {
 		if (filp->f_op->write == redirected_tty_write)
@@ -530,7 +533,7 @@
 		__tty_fasync(-1, filp, 0);	/* can't block */
 		filp->f_op = &hung_up_tty_fops;
 	}
-	file_list_unlock();
+	spin_unlock(&tty_files_lock);
  
 	tty_ldisc_hangup(tty);
  
  
@@ -1424,9 +1427,9 @@
 	tty_driver_kref_put(driver);
 	module_put(driver->owner);
  
-	file_list_lock();
+	spin_lock(&tty_files_lock);
 	list_del_init(&tty->tty_files);
-	file_list_unlock();
+	spin_unlock(&tty_files_lock);
  
 	put_pid(tty->pgrp);
 	put_pid(tty->session);
@@ -1671,7 +1674,10 @@
 	 *  - do_tty_hangup no longer sees this file descriptor as
 	 *    something that needs to be handled for hangups.
 	 */
-	file_kill(filp);
+	spin_lock(&tty_files_lock);
+	BUG_ON(list_empty(&filp->f_u.fu_list));
+	list_del_init(&filp->f_u.fu_list);
+	spin_unlock(&tty_files_lock);
 	filp->private_data = NULL;
  
 	/*
@@ -1840,7 +1846,11 @@
 	}
  
 	filp->private_data = tty;
-	file_move(filp, &tty->tty_files);
+	BUG_ON(list_empty(&filp->f_u.fu_list));
+	file_sb_list_del(filp); /* __dentry_open has put it on the sb list */
+	spin_lock(&tty_files_lock);
+	list_add(&filp->f_u.fu_list, &tty->tty_files);
+	spin_unlock(&tty_files_lock);
 	check_tty_count(tty, "tty_open");
 	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
 	    tty->driver->subtype == PTY_TYPE_MASTER)
@@ -32,8 +32,7 @@
 	.max_files = NR_FILE
 };
  
-/* public. Not pretty! */
-__cacheline_aligned_in_smp DEFINE_SPINLOCK(files_lock);
+static __cacheline_aligned_in_smp DEFINE_SPINLOCK(files_lock);
  
 /* SLAB cache for file structures */
 static struct kmem_cache *filp_cachep __read_mostly;
@@ -249,7 +248,7 @@
 		cdev_put(inode->i_cdev);
 	fops_put(file->f_op);
 	put_pid(file->f_owner.pid);
-	file_kill(file);
+	file_sb_list_del(file);
 	if (file->f_mode & FMODE_WRITE)
 		drop_file_write_access(file);
 	file->f_path.dentry = NULL;
  
  
  
  
  
  
@@ -328,31 +327,29 @@
 	return file;
 }
  
-
 void put_filp(struct file *file)
 {
 	if (atomic_long_dec_and_test(&file->f_count)) {
 		security_file_free(file);
-		file_kill(file);
+		file_sb_list_del(file);
 		file_free(file);
 	}
 }
  
-void file_move(struct file *file, struct list_head *list)
+void file_sb_list_add(struct file *file, struct super_block *sb)
 {
-	if (!list)
-		return;
-	file_list_lock();
-	list_move(&file->f_u.fu_list, list);
-	file_list_unlock();
+	spin_lock(&files_lock);
+	BUG_ON(!list_empty(&file->f_u.fu_list));
+	list_add(&file->f_u.fu_list, &sb->s_files);
+	spin_unlock(&files_lock);
 }
  
-void file_kill(struct file *file)
+void file_sb_list_del(struct file *file)
 {
 	if (!list_empty(&file->f_u.fu_list)) {
-		file_list_lock();
+		spin_lock(&files_lock);
 		list_del_init(&file->f_u.fu_list);
-		file_list_unlock();
+		spin_unlock(&files_lock);
 	}
 }
  
@@ -361,7 +358,7 @@
 	struct file *file;
  
 	/* Check that no files are currently opened for writing. */
-	file_list_lock();
+	spin_lock(&files_lock);
 	list_for_each_entry(file, &sb->s_files, f_u.fu_list) {
 		struct inode *inode = file->f_path.dentry->d_inode;
  
  
@@ -373,10 +370,10 @@
 		if (S_ISREG(inode->i_mode) && (file->f_mode & FMODE_WRITE))
 			goto too_bad;
 	}
-	file_list_unlock();
+	spin_unlock(&files_lock);
 	return 1; /* Tis' cool bro. */
 too_bad:
-	file_list_unlock();
+	spin_unlock(&files_lock);
 	return 0;
 }
  
@@ -392,7 +389,7 @@
 	struct file *f;
  
 retry:
-	file_list_lock();
+	spin_lock(&files_lock);
 	list_for_each_entry(f, &sb->s_files, f_u.fu_list) {
 		struct vfsmount *mnt;
 		if (!S_ISREG(f->f_path.dentry->d_inode->i_mode))
  
@@ -408,16 +405,13 @@
 			continue;
 		file_release_write(f);
 		mnt = mntget(f->f_path.mnt);
-		file_list_unlock();
-		/*
-		 * This can sleep, so we can't hold
-		 * the file_list_lock() spinlock.
-		 */
+		/* This can sleep, so we can't hold the spinlock. */
+		spin_unlock(&files_lock);
 		mnt_drop_write(mnt);
 		mntput(mnt);
 		goto retry;
 	}
-	file_list_unlock();
+	spin_unlock(&files_lock);
 }
  
 void __init files_init(unsigned long mempages)
@@ -675,7 +675,7 @@
 	f->f_path.mnt = mnt;
 	f->f_pos = 0;
 	f->f_op = fops_get(inode->i_fop);
-	file_move(f, &inode->i_sb->s_files);
+	file_sb_list_add(f, inode->i_sb);
  
 	error = security_dentry_open(f, cred);
 	if (error)
@@ -721,7 +721,7 @@
 			mnt_drop_write(mnt);
 		}
 	}
-	file_kill(f);
+	file_sb_list_del(f);
 	f->f_path.dentry = NULL;
 	f->f_path.mnt = NULL;
 cleanup_file:
@@ -944,9 +944,6 @@
 	unsigned long f_mnt_write_state;
 #endif
 };
-extern spinlock_t files_lock;
-#define file_list_lock() spin_lock(&files_lock);
-#define file_list_unlock() spin_unlock(&files_lock);
  
 #define get_file(x)	atomic_long_inc(&(x)->f_count)
 #define fput_atomic(x)	atomic_long_add_unless(&(x)->f_count, -1, 1)
@@ -2188,8 +2185,8 @@
 	__insert_inode_hash(inode, inode->i_ino);
 }
  
-extern void file_move(struct file *f, struct list_head *list);
-extern void file_kill(struct file *f);
+extern void file_sb_list_add(struct file *f, struct super_block *sb);
+extern void file_sb_list_del(struct file *f);
 #ifdef CONFIG_BLOCK
 extern void submit_bio(int, struct bio *);
 extern int bdev_read_only(struct block_device *);
@@ -470,6 +470,7 @@
 extern struct tty_struct *tty_pair_get_pty(struct tty_struct *tty);
  
 extern struct mutex tty_mutex;
+extern spinlock_t tty_files_lock;
  
 extern void tty_write_unlock(struct tty_struct *tty);
 extern int tty_write_lock(struct tty_struct *tty, int ndelay);
@@ -2170,7 +2170,7 @@
  
 	tty = get_current_tty();
 	if (tty) {
-		file_list_lock();
+		spin_lock(&tty_files_lock);
 		if (!list_empty(&tty->tty_files)) {
 			struct inode *inode;
  
@@ -2186,7 +2186,7 @@
 				drop_tty = 1;
 			}
 		}
-		file_list_unlock();
+		spin_unlock(&tty_files_lock);
 		tty_kref_put(tty);
 	}
 	/* Reset controlling tty. */
...	...	@@ -676,7 +676,11 @@
676	676
677	677	set_bit(TTY_PTY_LOCK, &tty->flags); /* LOCK THE SLAVE */
678	678	filp->private_data = tty;
679		- file_move(filp, &tty->tty_files);
	679	+
	680	+ file_sb_list_del(filp); /* __dentry_open has put it on the sb list */
	681	+ spin_lock(&tty_files_lock);
	682	+ list_add(&filp->f_u.fu_list, &tty->tty_files);
	683	+ spin_unlock(&tty_files_lock);
680	684
681	685	retval = devpts_pty_new(inode, tty->link);
682	686	if (retval)
...	...	@@ -136,6 +136,9 @@
136	136	DEFINE_MUTEX(tty_mutex);
137	137	EXPORT_SYMBOL(tty_mutex);
138	138
	139	+/* Spinlock to protect the tty->tty_files list */
	140	+DEFINE_SPINLOCK(tty_files_lock);
	141	+
139	142	static ssize_t tty_read(struct file , char __user , size_t, loff_t *);
140	143	static ssize_t tty_write(struct file , const char __user , size_t, loff_t *);
141	144	ssize_t redirected_tty_write(struct file , const char __user ,
142	145
...	...	@@ -235,11 +238,11 @@
235	238	struct list_head *p;
236	239	int count = 0;
237	240
238		- file_list_lock();
	241	+ spin_lock(&tty_files_lock);
239	242	list_for_each(p, &tty->tty_files) {
240	243	count++;
241	244	}
242		- file_list_unlock();
	245	+ spin_unlock(&tty_files_lock);
243	246	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
244	247	tty->driver->subtype == PTY_TYPE_SLAVE &&
245	248	tty->link && tty->link->count)
...	...	@@ -519,7 +522,7 @@
519	522	workqueue with the lock held */
520	523	check_tty_count(tty, "tty_hangup");
521	524
522		- file_list_lock();
	525	+ spin_lock(&tty_files_lock);
523	526	/* This breaks for file handles being sent over AF_UNIX sockets ? */
524	527	list_for_each_entry(filp, &tty->tty_files, f_u.fu_list) {
525	528	if (filp->f_op->write == redirected_tty_write)
...	...	@@ -530,7 +533,7 @@
530	533	__tty_fasync(-1, filp, 0); /* can't block */
531	534	filp->f_op = &hung_up_tty_fops;
532	535	}
533		- file_list_unlock();
	536	+ spin_unlock(&tty_files_lock);
534	537
535	538	tty_ldisc_hangup(tty);
536	539
537	540
...	...	@@ -1424,9 +1427,9 @@
1424	1427	tty_driver_kref_put(driver);
1425	1428	module_put(driver->owner);
1426	1429
1427		- file_list_lock();
	1430	+ spin_lock(&tty_files_lock);
1428	1431	list_del_init(&tty->tty_files);
1429		- file_list_unlock();
	1432	+ spin_unlock(&tty_files_lock);
1430	1433
1431	1434	put_pid(tty->pgrp);
1432	1435	put_pid(tty->session);
...	...	@@ -1671,7 +1674,10 @@
1671	1674	* - do_tty_hangup no longer sees this file descriptor as
1672	1675	* something that needs to be handled for hangups.
1673	1676	*/
1674		- file_kill(filp);
	1677	+ spin_lock(&tty_files_lock);
	1678	+ BUG_ON(list_empty(&filp->f_u.fu_list));
	1679	+ list_del_init(&filp->f_u.fu_list);
	1680	+ spin_unlock(&tty_files_lock);
1675	1681	filp->private_data = NULL;
1676	1682
1677	1683	/*
...	...	@@ -1840,7 +1846,11 @@
1840	1846	}
1841	1847
1842	1848	filp->private_data = tty;
1843		- file_move(filp, &tty->tty_files);
	1849	+ BUG_ON(list_empty(&filp->f_u.fu_list));
	1850	+ file_sb_list_del(filp); /* __dentry_open has put it on the sb list */
	1851	+ spin_lock(&tty_files_lock);
	1852	+ list_add(&filp->f_u.fu_list, &tty->tty_files);
	1853	+ spin_unlock(&tty_files_lock);
1844	1854	check_tty_count(tty, "tty_open");
1845	1855	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
1846	1856	tty->driver->subtype == PTY_TYPE_MASTER)
...	...	@@ -32,8 +32,7 @@
32	32	.max_files = NR_FILE
33	33	};
34	34
35		-/* public. Not pretty! */
36		-__cacheline_aligned_in_smp DEFINE_SPINLOCK(files_lock);
	35	+static __cacheline_aligned_in_smp DEFINE_SPINLOCK(files_lock);
37	36
38	37	/* SLAB cache for file structures */
39	38	static struct kmem_cache *filp_cachep __read_mostly;
...	...	@@ -249,7 +248,7 @@
249	248	cdev_put(inode->i_cdev);
250	249	fops_put(file->f_op);
251	250	put_pid(file->f_owner.pid);
252		- file_kill(file);
	251	+ file_sb_list_del(file);
253	252	if (file->f_mode & FMODE_WRITE)
254	253	drop_file_write_access(file);
255	254	file->f_path.dentry = NULL;
256	255
257	256
258	257
259	258
260	259
261	260
...	...	@@ -328,31 +327,29 @@
328	327	return file;
329	328	}
330	329
331		-
332	330	void put_filp(struct file *file)
333	331	{
334	332	if (atomic_long_dec_and_test(&file->f_count)) {
335	333	security_file_free(file);
336		- file_kill(file);
	334	+ file_sb_list_del(file);
337	335	file_free(file);
338	336	}
339	337	}
340	338
341		-void file_move(struct file file, struct list_head list)
	339	+void file_sb_list_add(struct file file, struct super_block sb)
342	340	{
343		- if (!list)
344		- return;
345		- file_list_lock();
346		- list_move(&file->f_u.fu_list, list);
347		- file_list_unlock();
	341	+ spin_lock(&files_lock);
	342	+ BUG_ON(!list_empty(&file->f_u.fu_list));
	343	+ list_add(&file->f_u.fu_list, &sb->s_files);
	344	+ spin_unlock(&files_lock);
348	345	}
349	346
350		-void file_kill(struct file *file)
	347	+void file_sb_list_del(struct file *file)
351	348	{
352	349	if (!list_empty(&file->f_u.fu_list)) {
353		- file_list_lock();
	350	+ spin_lock(&files_lock);
354	351	list_del_init(&file->f_u.fu_list);
355		- file_list_unlock();
	352	+ spin_unlock(&files_lock);
356	353	}
357	354	}
358	355
...	...	@@ -361,7 +358,7 @@
361	358	struct file *file;
362	359
363	360	/* Check that no files are currently opened for writing. */
364		- file_list_lock();
	361	+ spin_lock(&files_lock);
365	362	list_for_each_entry(file, &sb->s_files, f_u.fu_list) {
366	363	struct inode *inode = file->f_path.dentry->d_inode;
367	364
368	365
...	...	@@ -373,10 +370,10 @@
373	370	if (S_ISREG(inode->i_mode) && (file->f_mode & FMODE_WRITE))
374	371	goto too_bad;
375	372	}
376		- file_list_unlock();
	373	+ spin_unlock(&files_lock);
377	374	return 1; /* Tis' cool bro. */
378	375	too_bad:
379		- file_list_unlock();
	376	+ spin_unlock(&files_lock);
380	377	return 0;
381	378	}
382	379
...	...	@@ -392,7 +389,7 @@
392	389	struct file *f;
393	390
394	391	retry:
395		- file_list_lock();
	392	+ spin_lock(&files_lock);
396	393	list_for_each_entry(f, &sb->s_files, f_u.fu_list) {
397	394	struct vfsmount *mnt;
398	395	if (!S_ISREG(f->f_path.dentry->d_inode->i_mode))
399	396
...	...	@@ -408,16 +405,13 @@
408	405	continue;
409	406	file_release_write(f);
410	407	mnt = mntget(f->f_path.mnt);
411		- file_list_unlock();
412		- /*
413		- * This can sleep, so we can't hold
414		- * the file_list_lock() spinlock.
415		- */
	408	+ /* This can sleep, so we can't hold the spinlock. */
	409	+ spin_unlock(&files_lock);
416	410	mnt_drop_write(mnt);
417	411	mntput(mnt);
418	412	goto retry;
419	413	}
420		- file_list_unlock();
	414	+ spin_unlock(&files_lock);
421	415	}
422	416
423	417	void __init files_init(unsigned long mempages)
...	...	@@ -675,7 +675,7 @@
675	675	f->f_path.mnt = mnt;
676	676	f->f_pos = 0;
677	677	f->f_op = fops_get(inode->i_fop);
678		- file_move(f, &inode->i_sb->s_files);
	678	+ file_sb_list_add(f, inode->i_sb);
679	679
680	680	error = security_dentry_open(f, cred);
681	681	if (error)
...	...	@@ -721,7 +721,7 @@
721	721	mnt_drop_write(mnt);
722	722	}
723	723	}
724		- file_kill(f);
	724	+ file_sb_list_del(f);
725	725	f->f_path.dentry = NULL;
726	726	f->f_path.mnt = NULL;
727	727	cleanup_file:
...	...	@@ -944,9 +944,6 @@
944	944	unsigned long f_mnt_write_state;
945	945	#endif
946	946	};
947		-extern spinlock_t files_lock;
948		-#define file_list_lock() spin_lock(&files_lock);
949		-#define file_list_unlock() spin_unlock(&files_lock);
950	947
951	948	#define get_file(x) atomic_long_inc(&(x)->f_count)
952	949	#define fput_atomic(x) atomic_long_add_unless(&(x)->f_count, -1, 1)
...	...	@@ -2188,8 +2185,8 @@
2188	2185	__insert_inode_hash(inode, inode->i_ino);
2189	2186	}
2190	2187
2191		-extern void file_move(struct file f, struct list_head list);
2192		-extern void file_kill(struct file *f);
	2188	+extern void file_sb_list_add(struct file f, struct super_block sb);
	2189	+extern void file_sb_list_del(struct file *f);
2193	2190	#ifdef CONFIG_BLOCK
2194	2191	extern void submit_bio(int, struct bio *);
2195	2192	extern int bdev_read_only(struct block_device *);
...	...	@@ -470,6 +470,7 @@
470	470	extern struct tty_struct tty_pair_get_pty(struct tty_struct tty);
471	471
472	472	extern struct mutex tty_mutex;
	473	+extern spinlock_t tty_files_lock;
473	474
474	475	extern void tty_write_unlock(struct tty_struct *tty);
475	476	extern int tty_write_lock(struct tty_struct *tty, int ndelay);
...	...	@@ -2170,7 +2170,7 @@
2170	2170
2171	2171	tty = get_current_tty();
2172	2172	if (tty) {
2173		- file_list_lock();
	2173	+ spin_lock(&tty_files_lock);
2174	2174	if (!list_empty(&tty->tty_files)) {
2175	2175	struct inode *inode;
2176	2176
...	...	@@ -2186,7 +2186,7 @@
2186	2186	drop_tty = 1;
2187	2187	}
2188	2188	}
2189		- file_list_unlock();
	2189	+ spin_unlock(&tty_files_lock);
2190	2190	tty_kref_put(tty);
2191	2191	}
2192	2192	/* Reset controlling tty. */